From 3498a1fc59211641606e1e5df71d52a46f6f3c0c Mon Sep 17 00:00:00 2001 From: yandongxiao Date: Fri, 15 Dec 2023 11:06:07 +0800 Subject: [PATCH] [BugFix] Remove HPA resource when CN spec is removed Signed-off-by: yandongxiao --- pkg/apis/starrocks/v1/auto_scale.go | 49 +++++++++++++++------- pkg/k8sutils/k8sutils.go | 25 +++-------- pkg/k8sutils/k8sutils_test.go | 58 +------------------------- pkg/subcontrollers/cn/cn_controller.go | 48 ++++++--------------- 4 files changed, 53 insertions(+), 127 deletions(-) diff --git a/pkg/apis/starrocks/v1/auto_scale.go b/pkg/apis/starrocks/v1/auto_scale.go index e69250fa..225a5f59 100644 --- a/pkg/apis/starrocks/v1/auto_scale.go +++ b/pkg/apis/starrocks/v1/auto_scale.go @@ -19,7 +19,10 @@ package v1 import ( "strconv" - "k8s.io/api/autoscaling/v2beta2" + autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2 "k8s.io/api/autoscaling/v2" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" + "sigs.k8s.io/controller-runtime/pkg/client" ) // AutoScalingPolicy defines the auto scale @@ -41,6 +44,22 @@ type AutoScalingPolicy struct { MaxReplicas int32 `json:"maxReplicas"` } +type HPAPolicy struct { + // +optional + // Metrics specifies how to scale based on a single metric + // the struct copy from k8s.io/api/autoscaling/v2beta2/types.go. the redundancy code will hide the restriction about + // HorizontalPodAutoscaler version and kubernetes releases matching issue. + // the splice will have unsafe.Pointer convert, so be careful to edit the struct fields. + Metrics []autoscalingv2beta2.MetricSpec `json:"metrics,omitempty"` + + // +optional + // HorizontalPodAutoscalerBehavior configures the scaling behavior of the target. + // the struct copy from k8s.io/api/autoscaling/v2beta2/types.go. the redundancy code will hide the restriction about + // HorizontalPodAutoscaler version and kubernetes releases matching issue. + // the + Behavior *autoscalingv2beta2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"` +} + type AutoScalerVersion string const ( @@ -77,18 +96,18 @@ func (version AutoScalerVersion) Complete(major, minor string) AutoScalerVersion return AutoScalerV2 } -type HPAPolicy struct { - // +optional - // Metrics specifies how to scale based on a single metric - // the struct copy from k8s.io/api/autoscaling/v2beta2/types.go. the redundancy code will hide the restriction about - // HorizontalPodAutoscaler version and kubernetes releases matching issue. - // the splice will have unsafe.Pointer convert, so be careful to edit the struct fields. - Metrics []v2beta2.MetricSpec `json:"metrics,omitempty"` - - // +optional - // HorizontalPodAutoscalerBehavior configures the scaling behavior of the target. - // the struct copy from k8s.io/api/autoscaling/v2beta2/types.go. the redundancy code will hide the restriction about - // HorizontalPodAutoscaler version and kubernetes releases matching issue. - // the - Behavior *v2beta2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"` +// CreateEmptyHPA create an empty HPA object based on the version information +func (version AutoScalerVersion) CreateEmptyHPA(major, minor string) client.Object { + filledVersion := version.Complete(major, minor) + + var object client.Object + switch filledVersion { + case AutoScalerV1: + object = &autoscalingv1.HorizontalPodAutoscaler{} + case AutoScalerV2: + object = &autoscalingv2.HorizontalPodAutoscaler{} + case AutoScalerV2Beta2: + object = &autoscalingv2beta2.HorizontalPodAutoscaler{} + } + return object } diff --git a/pkg/k8sutils/k8sutils.go b/pkg/k8sutils/k8sutils.go index d0f902f6..75069cbf 100644 --- a/pkg/k8sutils/k8sutils.go +++ b/pkg/k8sutils/k8sutils.go @@ -24,9 +24,6 @@ import ( "github.com/go-logr/logr" appv1 "k8s.io/api/apps/v1" - v1 "k8s.io/api/autoscaling/v1" - v2 "k8s.io/api/autoscaling/v2" - "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -249,31 +246,19 @@ func DeleteConfigMap(ctx context.Context, k8sClient client.Client, namespace, na return k8sClient.Delete(ctx, &cm) } -// DeleteAutoscaler as version type delete response autoscaler. -func DeleteAutoscaler(ctx context.Context, k8sClient client.Client, - namespace, name string, autoscalerVersion srapi.AutoScalerVersion) error { +func DeleteAutoscaler(ctx context.Context, k8sClient client.Client, namespace, name string) error { logger := logr.FromContextOrDiscard(ctx) logger.Info("delete autoscaler from kubernetes", "name", name) - var autoscaler client.Object - switch autoscalerVersion { - case srapi.AutoScalerV1: - autoscaler = &v1.HorizontalPodAutoscaler{} - case srapi.AutoScalerV2: - autoscaler = &v2.HorizontalPodAutoscaler{} - case srapi.AutoScalerV2Beta2: - autoscaler = &v2beta2.HorizontalPodAutoscaler{} - default: - return fmt.Errorf("the autoscaler type %s is not supported", autoscalerVersion) - } - - if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, autoscaler); apierrors.IsNotFound(err) { + var version srapi.AutoScalerVersion + hpaObject := version.CreateEmptyHPA(KUBE_MAJOR_VERSION, KUBE_MINOR_VERSION) + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, hpaObject); apierrors.IsNotFound(err) { return nil } else if err != nil { return err } - return k8sClient.Delete(ctx, autoscaler) + return k8sClient.Delete(ctx, hpaObject) } func PodIsReady(status *corev1.PodStatus) bool { diff --git a/pkg/k8sutils/k8sutils_test.go b/pkg/k8sutils/k8sutils_test.go index b14b949d..03b119dc 100644 --- a/pkg/k8sutils/k8sutils_test.go +++ b/pkg/k8sutils/k8sutils_test.go @@ -18,13 +18,8 @@ import ( "context" "testing" - "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" - v1 "k8s.io/api/autoscaling/v1" - v2 "k8s.io/api/autoscaling/v2" - "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,59 +34,8 @@ func init() { srapi.Register() } +// fake client can not delete a resource when resource version is wrong func Test_DeleteAutoscaler(t *testing.T) { - v1autoscaler := v1.HorizontalPodAutoscaler{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - - v2autoscaler := v2.HorizontalPodAutoscaler{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - } - - v2beta2Autoscaler := v2beta2.HorizontalPodAutoscaler{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - - k8sClient := fake.NewFakeClient(srapi.Scheme, &v1autoscaler, &v2autoscaler, &v2beta2Autoscaler) - // confirm the v1.autoscaler exist. - var cv1autoscaler v1.HorizontalPodAutoscaler - cerr := k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &cv1autoscaler) - require.Equal(t, nil, cerr) - require.Equal(t, "test", cv1autoscaler.Name) - delerr := k8sutils.DeleteAutoscaler(context.Background(), k8sClient, "default", "test", srapi.AutoScalerV1) - require.Equal(t, nil, delerr) - var ev1autoscaler v1.HorizontalPodAutoscaler - geterr := k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &ev1autoscaler) - require.True(t, apierrors.IsNotFound(geterr)) - - var cv2autoscaler v2.HorizontalPodAutoscaler - cerr = k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &cv2autoscaler) - require.Equal(t, nil, cerr) - require.Equal(t, "test", v2autoscaler.Name) - delerr = k8sutils.DeleteAutoscaler(context.Background(), k8sClient, "default", "test", srapi.AutoScalerV2) - require.Equal(t, nil, delerr) - var ev2autoscaler v2.HorizontalPodAutoscaler - geterr = k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &ev2autoscaler) - require.True(t, apierrors.IsNotFound(geterr)) - - var cv2beta2autoscaler v2beta2.HorizontalPodAutoscaler - cerr = k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &cv2beta2autoscaler) - require.Equal(t, nil, cerr) - require.Equal(t, "test", cv2beta2autoscaler.Name) - delerr = k8sutils.DeleteAutoscaler(context.Background(), k8sClient, "default", "test", srapi.AutoScalerV2Beta2) - require.Equal(t, nil, delerr) - var ev2beta2autoscaler v2beta2.HorizontalPodAutoscaler - geterr = k8sClient.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &ev2beta2autoscaler) - require.True(t, apierrors.IsNotFound(geterr)) } func Test_getValueFromConfigmap(t *testing.T) { diff --git a/pkg/subcontrollers/cn/cn_controller.go b/pkg/subcontrollers/cn/cn_controller.go index fc98327d..d9968517 100644 --- a/pkg/subcontrollers/cn/cn_controller.go +++ b/pkg/subcontrollers/cn/cn_controller.go @@ -25,9 +25,6 @@ import ( "github.com/go-logr/logr" appv1 "k8s.io/api/apps/v1" - v1 "k8s.io/api/autoscaling/v1" - v2 "k8s.io/api/autoscaling/v2" - "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -310,67 +307,48 @@ func (cc *CnController) deployAutoScaler(ctx context.Context, object object.Star ScalerPolicy: &policy, } - autoScaler := rutils.BuildHorizontalPodAutoscaler(autoscalerParams, "") - autoScaler.SetAnnotations(make(map[string]string)) - var clientObject client.Object - t := autoscalerParams.AutoscalerType.Complete(k8sutils.KUBE_MAJOR_VERSION, k8sutils.KUBE_MINOR_VERSION) - switch t { - case srapi.AutoScalerV1: - clientObject = &v1.HorizontalPodAutoscaler{} - case srapi.AutoScalerV2: - clientObject = &v2.HorizontalPodAutoscaler{} - case srapi.AutoScalerV2Beta2: - clientObject = &v2beta2.HorizontalPodAutoscaler{} - } + expectHPA := rutils.BuildHorizontalPodAutoscaler(autoscalerParams, "") + expectHPA.SetAnnotations(make(map[string]string)) + + actualHPA := autoscalerParams.AutoscalerType.CreateEmptyHPA(k8sutils.KUBE_MAJOR_VERSION, k8sutils.KUBE_MINOR_VERSION) if err := cc.k8sClient.Get(ctx, types.NamespacedName{ Namespace: autoscalerParams.Namespace, Name: autoscalerParams.Name, }, - clientObject, + actualHPA, ); err != nil { if apierrors.IsNotFound(err) { - return cc.k8sClient.Create(ctx, autoScaler) + return cc.k8sClient.Create(ctx, expectHPA) } return err } var expectHash, actualHash string - expectHash = hash.HashObject(autoScaler) - if v, ok := clientObject.GetAnnotations()[srapi.ComponentResourceHash]; ok { + expectHash = hash.HashObject(expectHPA) + if v, ok := actualHPA.GetAnnotations()[srapi.ComponentResourceHash]; ok { actualHash = v } else { - actualHash = hash.HashObject(clientObject) + actualHash = hash.HashObject(actualHPA) } if expectHash == actualHash { logger.Info("expectHash == actualHash, no need to update HPA resource") return nil } - autoScaler.GetAnnotations()[srapi.ComponentResourceHash] = expectHash - return cc.k8sClient.Update(ctx, autoScaler) + expectHPA.GetAnnotations()[srapi.ComponentResourceHash] = expectHash + return cc.k8sClient.Update(ctx, expectHPA) } // deleteAutoScaler delete the autoscaler. func (cc *CnController) deleteAutoScaler(ctx context.Context, src *srapi.StarRocksCluster) error { logger := logr.FromContextOrDiscard(ctx) - if src.Status.StarRocksCnStatus == nil { - return nil - } - - if src.Status.StarRocksCnStatus.HorizontalScaler.Name == "" { - return nil - } - - autoScalerName := src.Status.StarRocksCnStatus.HorizontalScaler.Name - version := src.Status.StarRocksCnStatus.HorizontalScaler.Version - if err := k8sutils.DeleteAutoscaler(ctx, cc.k8sClient, src.Namespace, autoScalerName, version); err != nil && !apierrors.IsNotFound(err) { + autoScalerName := cc.generateAutoScalerName(src.Name, (*srapi.StarRocksCnSpec)(nil)) + if err := k8sutils.DeleteAutoscaler(ctx, cc.k8sClient, src.Namespace, autoScalerName); err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "delete autoscaler failed") return err } - - src.Status.StarRocksCnStatus.HorizontalScaler = srapi.HorizontalScaler{} return nil }