From d7f488b5e341dec8f6ef180f488e788a4103821c Mon Sep 17 00:00:00 2001 From: ndixita Date: Wed, 16 Oct 2024 17:01:47 +0000 Subject: [PATCH] API changes for Pod Level Resources 1. Add Resources struct to PodSpec struct in both external and internal API packages 2. Adding feature gate and logic for dropping disabled fields for Pod Level Resources KEP: enhancements/keps/sig-node/2837-pod-level-resource-spec --- pkg/api/pod/util.go | 31 +++++ pkg/api/pod/util_test.go | 143 ++++++++++++++++++++++++ pkg/apis/core/types.go | 14 +++ pkg/features/kube_features.go | 7 ++ pkg/features/versioned_kube_features.go | 4 + staging/src/k8s.io/api/core/v1/types.go | 14 +++ 6 files changed, 213 insertions(+) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 9ad259147d0..b472d00fe10 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -621,6 +621,7 @@ func dropDisabledFields( } } + dropDisabledPodLevelResources(podSpec, oldPodSpec) dropDisabledProcMountField(podSpec, oldPodSpec) dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec) @@ -674,6 +675,14 @@ func dropDisabledFields( dropSELinuxChangePolicy(podSpec, oldPodSpec) } +func dropDisabledPodLevelResources(podSpec, oldPodSpec *api.PodSpec) { + // If the feature is disabled and not in use, drop Resources at the pod-level + // from PodSpec. + if !utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources) && !podLevelResourcesInUse(oldPodSpec) { + podSpec.Resources = nil + } +} + func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) { if utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepAction) || podLifecycleSleepActionInUse(oldPodSpec) { return @@ -1050,6 +1059,28 @@ func supplementalGroupsPolicyInUse(podSpec *api.PodSpec) bool { return false } +// podLevelResourcesInUse returns true if pod-spec is non-nil and Resources field at +// pod-level has non-empty Requests or Limits. +func podLevelResourcesInUse(podSpec *api.PodSpec) bool { + if podSpec == nil { + return false + } + + if podSpec.Resources == nil { + return false + } + + if len(podSpec.Resources.Requests) > 0 { + return true + } + + if len(podSpec.Resources.Limits) > 0 { + return true + } + + return false +} + // inPlacePodVerticalScalingInUse returns true if pod spec is non-nil and ResizePolicy is set func inPlacePodVerticalScalingInUse(podSpec *api.PodSpec) bool { if podSpec == nil { diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 67ab805b7a6..2f2f58e3f81 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -2703,6 +2703,149 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) { } } +func TestDropPodLevelResources(t *testing.T) { + containers := []api.Container{ + { + Name: "c1", + Image: "image", + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("100m")}, + Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("200m")}, + }, + }, + } + podWithPodLevelResources := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + Resources: &api.ResourceRequirements{ + Requests: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100m"), + api.ResourceMemory: resource.MustParse("50Gi"), + }, + Limits: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100m"), + api.ResourceMemory: resource.MustParse("50Gi"), + }, + }, + Containers: containers, + }, + } + } + + podWithoutPodLevelResources := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + Containers: containers, + }, + } + } + + podInfo := []struct { + description string + hasPodLevelResources bool + pod func() *api.Pod + }{ + { + description: "has pod-level resources", + hasPodLevelResources: true, + pod: podWithPodLevelResources, + }, + { + description: "does not have pod-level resources", + hasPodLevelResources: false, + pod: podWithoutPodLevelResources, + }, + { + description: "is nil", + hasPodLevelResources: false, + pod: func() *api.Pod { return nil }, + }, + { + description: "is empty struct", + hasPodLevelResources: false, + // refactor to generalize and use podWithPodLevelResources() + pod: func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + Resources: &api.ResourceRequirements{}, + Containers: containers, + }, + } + }, + }, + { + description: "is empty Requests list", + hasPodLevelResources: false, + pod: func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{Resources: &api.ResourceRequirements{ + Requests: api.ResourceList{}, + }}} + }, + }, + { + description: "is empty Limits list", + hasPodLevelResources: false, + pod: func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{Resources: &api.ResourceRequirements{ + Limits: api.ResourceList{}, + }}} + }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPodInfo := range podInfo { + for _, newPodInfo := range podInfo { + oldPodHasPodLevelResources, oldPod := oldPodInfo.hasPodLevelResources, oldPodInfo.pod() + newPodHasPodLevelResources, newPod := newPodInfo.hasPodLevelResources, newPodInfo.pod() + if newPod == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodLevelResources, enabled) + + var oldPodSpec *api.PodSpec + if oldPod != nil { + oldPodSpec = &oldPod.Spec + } + + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) + + // old pod should never be changed + if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { + t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod())) + } + + switch { + case enabled || oldPodHasPodLevelResources: + // new pod shouldn't change if feature enabled or if old pod has + // any pod level resources + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod())) + } + case newPodHasPodLevelResources: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod was not changed") + } + // new pod should not have any pod-level resources + if !reflect.DeepEqual(newPod, podWithoutPodLevelResources()) { + t.Errorf("new pod has pod-level resources: %v", cmp.Diff(newPod, podWithoutPodLevelResources())) + } + default: + if newPod.Spec.Resources != nil { + t.Errorf("expected nil, got: %v", newPod.Spec.Resources) + } + } + }) + } + } + } +} + func TestDropSidecarContainers(t *testing.T) { containerRestartPolicyAlways := api.ContainerRestartPolicyAlways diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index a040527765c..220d018912f 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3609,6 +3609,20 @@ type PodSpec struct { // +featureGate=DynamicResourceAllocation // +optional ResourceClaims []PodResourceClaim + // Resources is the total amount of CPU and Memory resources required by all + // containers in the pod. It supports specifying Requests and Limits for + // "cpu" and "memory" resource names only. ResourceClaims are not supported. + // + // This field enables fine-grained control over resource allocation for the + // entire pod, allowing resource sharing among containers in a pod. + // TODO: For beta graduation, expand this comment with a detailed explanation. + // + // This is an alpha field and requires enabling the PodLevelResources feature + // gate. + // + // +featureGate=PodLevelResources + // +optional + Resources *ResourceRequirements } // PodResourceClaim references exactly one ResourceClaim through a ClaimSource. diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index fe387e8f6a9..7a6027aa1ac 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -835,6 +835,13 @@ const ( // Enables external service account JWT signing and key management. // If enabled, it allows passing --service-account-signing-endpoint flag to configure external signer. ExternalServiceAccountTokenSigner featuregate.Feature = "ExternalServiceAccountTokenSigner" + + // owner: @ndixita + // key: https://kep.k8s.io/2837 + // alpha: 1.32 + // + // Enables specifying resources at pod-level. + PodLevelResources featuregate.Feature = "PodLevelResources" ) func init() { diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index 693067e6c3c..7dc036b87d8 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -567,6 +567,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate {Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.35 }, + PodLevelResources: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, + }, + PodLifecycleSleepAction: { {Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index d1700f10e73..69e1428d4c9 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4080,6 +4080,20 @@ type PodSpec struct { // +featureGate=DynamicResourceAllocation // +optional ResourceClaims []PodResourceClaim `json:"resourceClaims,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,39,rep,name=resourceClaims"` + // Resources is the total amount of CPU and Memory resources required by all + // containers in the pod. It supports specifying Requests and Limits for + // "cpu" and "memory" resource names only. ResourceClaims are not supported. + // + // This field enables fine-grained control over resource allocation for the + // entire pod, allowing resource sharing among containers in a pod. + // TODO: For beta graduation, expand this comment with a detailed explanation. + // + // This is an alpha field and requires enabling the PodLevelResources feature + // gate. + // + // +featureGate=PodLevelResources + // +optional + Resources *ResourceRequirements `json:"resources,omitempty" protobuf:"bytes,40,opt,name=resources"` } // PodResourceClaim references exactly one ResourceClaim, either directly