Skip to content

Commit

Permalink
API changes for Pod Level Resources
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ndixita committed Nov 8, 2024
1 parent 210deea commit d7f488b
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 0 deletions.
31 changes: 31 additions & 0 deletions pkg/api/pod/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ func dropDisabledFields(
}
}

dropDisabledPodLevelResources(podSpec, oldPodSpec)
dropDisabledProcMountField(podSpec, oldPodSpec)

dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
143 changes: 143 additions & 0 deletions pkg/api/pod/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions pkg/apis/core/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions pkg/features/versioned_kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
14 changes: 14 additions & 0 deletions staging/src/k8s.io/api/core/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d7f488b

Please sign in to comment.