fix: fixed defaults and validations for operator version

Signed-off-by: Aylei <[email protected]>
matrixorigin · Nov 13, 2024 · 8fd82c1 · 8fd82c1
1 parent 2b220de
commit 8fd82c1
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 16 deletions.
diff --git a/api/core/v1alpha1/operator_version.go b/api/core/v1alpha1/operator_version.go
@@ -14,7 +14,9 @@
 
 package v1alpha1
 
-import "github.com/blang/semver/v4"
+import (
+	"github.com/blang/semver/v4"
+)
 
 type Gate string
 

diff --git a/pkg/controllers/cnset/controller.go b/pkg/controllers/cnset/controller.go
@@ -66,6 +66,7 @@ func (c *Actor) with(cs *kruisev1alpha1.CloneSet) *WithResources {
 func (c *Actor) Observe(ctx *recon.Context[*v1alpha1.CNSet]) (recon.Action[*v1alpha1.CNSet], error) {
 	cn := ctx.Obj
 
+	ctx.Log.Info("observe cnset", "name", cn.Name, "operatorVersion", cn.Spec.GetOperatorVersion())
 	cs := &kruisev1alpha1.CloneSet{}
 	err, foundCs := util.IsFound(ctx.Get(client.ObjectKey{Namespace: cn.Namespace, Name: setName(cn)}, cs))
 	if err != nil {

diff --git a/pkg/webhook/cnset_webhook.go b/pkg/webhook/cnset_webhook.go
@@ -54,7 +54,7 @@ type cnSetDefaulter struct{}
 
 var _ webhook.CustomDefaulter = &cnSetDefaulter{}
 
-func (c *cnSetDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (c *cnSetDefaulter) Default(_ context.Context, obj runtime.Object) error {
 	cnSet, ok := obj.(*v1alpha1.CNSet)
 	if !ok {
 		return unexpectedKindError("CNSet", obj)
@@ -64,7 +64,7 @@ func (c *cnSetDefaulter) Default(ctx context.Context, obj runtime.Object) error
 	if cnSet.Spec.Role == "" {
 		cnSet.Spec.Role = v1alpha1.CNRoleTP
 	}
-	return setDefaultOperatorVersion(ctx, &cnSet.Spec.PodSet)
+	return nil
 }
 
 func (c *cnSetDefaulter) DefaultSpec(spec *v1alpha1.CNSetSpec) {
@@ -118,6 +118,7 @@ func (c *cnSetValidator) ValidateCreate(_ context.Context, obj runtime.Object) (
 }
 
 func (c *cnSetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
+	var errs field.ErrorList
 	warnings, err = c.ValidateCreate(ctx, newObj)
 	if err != nil {
 		return warnings, err
@@ -130,13 +131,8 @@ func (c *cnSetValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runt
 	if !ok {
 		return nil, unexpectedKindError("CNSet", newObj)
 	}
-	if _, ok := oldCN.Spec.PodSet.GetSemVer(); ok {
-		// if the old CNSet has a semantic version, then we need to make sure the new one is compatible
-		if _, ok := newCN.Spec.PodSet.GetSemVer(); !ok {
-			return nil, field.Invalid(field.NewPath("spec").Child("podSet").Child("semanticVersion"), newCN.Spec.PodSet.SemanticVersion, "new version must also be semantic")
-		}
-	}
-	return warnings, nil
+	errs = append(errs, validatePodSetUpdate(&oldCN.Spec.PodSet, &newCN.Spec.PodSet, field.NewPath("spec"))...)
+	return nil, invalidOrNil(errs, newCN)
 }
 
 func (c *cnSetValidator) ValidateDelete(_ context.Context, _ runtime.Object) (warnings admission.Warnings, err error) {

diff --git a/pkg/webhook/common.go b/pkg/webhook/common.go
@@ -283,3 +283,17 @@ func validateVolumeClaims(pvcs []core.PersistentVolumeClaim, path *field.Path) f
 	}
 	return errs
 }
+
+func validatePodSetUpdate(oldPodSet, newPodSet *v1alpha1.PodSet, path *field.Path) field.ErrorList {
+	var errs field.ErrorList
+	if _, ok := oldPodSet.GetSemVer(); ok {
+		// if the old CNSet has a semantic version, then we need to make sure the new one is compatible
+		if _, ok := newPodSet.GetSemVer(); !ok {
+			errs = append(errs, field.Invalid(path.Child("semanticVersion"), newPodSet.SemanticVersion, "new version must also be semantic"))
+		}
+	}
+	if newPodSet.GetOperatorVersion().LT(oldPodSet.GetOperatorVersion()) {
+		errs = append(errs, field.Invalid(path.Child("operatorVersion"), newPodSet.OperatorVersion, "new operatorVersion must be greater than or equal to the oldVersion"))
+	}
+	return errs
+}
diff --git a/pkg/webhook/dnset_webhook.go b/pkg/webhook/dnset_webhook.go
@@ -43,13 +43,13 @@ type dnSetDefaulter struct{}
 
 var _ webhook.CustomDefaulter = &dnSetDefaulter{}
 
-func (d *dnSetDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (d *dnSetDefaulter) Default(_ context.Context, obj runtime.Object) error {
 	dnSet, ok := obj.(*v1alpha1.DNSet)
 	if !ok {
 		return unexpectedKindError("DNSet", obj)
 	}
 	d.DefaultSpec(&dnSet.Spec)
-	return setDefaultOperatorVersion(ctx, &dnSet.Spec.PodSet)
+	return nil
 }
 
 func (d *dnSetDefaulter) DefaultSpec(spec *v1alpha1.DNSetSpec) {

diff --git a/pkg/webhook/logset_webhook.go b/pkg/webhook/logset_webhook.go
@@ -61,13 +61,13 @@ type logSetDefaulter struct{}
 
 var _ webhook.CustomDefaulter = &logSetDefaulter{}
 
-func (l *logSetDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (l *logSetDefaulter) Default(_ context.Context, obj runtime.Object) error {
 	logSet, ok := obj.(*v1alpha1.LogSet)
 	if !ok {
 		return unexpectedKindError("LogSet", obj)
 	}
 	l.DefaultSpec(&logSet.Spec)
-	return setDefaultOperatorVersion(ctx, &logSet.Spec.PodSet)
+	return nil
 }
 
 func (l *logSetDefaulter) DefaultSpec(spec *v1alpha1.LogSetSpec) {

diff --git a/pkg/webhook/proxy_webhook.go b/pkg/webhook/proxy_webhook.go
@@ -16,6 +16,7 @@ package webhook
 
 import (
 	"context"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -41,13 +42,13 @@ type proxySetDefaulter struct{}
 
 var _ webhook.CustomDefaulter = &proxySetDefaulter{}
 
-func (p *proxySetDefaulter) Default(ctx context.Context, obj runtime.Object) error {
+func (p *proxySetDefaulter) Default(_ context.Context, obj runtime.Object) error {
 	proxySet, ok := obj.(*v1alpha1.ProxySet)
 	if !ok {
 		return unexpectedKindError("ProxySet", obj)
 	}
 	p.DefaultSpec(&proxySet.Spec)
-	return setDefaultOperatorVersion(ctx, &proxySet.Spec.PodSet)
+	return nil
 }
 
 func (p *proxySetDefaulter) DefaultSpec(spec *v1alpha1.ProxySetSpec) {