From 6cb716e254a9032a35f9048c0b448afdbc89d512 Mon Sep 17 00:00:00 2001 From: Joyce Ma Date: Tue, 2 Jan 2024 23:25:37 +0000 Subject: [PATCH] Make a local copy of ResourceRequirements type without Claims field --- ....cloud.google.com_controllerresources.yaml | 44 ------------------- ...gle.com_namespacedcontrollerresources.yaml | 44 ------------------- .../v1alpha1/controllerresource_types.go | 19 +++++++- .../v1alpha1/zz_generated.deepcopy.go | 30 +++++++++++++ .../v1beta1/controllerresource_types.go | 19 +++++++- .../v1beta1/zz_generated.deepcopy.go | 30 +++++++++++++ .../configconnector_controller_test.go | 2 +- .../configconnectorcontext_controller_test.go | 4 +- operator/pkg/controllers/util_test.go | 26 ++++++----- operator/pkg/controllers/utils.go | 9 ++-- operator/pkg/test/controller/customization.go | 12 ++--- 11 files changed, 123 insertions(+), 116 deletions(-) diff --git a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerresources.yaml b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerresources.yaml index a61609ceab..c7bda7688b 100644 --- a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerresources.yaml +++ b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_controllerresources.yaml @@ -59,28 +59,6 @@ spec: description: Resources specifies the resource customization of this container. Required properties: - claims: - description: "Claims lists the names of resources, defined - in spec.resourceClaims, that are used by this container. - \n This is an alpha field and requires enabling the DynamicResourceAllocation - feature gate. \n This field is immutable. It can only - be set for containers." - items: - description: ResourceClaim references one entry in PodSpec.ResourceClaims. - properties: - name: - description: Name must match the name of one entry - in pod.spec.resourceClaims of the Pod where this - field is used. It makes that resource available - inside a container. - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map limits: additionalProperties: anyOf: @@ -182,28 +160,6 @@ spec: description: Resources specifies the resource customization of this container. Required properties: - claims: - description: "Claims lists the names of resources, defined - in spec.resourceClaims, that are used by this container. - \n This is an alpha field and requires enabling the DynamicResourceAllocation - feature gate. \n This field is immutable. It can only - be set for containers." - items: - description: ResourceClaim references one entry in PodSpec.ResourceClaims. - properties: - name: - description: Name must match the name of one entry - in pod.spec.resourceClaims of the Pod where this - field is used. It makes that resource available - inside a container. - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map limits: additionalProperties: anyOf: diff --git a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerresources.yaml b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerresources.yaml index bfe06e15e4..406e5c7827 100644 --- a/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerresources.yaml +++ b/operator/config/crd/bases/customize.core.cnrm.cloud.google.com_namespacedcontrollerresources.yaml @@ -60,28 +60,6 @@ spec: description: Resources specifies the resource customization of this container. Required properties: - claims: - description: "Claims lists the names of resources, defined - in spec.resourceClaims, that are used by this container. - \n This is an alpha field and requires enabling the DynamicResourceAllocation - feature gate. \n This field is immutable. It can only - be set for containers." - items: - description: ResourceClaim references one entry in PodSpec.ResourceClaims. - properties: - name: - description: Name must match the name of one entry - in pod.spec.resourceClaims of the Pod where this - field is used. It makes that resource available - inside a container. - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map limits: additionalProperties: anyOf: @@ -181,28 +159,6 @@ spec: description: Resources specifies the resource customization of this container. Required properties: - claims: - description: "Claims lists the names of resources, defined - in spec.resourceClaims, that are used by this container. - \n This is an alpha field and requires enabling the DynamicResourceAllocation - feature gate. \n This field is immutable. It can only - be set for containers." - items: - description: ResourceClaim references one entry in PodSpec.ResourceClaims. - properties: - name: - description: Name must match the name of one entry - in pod.spec.resourceClaims of the Pod where this - field is used. It makes that resource available - inside a container. - type: string - required: - - name - type: object - type: array - x-kubernetes-list-map-keys: - - name - x-kubernetes-list-type: map limits: additionalProperties: anyOf: diff --git a/operator/pkg/apis/core/customize/v1alpha1/controllerresource_types.go b/operator/pkg/apis/core/customize/v1alpha1/controllerresource_types.go index 40245df6e5..49a34de6cd 100644 --- a/operator/pkg/apis/core/customize/v1alpha1/controllerresource_types.go +++ b/operator/pkg/apis/core/customize/v1alpha1/controllerresource_types.go @@ -55,7 +55,24 @@ type ContainerResourceSpec struct { Name string `json:"name"` // Resources specifies the resource customization of this container. // Required - Resources corev1.ResourceRequirements `json:"resources"` + Resources ResourceRequirements `json:"resources"` +} + +// ResourceRequirements describes the compute resource requirements that +// ContainerResourceSpec can leverage. +// It is a local copy of ResourceRequirements in k8s.io/api/core/v1 containing +// a subset of its fields. +type ResourceRequirements struct { + // Limits describes the maximum amount of compute resources allowed. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Limits corev1.ResourceList `json:"limits,omitempty" protobuf:"bytes,1,rep,name=limits,casttype=ResourceList,castkey=ResourceName"` + // Requests describes the minimum amount of compute resources required. + // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value. Requests cannot exceed Limits. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Requests corev1.ResourceList `json:"requests,omitempty" protobuf:"bytes,2,rep,name=requests,casttype=ResourceList,castkey=ResourceName"` } // ControllerResourceStatus defines the observed state of ControllerResource. diff --git a/operator/pkg/apis/core/customize/v1alpha1/zz_generated.deepcopy.go b/operator/pkg/apis/core/customize/v1alpha1/zz_generated.deepcopy.go index 3e6945b2a1..a07fdd7116 100644 --- a/operator/pkg/apis/core/customize/v1alpha1/zz_generated.deepcopy.go +++ b/operator/pkg/apis/core/customize/v1alpha1/zz_generated.deepcopy.go @@ -6,6 +6,7 @@ package v1alpha1 import ( + "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -283,6 +284,35 @@ func (in *NamespacedControllerResourceStatus) DeepCopy() *NamespacedControllerRe return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResourceRequirements) DeepCopyInto(out *ResourceRequirements) { + *out = *in + if in.Limits != nil { + in, out := &in.Limits, &out.Limits + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + if in.Requests != nil { + in, out := &in.Requests, &out.Requests + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResourceRequirements. +func (in *ResourceRequirements) DeepCopy() *ResourceRequirements { + if in == nil { + return nil + } + out := new(ResourceRequirements) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ValidatingWebhookConfigurationCustomization) DeepCopyInto(out *ValidatingWebhookConfigurationCustomization) { *out = *in diff --git a/operator/pkg/apis/core/customize/v1beta1/controllerresource_types.go b/operator/pkg/apis/core/customize/v1beta1/controllerresource_types.go index 90e3419b6c..276010e4e9 100644 --- a/operator/pkg/apis/core/customize/v1beta1/controllerresource_types.go +++ b/operator/pkg/apis/core/customize/v1beta1/controllerresource_types.go @@ -55,7 +55,24 @@ type ContainerResourceSpec struct { Name string `json:"name"` // Resources specifies the resource customization of this container. // Required - Resources corev1.ResourceRequirements `json:"resources"` + Resources ResourceRequirements `json:"resources"` +} + +// ResourceRequirements describes the compute resource requirements that +// ContainerResourceSpec can leverage. +// It is a local copy of ResourceRequirements in k8s.io/api/core/v1 containing +// a subset of its fields. +type ResourceRequirements struct { + // Limits describes the maximum amount of compute resources allowed. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Limits corev1.ResourceList `json:"limits,omitempty" protobuf:"bytes,1,rep,name=limits,casttype=ResourceList,castkey=ResourceName"` + // Requests describes the minimum amount of compute resources required. + // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + // otherwise to an implementation-defined value. Requests cannot exceed Limits. + // More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + // +optional + Requests corev1.ResourceList `json:"requests,omitempty" protobuf:"bytes,2,rep,name=requests,casttype=ResourceList,castkey=ResourceName"` } // ControllerResourceStatus defines the observed state of ControllerResource. diff --git a/operator/pkg/apis/core/customize/v1beta1/zz_generated.deepcopy.go b/operator/pkg/apis/core/customize/v1beta1/zz_generated.deepcopy.go index 7c6db418fc..ba00d517c8 100644 --- a/operator/pkg/apis/core/customize/v1beta1/zz_generated.deepcopy.go +++ b/operator/pkg/apis/core/customize/v1beta1/zz_generated.deepcopy.go @@ -6,6 +6,7 @@ package v1beta1 import ( + "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -283,6 +284,35 @@ func (in *NamespacedControllerResourceStatus) DeepCopy() *NamespacedControllerRe return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResourceRequirements) DeepCopyInto(out *ResourceRequirements) { + *out = *in + if in.Limits != nil { + in, out := &in.Limits, &out.Limits + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + if in.Requests != nil { + in, out := &in.Requests, &out.Requests + *out = make(v1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResourceRequirements. +func (in *ResourceRequirements) DeepCopy() *ResourceRequirements { + if in == nil { + return nil + } + out := new(ResourceRequirements) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ValidatingWebhookConfigurationCustomization) DeepCopyInto(out *ValidatingWebhookConfigurationCustomization) { *out = *in diff --git a/operator/pkg/controllers/configconnector/configconnector_controller_test.go b/operator/pkg/controllers/configconnector/configconnector_controller_test.go index 84f32ef5c7..a3ab476500 100644 --- a/operator/pkg/controllers/configconnector/configconnector_controller_test.go +++ b/operator/pkg/controllers/configconnector/configconnector_controller_test.go @@ -934,7 +934,7 @@ func TestConfigConnectorControllerWatchCustomizationCR(t *testing.T) { Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "webhook", - Resources: v1.ResourceRequirements{}, + Resources: customizev1beta1.ResourceRequirements{}, }, }, }, diff --git a/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller_test.go b/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller_test.go index 8f03ce2a2d..51a22b9804 100644 --- a/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller_test.go +++ b/operator/pkg/controllers/configconnectorcontext/configconnectorcontext_controller_test.go @@ -663,7 +663,7 @@ func TestConfigConnectorContextControllerWatchMultipleCustomizationCR(t *testing Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: v1.ResourceRequirements{}, + Resources: customizev1beta1.ResourceRequirements{}, }, }, }, @@ -677,7 +677,7 @@ func TestConfigConnectorContextControllerWatchMultipleCustomizationCR(t *testing Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: v1.ResourceRequirements{}, + Resources: customizev1beta1.ResourceRequirements{}, }, }, }, diff --git a/operator/pkg/controllers/util_test.go b/operator/pkg/controllers/util_test.go index fbc06d6d49..e1b85b472e 100644 --- a/operator/pkg/controllers/util_test.go +++ b/operator/pkg/controllers/util_test.go @@ -17,6 +17,8 @@ package controllers import ( "testing" + customizev1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/operator/pkg/apis/core/customize/v1beta1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" ) @@ -24,13 +26,13 @@ import ( func TestValidateContainerResourceCustomizationValues(t *testing.T) { tests := []struct { name string - containerResourceCustomization v1.ResourceRequirements + containerResourceCustomization customizev1beta1.ResourceRequirements containerInManifest map[string]interface{} wantErr string }{ { name: "valid customization - both limit and request are specified", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("512Mi"), @@ -43,7 +45,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "valid customization - only request is specified", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Requests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("200m"), v1.ResourceMemory: resource.MustParse("256Mi"), @@ -60,7 +62,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "valid customization - only limit is specified", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("512Mi"), @@ -77,7 +79,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "valid customization - only request is specified, limit is not found in manifest", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Requests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("200m"), v1.ResourceMemory: resource.MustParse("256Mi"), @@ -87,7 +89,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "valid customization - only limit is specified, request is not found in manifest", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("512Mi"), @@ -97,7 +99,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - memory limit is less than memory request", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("256Mi"), @@ -111,7 +113,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - memory limit is less than default memory request in manifest", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("256Mi"), @@ -129,7 +131,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - the default memory limit in manifest is less than memory request", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Requests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("200m"), v1.ResourceMemory: resource.MustParse("1Gi"), @@ -147,7 +149,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - cpu limit is less than cpu request", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("200m"), v1.ResourceMemory: resource.MustParse("512Mi"), @@ -161,7 +163,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - cpu limit is less than default cpu request in manifest", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Limits: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("100m"), v1.ResourceMemory: resource.MustParse("512Mi"), @@ -179,7 +181,7 @@ func TestValidateContainerResourceCustomizationValues(t *testing.T) { }, { name: "invalid customization - the default cpu limit in manifest is less than cpu request", - containerResourceCustomization: v1.ResourceRequirements{ + containerResourceCustomization: customizev1beta1.ResourceRequirements{ Requests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("400m"), v1.ResourceMemory: resource.MustParse("256Mi"), diff --git a/operator/pkg/controllers/utils.go b/operator/pkg/controllers/utils.go index 15bf4e6183..c83dcc7d39 100644 --- a/operator/pkg/controllers/utils.go +++ b/operator/pkg/controllers/utils.go @@ -23,7 +23,6 @@ import ( corev1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/operator/pkg/apis/core/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/operator/pkg/k8s" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -165,8 +164,8 @@ func ApplyContainerResourceCustomization(isNamespaced bool, m *manifest.Objects, if err := checkForDuplicateContainers(containers); err != nil { return err } - cMap := make(map[string]corev1.ResourceRequirements, len(containers)) // cMap is a map of container name to its corresponding resource customization. - cMapApplied := make(map[string]bool) // cMapApplied is a map of container name to a boolean indicating whether the customization for this container is applied. + cMap := make(map[string]customizev1beta1.ResourceRequirements, len(containers)) // cMap is a map of container name to its corresponding resource customization. + cMapApplied := make(map[string]bool) // cMapApplied is a map of container name to a boolean indicating whether the customization for this container is applied. for _, c := range containers { cMap[c.Name] = c.Resources cMapApplied[c.Name] = false @@ -233,7 +232,7 @@ func ApplyContainerResourceCustomization(isNamespaced bool, m *manifest.Objects, } // customizeContainerResourcesFn returns a function to customize container resources. -func customizeContainerResourcesFn(cMap map[string]corev1.ResourceRequirements, cMapApplied map[string]bool) func(container map[string]interface{}) error { +func customizeContainerResourcesFn(cMap map[string]customizev1beta1.ResourceRequirements, cMapApplied map[string]bool) func(container map[string]interface{}) error { return func(container map[string]interface{}) error { name, _, err := unstructured.NestedString(container, "name") if err != nil { @@ -391,7 +390,7 @@ func ListMutatingWebhookConfigurationCustomizations(ctx context.Context, c clien // validateContainerResourceCustomizationValues validates the container resource values specified by the user. // If both `request` and `limit` values are specified by the user, they are checked against each other. // Otherwise, the specified value is checked against the default value found in the KCC manifest object. -func validateContainerResourceCustomizationValues(r corev1.ResourceRequirements, container map[string]interface{}) error { +func validateContainerResourceCustomizationValues(r customizev1beta1.ResourceRequirements, container map[string]interface{}) error { // 1. validate CPU request and limit. cpuLimitIsSet := r.Limits != nil && !r.Limits.Cpu().IsZero() cpuRequestIsSet := r.Requests != nil && !r.Requests.Cpu().IsZero() diff --git a/operator/pkg/test/controller/customization.go b/operator/pkg/test/controller/customization.go index 031a14d7e4..fe8434d018 100644 --- a/operator/pkg/test/controller/customization.go +++ b/operator/pkg/test/controller/customization.go @@ -34,7 +34,7 @@ var ( Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("400m"), }, @@ -55,7 +55,7 @@ var ( Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{}, Requests: corev1.ResourceList{}, }, @@ -72,7 +72,7 @@ var ( Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "webhook", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("512Mi"), }, @@ -93,7 +93,7 @@ var ( Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "webhook", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("512Mi"), }, @@ -114,7 +114,7 @@ var ( Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("400m"), }, @@ -211,7 +211,7 @@ var NamespacedControllerResourceCRWrongNamespace = &customizev1beta1.NamespacedC Containers: []customizev1beta1.ContainerResourceSpec{ { Name: "manager", - Resources: corev1.ResourceRequirements{ + Resources: customizev1beta1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("400m"), },