mirror of
https://github.com/k3s-io/kubernetes.git
synced 2026-07-18 13:57:38 +00:00
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]:461ba83084/pkg/kubelet/kubelet.go (L1078-L1087)[2]:461ba83084/pkg/kubelet/kubelet_getters.go (L145)Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This commit is contained in:
committed by
Akihiro Suda
parent
b35220c725
commit
db6a2b2318
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -26,5 +26,4 @@ type userNsPodsManager interface {
|
||||
ListPodsFromDisk() ([]types.UID, error)
|
||||
GetKubeletMappings() (uint32, uint32, error)
|
||||
GetMaxPods() int
|
||||
GetUserNamespacesIDsPerPod() uint32
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user