diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index b327a713b9b..9abdacb93a7 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -245,6 +245,7 @@ var ( lifecycle.PodOSNotSupported, lifecycle.InvalidNodeInfo, lifecycle.InitContainerRestartPolicyForbidden, + lifecycle.SupplementalGroupsPolicyNotSupported, lifecycle.UnexpectedAdmissionError, lifecycle.UnknownReason, lifecycle.UnexpectedPredicateFailureType, diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index ea7dc956b35..364f06ef3e7 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3657,6 +3657,15 @@ func TestRecordAdmissionRejection(t *testing.T) { # HELP kubelet_admission_rejections_total [ALPHA] Cumulative number pod admission rejections by the Kubelet. # TYPE kubelet_admission_rejections_total counter kubelet_admission_rejections_total{reason="InitContainerRestartPolicyForbidden"} 1 + `, + }, + { + name: "SupplementalGroupsPolicyNotSupported", + reason: lifecycle.SupplementalGroupsPolicyNotSupported, + wants: ` + # HELP kubelet_admission_rejections_total [ALPHA] Cumulative number pod admission rejections by the Kubelet. + # TYPE kubelet_admission_rejections_total counter + kubelet_admission_rejections_total{reason="SupplementalGroupsPolicyNotSupported"} 1 `, }, { diff --git a/pkg/kubelet/lifecycle/predicate.go b/pkg/kubelet/lifecycle/predicate.go index 9f1c5cce7b2..a505faca694 100644 --- a/pkg/kubelet/lifecycle/predicate.go +++ b/pkg/kubelet/lifecycle/predicate.go @@ -21,13 +21,17 @@ import ( "runtime" v1 "k8s.io/api/core/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" "k8s.io/component-helpers/scheduling/corev1" "k8s.io/klog/v2" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/types" "k8s.io/kubernetes/pkg/scheduler" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration" + "k8s.io/utils/ptr" ) const ( @@ -49,6 +53,11 @@ const ( // than Always for some of its init containers. InitContainerRestartPolicyForbidden = "InitContainerRestartPolicyForbidden" + // SupplementalGroupsPolicyNotSupported is used to denote that the pod was + // rejected admission to the node because the node does not support + // the pod's SupplementalGroupsPolicy. + SupplementalGroupsPolicyNotSupported = "SupplementalGroupsPolicyNotSupported" + // UnexpectedAdmissionError is used to denote that the pod was rejected // admission to the node because of an error during admission that could not // be categorized. @@ -132,6 +141,16 @@ func (w *predicateAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmitResult } } + if rejectPodAdmissionBasedOnSupplementalGroupsPolicy(admitPod, node) { + message := fmt.Sprintf("SupplementalGroupsPolicy=%s is not supported in this node", v1.SupplementalGroupsPolicyStrict) + klog.InfoS("Failed to admit pod", "pod", klog.KObj(admitPod), "message", message) + return PodAdmitResult{ + Admit: false, + Reason: SupplementalGroupsPolicyNotSupported, + Message: message, + } + } + pods := attrs.OtherPods nodeInfo := schedulerframework.NewNodeInfo(pods...) nodeInfo.SetNode(node) @@ -254,6 +273,45 @@ func rejectPodAdmissionBasedOnOSField(pod *v1.Pod) bool { return string(pod.Spec.OS.Name) != runtime.GOOS } +// rejectPodAdmissionBasedOnSupplementalGroupsPolicy rejects pod only if +// - the feature is beta or above, and SupplementalPolicy=Strict is set in the pod +// - but, the node does not support the feature +// +// Note: During the feature is alpha or before(not yet released) in emulated version, +// it should admit for backward compatibility +func rejectPodAdmissionBasedOnSupplementalGroupsPolicy(pod *v1.Pod, node *v1.Node) bool { + admit, reject := false, true // just for readability + + inUse := (pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SupplementalGroupsPolicy != nil) + if !inUse { + return admit + } + + isBetaOrAbove := false + if featureSpec, ok := utilfeature.DefaultMutableFeatureGate.GetAll()[features.SupplementalGroupsPolicy]; ok { + isBetaOrAbove = (featureSpec.PreRelease == featuregate.Beta) || (featureSpec.PreRelease == featuregate.GA) + } + + if !isBetaOrAbove { + return admit + } + + featureSupportedOnNode := ptr.Deref( + ptr.Deref(node.Status.Features, v1.NodeFeatures{SupplementalGroupsPolicy: ptr.To(false)}).SupplementalGroupsPolicy, + false, + ) + effectivePolicy := ptr.Deref( + pod.Spec.SecurityContext.SupplementalGroupsPolicy, + v1.SupplementalGroupsPolicyMerge, + ) + + if effectivePolicy == v1.SupplementalGroupsPolicyStrict && !featureSupportedOnNode { + return reject + } + + return admit +} + func removeMissingExtendedResources(pod *v1.Pod, nodeInfo *schedulerframework.NodeInfo) *v1.Pod { filterExtendedResources := func(containers []v1.Container) { for i, c := range containers { diff --git a/pkg/kubelet/lifecycle/predicate_test.go b/pkg/kubelet/lifecycle/predicate_test.go index 98db94d39b9..505959af900 100644 --- a/pkg/kubelet/lifecycle/predicate_test.go +++ b/pkg/kubelet/lifecycle/predicate_test.go @@ -24,12 +24,16 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilversion "k8s.io/apimachinery/pkg/util/version" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/kubelet/types" schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodename" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodeports" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration" + "k8s.io/utils/ptr" ) var ( @@ -431,3 +435,115 @@ func TestRejectPodAdmissionBasedOnOSField(t *testing.T) { }) } } + +func TestPodAdmissionBasedOnSupplementalGroupsPolicy(t *testing.T) { + nodeSupportedTheFeature := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Status: v1.NodeStatus{ + Features: &v1.NodeFeatures{ + SupplementalGroupsPolicy: ptr.To(true), + }, + }, + } + nodeNotSupportedTheFeature := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + } + podNotUsedTheFeature := &v1.Pod{} + podUsedTheFeature := &v1.Pod{Spec: v1.PodSpec{ + SecurityContext: &v1.PodSecurityContext{ + SupplementalGroupsPolicy: ptr.To(v1.SupplementalGroupsPolicyStrict), + }, + }} + tests := []struct { + name string + emulationVersion *utilversion.Version + node *v1.Node + pod *v1.Pod + expectRejection bool + }{ + // The feature is Beta in v1.33 + { + name: "feature=Beta, node=feature not supported, pod=in use: it should REJECT", + emulationVersion: utilversion.MustParse("1.33"), + node: nodeNotSupportedTheFeature, + pod: podUsedTheFeature, + expectRejection: true, + }, + { + name: "feature=Beta, node=feature supported, pod=in use: it should ADMIT", + emulationVersion: utilversion.MustParse("1.33"), + node: nodeSupportedTheFeature, + pod: podUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=Beta, node=feature not supported, pod=not in use: it should ADMIT", + emulationVersion: utilversion.MustParse("1.33"), + node: nodeNotSupportedTheFeature, + pod: podNotUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=Beta, node=feature supported, pod=not in use: it should ADMIT", + emulationVersion: utilversion.MustParse("1.33"), + node: nodeSupportedTheFeature, + pod: podNotUsedTheFeature, + expectRejection: false, + }, + // The feature is Alpha(v1.31, v1.32) in emulated version + // Note: When the feature is alpha in emulated version, it should always admit for backward compatibility + { + name: "feature=Alpha, node=feature not supported, pod=feature used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.32"), + node: nodeNotSupportedTheFeature, + pod: podUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=Alpha, node=feature not supported, pod=feature not used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.32"), + node: nodeNotSupportedTheFeature, + pod: podNotUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=Alpha, node=feature supported, pod=feature used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.32"), + node: nodeSupportedTheFeature, + pod: podUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=Alpha, node=feature supported, pod=feature not used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.32"), + node: nodeSupportedTheFeature, + pod: podNotUsedTheFeature, + expectRejection: false, + }, + // The feature is not yet released (< v1.31) in emulated version (this can happen when only kubelet downgraded). + // Note: When the feature is not yet released in emulated version, it should always admit for backward compatibility + { + name: "feature=NotReleased, node=feature not supported, pod=feature used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.30"), + node: nodeNotSupportedTheFeature, + pod: podUsedTheFeature, + expectRejection: false, + }, + { + name: "feature=NotReleased, node=feature not supported, pod=feature not used: it should ADMIT", + emulationVersion: utilversion.MustParse("1.30"), + node: nodeNotSupportedTheFeature, + pod: podNotUsedTheFeature, + expectRejection: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, test.emulationVersion) + actualResult := rejectPodAdmissionBasedOnSupplementalGroupsPolicy(test.pod, test.node) + if test.expectRejection != actualResult { + t.Errorf("unexpected result, expected %v but got %v", test.expectRejection, actualResult) + } + }) + } +}