Copy values to replicaServiceTemplate in conversion webhook

api/v1beta1/conversion.go

@@ -5,13 +5,15 @@ import (
 	"unsafe"
 
 	"github.com/cybozu-go/moco/api/v1beta2"
+	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apiconversion "k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
 )
 
 const (
+	SpecPrimaryServiceTemplateAnnotation = "mysqlcluster.v1beta2.moco.cybozu.com/spec.primaryServiceTemplate"
 	SpecReplicaServiceTemplateAnnotation = "mysqlcluster.v1beta2.moco.cybozu.com/spec.replicaServiceTemplate"
 )
 
@@ -51,7 +53,7 @@ func Convert__MySQLCluster_To_v1beta2_MySQLCluster(in *MySQLCluster, out *v1beta
 		return err
 	}
 
-	if _, err := unmarshalReplicaServiceTemplate(in, out); err != nil {
+	if err := unmarshalServiceTemplate(in, out); err != nil {
 		return err
 	}
 
@@ -64,7 +66,7 @@ func Convert_v1beta2_MySQLCluster_To__MySQLCluster(in *v1beta2.MySQLCluster, out
 		return err
 	}
 
-	if err := marshalReplicaServiceTemplate(&in.Spec, out); err != nil {
+	if err := marshalServiceTemplate(&in.Spec, out); err != nil {
 		return err
 	}
 
@@ -76,8 +78,6 @@ func Convert__MySQLClusterSpec_To_v1beta2_MySQLClusterSpec(in *MySQLClusterSpec,
 		return err
 	}
 
-	out.PrimaryServiceTemplate = (*v1beta2.ServiceTemplate)(unsafe.Pointer(in.ServiceTemplate))
-
 	return nil
 }
 
@@ -91,55 +91,96 @@ func Convert_v1beta2_MySQLClusterSpec_To__MySQLClusterSpec(in *v1beta2.MySQLClus
 	return nil
 }
 
-// marshalReplicaServiceTemplate stores the service template as json data in the destination object annotations.
-func marshalReplicaServiceTemplate(spec *v1beta2.MySQLClusterSpec, dst metav1.Object) error {
-	if spec.ReplicaServiceTemplate == nil {
+// marshalServiceTemplate stores the service template as json data in the destination object annotations.
+func marshalServiceTemplate(spec *v1beta2.MySQLClusterSpec, dst metav1.Object) error {
+	if spec.PrimaryServiceTemplate == nil && spec.ReplicaServiceTemplate == nil {
 		return nil
 	}
 
-	u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(spec.ReplicaServiceTemplate)
-	if err != nil {
-		return err
-	}
-
-	data, err := json.Marshal(u)
-	if err != nil {
-		return err
+	if equality.Semantic.DeepEqual(spec.PrimaryServiceTemplate, spec.ReplicaServiceTemplate) {
+		return nil
 	}
 
 	annotations := dst.GetAnnotations()
 	if annotations == nil {
 		annotations = map[string]string{}
 	}
 
-	annotations[SpecReplicaServiceTemplateAnnotation] = string(data)
-	dst.SetAnnotations(annotations)
+	if spec.PrimaryServiceTemplate != nil {
+		u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(spec.PrimaryServiceTemplate)
+		if err != nil {
+			return err
+		}
 
-	return nil
-}
+		data, err := json.Marshal(u)
+		if err != nil {
+			return err
+		}
 
-// unmarshalReplicaServiceTemplate tries to retrieve the data from the annotation and unmarshal it into the service template passed as input.
-func unmarshalReplicaServiceTemplate(src metav1.Object, dst *v1beta2.MySQLCluster) (bool, error) {
-	data, ok := src.GetAnnotations()[SpecReplicaServiceTemplateAnnotation]
-	if !ok {
-		return false, nil
+		annotations[SpecPrimaryServiceTemplateAnnotation] = string(data)
 	}
 
-	var s *v1beta2.ServiceTemplate
+	if spec.ReplicaServiceTemplate != nil {
+		u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(spec.ReplicaServiceTemplate)
+		if err != nil {
+			return err
+		}
+
+		data, err := json.Marshal(u)
+		if err != nil {
+			return err
+		}
 
-	if err := json.Unmarshal([]byte(data), &s); err != nil {
-		return false, err
+		annotations[SpecReplicaServiceTemplateAnnotation] = string(data)
 	}
 
-	dst.Spec.ReplicaServiceTemplate = s
+	dst.SetAnnotations(annotations)
 
+	return nil
+}
+
+// unmarshalServiceTemplate tries to retrieve the data from the annotation and unmarshal it into the service template passed as input.
+func unmarshalServiceTemplate(src *MySQLCluster, dst *v1beta2.MySQLCluster) error {
 	dstAnnotation := dst.GetAnnotations()
 
-	delete(dstAnnotation, SpecReplicaServiceTemplateAnnotation)
+	if hasServiceTemplateAnnotation(src) {
+		if primary, ok := src.GetAnnotations()[SpecPrimaryServiceTemplateAnnotation]; ok {
+			var s *v1beta2.ServiceTemplate
+			if err := json.Unmarshal([]byte(primary), &s); err != nil {
+				return err
+			}
+
+			dst.Spec.PrimaryServiceTemplate = s
+			delete(dstAnnotation, SpecPrimaryServiceTemplateAnnotation)
+		}
+
+		if replica, ok := src.GetAnnotations()[SpecReplicaServiceTemplateAnnotation]; ok {
+			var s *v1beta2.ServiceTemplate
+			if err := json.Unmarshal([]byte(replica), &s); err != nil {
+				return err
+			}
+
+			dst.Spec.ReplicaServiceTemplate = s
+			delete(dstAnnotation, SpecReplicaServiceTemplateAnnotation)
+		}
+	} else {
+		// If the annotation does not exist, copy the same value to primary and replica.
+		serviceTemplate := (*v1beta2.ServiceTemplate)(unsafe.Pointer(src.Spec.ServiceTemplate))
+		dst.Spec.PrimaryServiceTemplate = serviceTemplate.DeepCopy()
+		dst.Spec.ReplicaServiceTemplate = serviceTemplate.DeepCopy()
+	}
 
 	if len(dstAnnotation) == 0 {
 		dst.SetAnnotations(nil)
 	}
 
-	return true, nil
+	return nil
+}
+
+// hasServiceTemplateAnnotation checks if the given object has the service template annotation.
+func hasServiceTemplateAnnotation(obj metav1.Object) bool {
+	_, primaryFound := obj.GetAnnotations()[SpecPrimaryServiceTemplateAnnotation]
+	_, replicaFound := obj.GetAnnotations()[SpecReplicaServiceTemplateAnnotation]
+
+	return primaryFound || replicaFound
 }

api/v1beta1/conversion_test.go

@@ -10,51 +10,110 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/apitesting/roundtrip"
 	"k8s.io/apimachinery/pkg/runtime"
+	corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
 )
 
 func TestCompatibility(t *testing.T) {
-	scheme := runtime.NewScheme()
-	_ = AddToScheme(scheme)
-	_ = mocov1beta2.AddToScheme(scheme)
+	t.Run("MySQLCluster v1beta1 => v1beta2 => v1beta1", func(t *testing.T) {
+		t.Parallel()
 
-	// Suppress typed nil.
-	fn := func(l **corev1.ResourceList, c fuzz.Continue) {
-		if l != nil && *l == nil || **l == nil {
-			*l = nil
+		scheme := newScheme(t)
+		f := newCompatibilityTestFuzzer(t, scheme)
+
+		for i := 0; i < 10000; i++ {
+			var v1beta1Cluster1, v1beta1Cluster2 MySQLCluster
+			var v1beta2Cluster mocov1beta2.MySQLCluster
+			f.Fuzz(&v1beta1Cluster1)
+
+			var tmp1, tmp2 mocov1beta2.MySQLCluster
+
+			if err := scheme.Convert(v1beta1Cluster1.DeepCopy(), &tmp1, nil); err != nil {
+				t.Fatal(err)
+			}
+			if err := scheme.Convert(&tmp1, &v1beta2Cluster, nil); err != nil {
+				t.Fatal(err)
+			}
+			if err := scheme.Convert(&v1beta2Cluster, &tmp2, nil); err != nil {
+				t.Fatal(err)
+			}
+			if err := scheme.Convert(&tmp2, &v1beta1Cluster2, nil); err != nil {
+				t.Fatal(err)
+			}
+
+			if diff := cmp.Diff(v1beta1Cluster1, v1beta1Cluster2, cmpopts.EquateEmpty()); diff != "" {
+				t.Fatalf("compatibility error case #%d (-want +got):\n%s", i, diff)
+			}
 		}
-	}
+	})
 
-	f := roundtrip.CompatibilityTestFuzzer(scheme, []interface{}{fn})
-	f.NilChance(0.5).NumElements(0, 3)
+	t.Run("MySQLCluster v1beta2 => v1beta1 => v1beta2", func(t *testing.T) {
+		t.Parallel()
+
+		scheme := newScheme(t)
+		f := newCompatibilityTestFuzzer(t, scheme)
 
-	t.Run("MySQLCluster v1beta1 => v1beta2 => v1beta1", func(t *testing.T) {
 		for i := 0; i < 10000; i++ {
-			var oldCluster1, oldCluster2 MySQLCluster
-			var cluster mocov1beta2.MySQLCluster
-			f.Fuzz(&oldCluster1)
+			var v1beta2Cluster1, v1beta2Cluster2 mocov1beta2.MySQLCluster
+			var v1beta1Cluster MySQLCluster
+			f.Fuzz(&v1beta2Cluster1)
 
-			var tmp1, tmp2 mocov1beta2.MySQLCluster
+			var tmp1, tmp2 MySQLCluster
 
-			if err := scheme.Convert(oldCluster1.DeepCopy(), &tmp1, nil); err != nil {
+			if err := scheme.Convert(v1beta2Cluster1.DeepCopy(), &tmp1, nil); err != nil {
 				t.Fatal(err)
 			}
-			if err := scheme.Convert(&tmp1, &cluster, nil); err != nil {
+			if err := scheme.Convert(&tmp1, &v1beta1Cluster, nil); err != nil {
 				t.Fatal(err)
 			}
-			if err := scheme.Convert(&cluster, &tmp2, nil); err != nil {
+			if err := scheme.Convert(&v1beta1Cluster, &tmp2, nil); err != nil {
 				t.Fatal(err)
 			}
-			if err := scheme.Convert(&tmp2, &oldCluster2, nil); err != nil {
+			if err := scheme.Convert(&tmp2, &v1beta2Cluster2, nil); err != nil {
 				t.Fatal(err)
 			}
 
-			if diff := cmp.Diff(oldCluster1, oldCluster2, cmpopts.EquateEmpty()); diff != "" {
+			if diff := cmp.Diff(v1beta2Cluster1, v1beta2Cluster2, cmpopts.EquateEmpty()); diff != "" {
 				t.Fatalf("compatibility error case #%d (-want +got):\n%s", i, diff)
 			}
 		}
 	})
 
+	t.Run("MySQLCluster v1beta1 => v1beta2 ServiceTemplate will be copied", func(t *testing.T) {
+		t.Parallel()
+
+		scheme := newScheme(t)
+		f := newCompatibilityTestFuzzer(t, scheme)
+
+		var v1beta2Cluster mocov1beta2.MySQLCluster
+		var v1beta1Cluster MySQLCluster
+		f.Fuzz(&v1beta1Cluster)
+
+		v1beta1Cluster.Spec.ServiceTemplate = &ServiceTemplate{
+			ObjectMeta: ObjectMeta{},
+			Spec: (*ServiceSpecApplyConfiguration)(
+				corev1ac.ServiceSpec().
+					WithPorts(corev1ac.ServicePort().
+						WithName("dummy").
+						WithPort(8080),
+					),
+			),
+		}
+
+		if err := scheme.Convert(&v1beta1Cluster, &v1beta2Cluster, nil); err != nil {
+			t.Fatal(err)
+		}
+
+		if diff := cmp.Diff(v1beta2Cluster.Spec.PrimaryServiceTemplate, v1beta2Cluster.Spec.ReplicaServiceTemplate, cmpopts.EquateEmpty()); diff != "" {
+			t.Fatalf("compatibility error case (-want +got):\n%s", diff)
+		}
+	})
+
 	t.Run("BackupPolicy v1beta1 => v1beta2 => v1beta1", func(t *testing.T) {
+		t.Parallel()
+
+		scheme := newScheme(t)
+		f := newCompatibilityTestFuzzer(t, scheme)
+
 		for i := 0; i < 10000; i++ {
 			var oldPolicy1, oldPolicy2 BackupPolicy
 			var policy mocov1beta2.BackupPolicy
@@ -81,3 +140,29 @@ func TestCompatibility(t *testing.T) {
 		}
 	})
 }
+
+func newScheme(t *testing.T) *runtime.Scheme {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	_ = AddToScheme(scheme)
+	_ = mocov1beta2.AddToScheme(scheme)
+
+	return scheme
+}
+
+func newCompatibilityTestFuzzer(t *testing.T, scheme *runtime.Scheme) *fuzz.Fuzzer {
+	t.Helper()
+
+	// Suppress typed nil.
+	fn := func(l **corev1.ResourceList, c fuzz.Continue) {
+		if l != nil && *l == nil || **l == nil {
+			*l = nil
+		}
+	}
+
+	f := roundtrip.CompatibilityTestFuzzer(scheme, []interface{}{fn})
+	f.NilChance(0.5).NumElements(0, 3)
+
+	return f
+}