From 1f342ffcf6649dd63a6e9451eab01dbdc871e388 Mon Sep 17 00:00:00 2001 From: xiaoqing Date: Thu, 29 Jun 2023 17:36:32 +0800 Subject: [PATCH] Add subresource status for vpa Add status field in subresource on crd yaml and add new ClusterRole system:vpa-actor to patch /status subresource. The `metadata.generation` only increase on vpa spec update. Fix e2e test for patch and create vpa --- vertical-pod-autoscaler/deploy/vpa-rbac.yaml | 26 ++++++++++++++++++- .../deploy/vpa-v1-crd-gen.yaml | 5 +++- vertical-pod-autoscaler/e2e/v1/common.go | 19 +++++++++++++- vertical-pod-autoscaler/e2e/v1beta2/common.go | 19 +++++++++++++- .../pkg/apis/autoscaling.k8s.io/v1/types.go | 1 + .../apis/autoscaling.k8s.io/v1beta2/types.go | 1 + vertical-pod-autoscaler/pkg/utils/vpa/api.go | 6 ++--- 7 files changed, 70 insertions(+), 7 deletions(-) diff --git a/vertical-pod-autoscaler/deploy/vpa-rbac.yaml b/vertical-pod-autoscaler/deploy/vpa-rbac.yaml index 8c81b9a3c72..45147c36b7e 100644 --- a/vertical-pod-autoscaler/deploy/vpa-rbac.yaml +++ b/vertical-pod-autoscaler/deploy/vpa-rbac.yaml @@ -44,7 +44,6 @@ rules: - get - list - watch - - patch - apiGroups: - "autoscaling.k8s.io" resources: @@ -53,6 +52,18 @@ rules: - get - list - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: system:vpa-status-actor +rules: + - apiGroups: + - "autoscaling.k8s.io" + resources: + - verticalpodautoscalers/status + verbs: + - get - patch --- apiVersion: rbac.authorization.k8s.io/v1 @@ -140,6 +151,19 @@ subjects: --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding +metadata: + name: system:vpa-status-actor +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:vpa-status-actor +subjects: + - kind: ServiceAccount + name: vpa-recommender + namespace: kube-system +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding metadata: name: system:vpa-checkpoint-actor roleRef: diff --git a/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml b/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml index 42a89bfb009..2442af6db54 100644 --- a/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml +++ b/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml @@ -513,7 +513,8 @@ spec: type: object served: true storage: true - subresources: {} + subresources: + status: {} - deprecated: true deprecationWarning: autoscaling.k8s.io/v1beta2 API is deprecated name: v1beta2 @@ -748,3 +749,5 @@ spec: type: object served: true storage: false + subresources: + status: {} diff --git a/vertical-pod-autoscaler/e2e/v1/common.go b/vertical-pod-autoscaler/e2e/v1/common.go index c90ec6fc8b2..0695306199d 100644 --- a/vertical-pod-autoscaler/e2e/v1/common.go +++ b/vertical-pod-autoscaler/e2e/v1/common.go @@ -372,6 +372,23 @@ func InstallVPA(f *framework.Framework, vpa *vpa_types.VerticalPodAutoscaler) { vpaClientSet := getVpaClientSet(f) _, err := vpaClientSet.AutoscalingV1().VerticalPodAutoscalers(f.Namespace.Name).Create(context.TODO(), vpa, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred(), "unexpected error creating VPA") + // apiserver ignore status in vpa create, so need to update status + if !isStatusEmpty(&vpa.Status) { + if vpa.Status.Recommendation != nil { + PatchVpaRecommendation(f, vpa, vpa.Status.Recommendation) + } + } +} + +func isStatusEmpty(status *vpa_types.VerticalPodAutoscalerStatus) bool { + if status == nil { + return true + } + + if len(status.Conditions) == 0 && status.Recommendation == nil { + return true + } + return false } // InstallRawVPA installs a VPA object passed in as raw json in the test cluster. @@ -396,7 +413,7 @@ func PatchVpaRecommendation(f *framework.Framework, vpa *vpa_types.VerticalPodAu Value: *newStatus, }}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - _, err = getVpaClientSet(f).AutoscalingV1().VerticalPodAutoscalers(f.Namespace.Name).Patch(context.TODO(), vpa.Name, types.JSONPatchType, bytes, metav1.PatchOptions{}) + _, err = getVpaClientSet(f).AutoscalingV1().VerticalPodAutoscalers(f.Namespace.Name).Patch(context.TODO(), vpa.Name, types.JSONPatchType, bytes, metav1.PatchOptions{}, "status") gomega.Expect(err).NotTo(gomega.HaveOccurred(), "Failed to patch VPA.") } diff --git a/vertical-pod-autoscaler/e2e/v1beta2/common.go b/vertical-pod-autoscaler/e2e/v1beta2/common.go index b4e31bd8a1a..aa358299c9e 100644 --- a/vertical-pod-autoscaler/e2e/v1beta2/common.go +++ b/vertical-pod-autoscaler/e2e/v1beta2/common.go @@ -360,6 +360,23 @@ func InstallVPA(f *framework.Framework, vpa *vpa_types.VerticalPodAutoscaler) { vpaClientSet := getVpaClientSet(f) _, err := vpaClientSet.AutoscalingV1beta2().VerticalPodAutoscalers(f.Namespace.Name).Create(context.TODO(), vpa, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred(), "unexpected error creating VPA") + // apiserver ignore status in vpa create, so need to update status + if !isStatusEmpty(&vpa.Status) { + if vpa.Status.Recommendation != nil { + PatchVpaRecommendation(f, vpa, vpa.Status.Recommendation) + } + } +} + +func isStatusEmpty(status *vpa_types.VerticalPodAutoscalerStatus) bool { + if status == nil { + return true + } + + if len(status.Conditions) == 0 && status.Recommendation == nil { + return true + } + return false } // InstallRawVPA installs a VPA object passed in as raw json in the test cluster. @@ -384,7 +401,7 @@ func PatchVpaRecommendation(f *framework.Framework, vpa *vpa_types.VerticalPodAu Value: *newStatus, }}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - _, err = getVpaClientSet(f).AutoscalingV1beta2().VerticalPodAutoscalers(f.Namespace.Name).Patch(context.TODO(), vpa.Name, types.JSONPatchType, bytes, metav1.PatchOptions{}) + _, err = getVpaClientSet(f).AutoscalingV1beta2().VerticalPodAutoscalers(f.Namespace.Name).Patch(context.TODO(), vpa.Name, types.JSONPatchType, bytes, metav1.PatchOptions{}, "status") gomega.Expect(err).NotTo(gomega.HaveOccurred(), "Failed to patch VPA.") } diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go index aff68d6fe40..0bc2e60f880 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go @@ -40,6 +40,7 @@ type VerticalPodAutoscalerList struct { // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:storageversion // +kubebuilder:resource:shortName=vpa +// +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Mode",type="string",JSONPath=".spec.updatePolicy.updateMode" // +kubebuilder:printcolumn:name="CPU",type="string",JSONPath=".status.recommendation.containerRecommendations[0].target.cpu" // +kubebuilder:printcolumn:name="Mem",type="string",JSONPath=".status.recommendation.containerRecommendations[0].target.memory" diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go index de4c0843a57..d2fca95a6df 100644 --- a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go +++ b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go @@ -39,6 +39,7 @@ type VerticalPodAutoscalerList struct { // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:resource:shortName=vpa +// +kubebuilder:subresource:status // +k8s:prerelease-lifecycle-gen=true // VerticalPodAutoscaler is the configuration for a vertical pod diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/api.go b/vertical-pod-autoscaler/pkg/utils/vpa/api.go index 7fa869390be..891c0e0bb3d 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/api.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/api.go @@ -49,14 +49,14 @@ type patchRecord struct { Value interface{} `json:"value"` } -func patchVpa(vpaClient vpa_api.VerticalPodAutoscalerInterface, vpaName string, patches []patchRecord) (result *vpa_types.VerticalPodAutoscaler, err error) { +func patchVpaStatus(vpaClient vpa_api.VerticalPodAutoscalerInterface, vpaName string, patches []patchRecord) (result *vpa_types.VerticalPodAutoscaler, err error) { bytes, err := json.Marshal(patches) if err != nil { klog.Errorf("Cannot marshal VPA status patches %+v. Reason: %+v", patches, err) return } - return vpaClient.Patch(context.TODO(), vpaName, types.JSONPatchType, bytes, meta.PatchOptions{}) + return vpaClient.Patch(context.TODO(), vpaName, types.JSONPatchType, bytes, meta.PatchOptions{}, "status") } // UpdateVpaStatusIfNeeded updates the status field of the VPA API object. @@ -69,7 +69,7 @@ func UpdateVpaStatusIfNeeded(vpaClient vpa_api.VerticalPodAutoscalerInterface, v }} if !apiequality.Semantic.DeepEqual(*oldStatus, *newStatus) { - return patchVpa(vpaClient, vpaName, patches) + return patchVpaStatus(vpaClient, vpaName, patches) } return nil, nil }