From db6a2b2318a84e0cdbdfff8a871d11d3aacfc747 Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Tue, 29 Jul 2025 15:25:18 +0200 Subject: [PATCH] kubelet: Don't ignore idsPerPod config The idsPerPod where completely ignored since they were introduced in PR: https://github.com/kubernetes/kubernetes/pull/130028 The problem was the following: 1. The userns manager (as well as all managers) is created before the config is copied to the kubelet object[1] 2. The userns manager on creation is calling the kubelet_getter GetUserNamespacesIDsPerPod to get the idsPerPod 3. The getter checks the configuration stored in the kubelet object, which hasn't been set at that point. 4. As the config is nil (unset), it returns the default value. Therefore, the value was ignored. To solve this, let's just pass the idsPerPod as a parameter to MakeUserNsManager(). This is the common pattern already used in the kubelet initialization. [1]: https://github.com/kubernetes/kubernetes/blob/461ba83084ab7cb91ab692687bb7aedb05c6eb65/pkg/kubelet/kubelet.go#L1078-L1087 [2]: https://github.com/kubernetes/kubernetes/blob/461ba83084ab7cb91ab692687bb7aedb05c6eb65/pkg/kubelet/kubelet_getters.go#L145 Signed-off-by: Rodrigo Campos --- pkg/kubelet/kubelet.go | 11 +++++++--- pkg/kubelet/kubelet_test.go | 2 +- pkg/kubelet/userns/types.go | 1 - pkg/kubelet/userns/userns_manager.go | 10 +++++++-- .../userns/userns_manager_disabled_test.go | 8 +++---- .../userns/userns_manager_switch_test.go | 10 ++++----- pkg/kubelet/userns/userns_manager_test.go | 22 +++++++------------ pkg/kubelet/userns/userns_manager_windows.go | 2 +- 8 files changed, 35 insertions(+), 31 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 0f1806958b8..13622516eec 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1076,12 +1076,17 @@ func NewMainKubelet(ctx context.Context, StateDirectory: rootDirectory, }) klet.shutdownManager = shutdownManager - klet.usernsManager, err = userns.MakeUserNsManager(logger, klet) + handlers = append(handlers, shutdownManager) + klet.allocationManager.AddPodAdmitHandlers(handlers) + + var usernsIDsPerPod *int64 + if kubeCfg.UserNamespaces != nil { + usernsIDsPerPod = kubeCfg.UserNamespaces.IDsPerPod + } + klet.usernsManager, err = userns.MakeUserNsManager(logger, klet, usernsIDsPerPod) if err != nil { return nil, fmt.Errorf("create user namespace manager: %w", err) } - handlers = append(handlers, shutdownManager) - klet.allocationManager.AddPodAdmitHandlers(handlers) // Finally, put the most recent version of the config on the Kubelet, so // people can see how it was configured. diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 5d3e476cc7d..4fb0fc0a811 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -405,7 +405,7 @@ func newTestKubeletWithImageList( ShutdownGracePeriodCriticalPods: 0, }) kubelet.shutdownManager = shutdownManager - kubelet.usernsManager, err = userns.MakeUserNsManager(logger, kubelet) + kubelet.usernsManager, err = userns.MakeUserNsManager(logger, kubelet, nil) if err != nil { t.Fatalf("Failed to create UserNsManager: %v", err) } diff --git a/pkg/kubelet/userns/types.go b/pkg/kubelet/userns/types.go index 820c2aaadca..a0422d0042c 100644 --- a/pkg/kubelet/userns/types.go +++ b/pkg/kubelet/userns/types.go @@ -26,5 +26,4 @@ type userNsPodsManager interface { ListPodsFromDisk() ([]types.UID, error) GetKubeletMappings() (uint32, uint32, error) GetMaxPods() int - GetUserNamespacesIDsPerPod() uint32 } diff --git a/pkg/kubelet/userns/userns_manager.go b/pkg/kubelet/userns/userns_manager.go index 472ebea034c..a0e00976f9b 100644 --- a/pkg/kubelet/userns/userns_manager.go +++ b/pkg/kubelet/userns/userns_manager.go @@ -35,6 +35,7 @@ import ( "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + "k8s.io/kubernetes/pkg/kubelet/kubeletconfig" utilstore "k8s.io/kubernetes/pkg/kubelet/util/store" "k8s.io/kubernetes/pkg/registry/core/service/allocator" utilfs "k8s.io/kubernetes/pkg/util/filesystem" @@ -129,13 +130,18 @@ func (m *UsernsManager) readMappingsFromFile(pod types.UID) ([]byte, error) { return fstore.Read(mappingsFile) } -func MakeUserNsManager(logger klog.Logger, kl userNsPodsManager) (*UsernsManager, error) { +func MakeUserNsManager(logger klog.Logger, kl userNsPodsManager, idsPerPod *int64) (*UsernsManager, error) { kubeletMappingID, kubeletMappingLen, err := kl.GetKubeletMappings() if err != nil { return nil, fmt.Errorf("kubelet mappings: %w", err) } - userNsLength := kl.GetUserNamespacesIDsPerPod() + userNsLength := uint32(kubeletconfig.DefaultKubeletUserNamespacesIDsPerPod) + if idsPerPod != nil { + // The value is already validated as part of kubelet config validation, so we can safely + // cast it. + userNsLength = uint32(*idsPerPod) + } if userNsLength%userNsUnitLength != 0 { return nil, fmt.Errorf("kubelet user namespace length %v is not a multiple of %d", userNsLength, userNsUnitLength) diff --git a/pkg/kubelet/userns/userns_manager_disabled_test.go b/pkg/kubelet/userns/userns_manager_disabled_test.go index b5e66737bf0..67f633009ef 100644 --- a/pkg/kubelet/userns/userns_manager_disabled_test.go +++ b/pkg/kubelet/userns/userns_manager_disabled_test.go @@ -38,7 +38,7 @@ func TestMakeUserNsManagerDisabled(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false) testUserNsPodsManager := &testUserNsPodsManager{} - _, err := MakeUserNsManager(logger, testUserNsPodsManager) + _, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) assert.NoError(t, err) } @@ -47,7 +47,7 @@ func TestReleaseDisabled(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false) testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) m.Release(logger, "some-pod") @@ -100,7 +100,7 @@ func TestGetOrCreateUserNamespaceMappingsDisabled(t *testing.T) { t.Run(test.name, func(t *testing.T) { logger, ctx := ktesting.NewTestContext(t) testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) userns, err := m.GetOrCreateUserNamespaceMappings(ctx, test.pod, "") @@ -119,7 +119,7 @@ func TestCleanupOrphanedPodUsernsAllocationsDisabled(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false) testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) err = m.CleanupOrphanedPodUsernsAllocations(ctx, nil, nil) diff --git a/pkg/kubelet/userns/userns_manager_switch_test.go b/pkg/kubelet/userns/userns_manager_switch_test.go index 0cd8090276e..0177914023d 100644 --- a/pkg/kubelet/userns/userns_manager_switch_test.go +++ b/pkg/kubelet/userns/userns_manager_switch_test.go @@ -46,7 +46,7 @@ func TestMakeUserNsManagerSwitch(t *testing.T) { // manager, it will find these pods on disk with userns data. podList: pods, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // Record the pods on disk. @@ -59,7 +59,7 @@ func TestMakeUserNsManagerSwitch(t *testing.T) { // Test re-init works when the feature gate is disabled and there were some // pods written on disk. featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false) - m2, err := MakeUserNsManager(logger, testUserNsPodsManager) + m2, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // The feature gate is off, no pods should be allocated. @@ -82,7 +82,7 @@ func TestGetOrCreateUserNamespaceMappingsSwitch(t *testing.T) { // manager, it will find these pods on disk with userns data. podList: pods, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // Record the pods on disk. @@ -96,7 +96,7 @@ func TestGetOrCreateUserNamespaceMappingsSwitch(t *testing.T) { // pods registered on disk. featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, false) // Create a new manager with the feature gate off and verify the userns range is nil. - m2, err := MakeUserNsManager(logger, testUserNsPodsManager) + m2, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) for _, podUID := range pods { @@ -120,7 +120,7 @@ func TestCleanupOrphanedPodUsernsAllocationsSwitch(t *testing.T) { podList: listPods, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // Record the pods on disk. diff --git a/pkg/kubelet/userns/userns_manager_test.go b/pkg/kubelet/userns/userns_manager_test.go index 1df4c7979fd..6f4d9a9c677 100644 --- a/pkg/kubelet/userns/userns_manager_test.go +++ b/pkg/kubelet/userns/userns_manager_test.go @@ -93,13 +93,6 @@ func (m *testUserNsPodsManager) GetMaxPods() int { return testMaxPods } -func (m *testUserNsPodsManager) GetUserNamespacesIDsPerPod() uint32 { - if m.userNsLength != 0 { - return m.userNsLength - } - return testUserNsLength -} - func TestUserNsManagerAllocate(t *testing.T) { logger, _ := ktesting.NewTestContext(t) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true) @@ -133,7 +126,8 @@ func TestUserNsManagerAllocate(t *testing.T) { mappingFirstID: tc.mappingFirstID, mappingLen: tc.mappingLen, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + idsPerPod := int64(tc.userNsLength) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, &idsPerPod) require.NoError(t, err) allocated, length, err := m.allocateOne(logger, "one") @@ -226,7 +220,7 @@ func TestMakeUserNsManager(t *testing.T) { mappingLen: tc.mappingLen, maxPods: tc.maxPods, } - _, err := MakeUserNsManager(logger, testUserNsPodsManager) + _, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) if tc.success { assert.NoError(t, err) @@ -305,7 +299,7 @@ func TestUserNsManagerParseUserNsFile(t *testing.T) { } testUserNsPodsManager := &testUserNsPodsManager{} - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) assert.NoError(t, err) for _, tc := range cases { @@ -397,7 +391,7 @@ func TestGetOrCreateUserNamespaceMappings(t *testing.T) { userns: tc.runtimeUserns, } logger, ctx := ktesting.NewTestContext(t) - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) assert.NoError(t, err) userns, err := m.GetOrCreateUserNamespaceMappings(ctx, tc.pod, tc.runtimeHandler) @@ -470,7 +464,7 @@ func TestCleanupOrphanedPodUsernsAllocations(t *testing.T) { podDir: t.TempDir(), podList: tc.listPods, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // Record the userns range as used @@ -508,7 +502,7 @@ func TestMakeUserNsManagerFailsListPod(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.UserNamespacesSupport, true) testUserNsPodsManager := &failingUserNsPodsManager{} - _, err := MakeUserNsManager(logger, testUserNsPodsManager) + _, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) assert.Error(t, err) assert.ErrorContains(t, err, "read pods from disk") } @@ -523,7 +517,7 @@ func TestRecordBounds(t *testing.T) { mappingLen: 65536, maxPods: 1, } - m, err := MakeUserNsManager(logger, testUserNsPodsManager) + m, err := MakeUserNsManager(logger, testUserNsPodsManager, nil) require.NoError(t, err) // The first pod allocation should succeed. diff --git a/pkg/kubelet/userns/userns_manager_windows.go b/pkg/kubelet/userns/userns_manager_windows.go index 4620842c156..2bd5c8390d4 100644 --- a/pkg/kubelet/userns/userns_manager_windows.go +++ b/pkg/kubelet/userns/userns_manager_windows.go @@ -28,7 +28,7 @@ import ( type UsernsManager struct{} -func MakeUserNsManager(klog.Logger, userNsPodsManager) (*UsernsManager, error) { +func MakeUserNsManager(klog.Logger, userNsPodsManager, *int64) (*UsernsManager, error) { return nil, nil }