K8SPXC-927: add serviceLabels and serviceAnnotations for PXC

deploy/cr.yaml

@@ -94,6 +94,10 @@ spec:
 #      iam.amazonaws.com/role: role-arn
 #    labels:
 #      rack: rack-22
+#    serviceAnnotations:
+#      service.beta.kubernetes.io/aws-load-balancer-backend-protocol: http
+#    serviceLabels:
+#      rack: rack-23
 #    readinessProbes:
 #      initialDelaySeconds: 15
 #      timeoutSeconds: 15

pkg/controller/pxc/controller.go

@@ -324,16 +324,17 @@ func (r *ReconcilePerconaXtraDBCluster) Reconcile(ctx context.Context, request r
 		return reconcile.Result{}, errors.Wrap(err, "pxc upgrade error")
 	}
 
-	for _, pxcService := range []*corev1.Service{pxc.NewServicePXC(o), pxc.NewServicePXCUnready(o)} {
-		err := setControllerReference(o, pxcService, r.scheme)
-		if err != nil {
-			return reconcile.Result{}, errors.Wrap(err, "setControllerReference")
-		}
-
-		err = r.createOrUpdateService(o, pxcService, true)
-		if err != nil {
-			return reconcile.Result{}, errors.Wrap(err, "PXC service upgrade error")
-		}
+	saveOldSvcMeta := true
+	if o.CompareVersionWith("1.14.0") >= 0 {
+		saveOldSvcMeta = len(o.Spec.PXC.ServiceLabels) == 0 && len(o.Spec.PXC.ServiceAnnotations) == 0
+	}
+	err = r.createOrUpdateService(o, pxc.NewServicePXC(o), saveOldSvcMeta)
+	if err != nil {
+		return reconcile.Result{}, errors.Wrap(err, "PXC service upgrade error")
+	}
+	err = r.createOrUpdateService(o, pxc.NewServicePXCUnready(o), true)
+	if err != nil {
+		return reconcile.Result{}, errors.Wrap(err, "PXC service upgrade error")
 	}
 
 	if o.Spec.PXC.Expose.Enabled {
@@ -372,19 +373,11 @@ func (r *ReconcilePerconaXtraDBCluster) Reconcile(ctx context.Context, request r
 			return reconcile.Result{}, errors.Wrap(err, "ProxySQL upgrade error")
 		}
 		svc := pxc.NewServiceProxySQL(o)
-		err := setControllerReference(o, svc, r.scheme)
-		if err != nil {
-			return reconcile.Result{}, errors.Wrapf(err, "%s setControllerReference", svc.Name)
-		}
 		err = r.createOrUpdateService(o, svc, len(o.Spec.ProxySQL.ServiceLabels) == 0 && len(o.Spec.ProxySQL.ServiceAnnotations) == 0)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrapf(err, "%s upgrade error", svc.Name)
 		}
 		svc = pxc.NewServiceProxySQLUnready(o)
-		err = setControllerReference(o, svc, r.scheme)
-		if err != nil {
-			return reconcile.Result{}, errors.Wrapf(err, "%s setControllerReference", svc.Name)
-		}
 		err = r.createOrUpdateService(o, svc, true)
 		if err != nil {
 			return reconcile.Result{}, errors.Wrapf(err, "%s upgrade error", svc.Name)
@@ -467,21 +460,13 @@ func (r *ReconcilePerconaXtraDBCluster) reconcileHAProxy(ctx context.Context, cr
 		return errors.Wrap(err, "HAProxy upgrade error")
 	}
 	svc := pxc.NewServiceHAProxy(cr)
-	err := setControllerReference(cr, svc, r.scheme)
-	if err != nil {
-		return errors.Wrapf(err, "%s setControllerReference", svc.Name)
-	}
 	podSpec := cr.Spec.HAProxy.PodSpec
-	err = r.createOrUpdateService(cr, svc, len(podSpec.ServiceLabels) == 0 && len(podSpec.ServiceAnnotations) == 0)
+	err := r.createOrUpdateService(cr, svc, len(podSpec.ServiceLabels) == 0 && len(podSpec.ServiceAnnotations) == 0)
 	if err != nil {
 		return errors.Wrapf(err, "%s upgrade error", svc.Name)
 	}
 	if cr.HAProxyReplicasServiceEnabled() {
 		svc := pxc.NewServiceHAProxyReplicas(cr)
-		err := setControllerReference(cr, svc, r.scheme)
-		if err != nil {
-			return errors.Wrapf(err, "%s setControllerReference", svc.Name)
-		}
 		err = r.createOrUpdateService(cr, svc, len(podSpec.ReplicasServiceLabels) == 0 && len(podSpec.ReplicasServiceAnnotations) == 0)
 		if err != nil {
 			return errors.Wrapf(err, "%s upgrade error", svc.Name)
@@ -1311,15 +1296,27 @@ func (r *ReconcilePerconaXtraDBCluster) createOrUpdate(cr *api.PerconaXtraDBClus
 
 func setIgnoredAnnotationsAndLabels(cr *api.PerconaXtraDBCluster, obj, oldObject client.Object) error {
 	oldAnnotations := oldObject.GetAnnotations()
+	if oldAnnotations == nil {
+		oldAnnotations = make(map[string]string)
+	}
 	annotations := obj.GetAnnotations()
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
 	for _, annotation := range cr.Spec.IgnoreAnnotations {
 		if v, ok := oldAnnotations[annotation]; ok {
 			annotations[annotation] = v
 		}
 	}
 	obj.SetAnnotations(annotations)
 	oldLabels := oldObject.GetLabels()
+	if oldLabels == nil {
+		oldLabels = make(map[string]string)
+	}
 	labels := obj.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string)
+	}
 	for _, label := range cr.Spec.IgnoreLabels {
 		if v, ok := oldLabels[label]; ok {
 			labels[label] = v
@@ -1342,11 +1339,15 @@ func mergeMaps(x, y map[string]string) map[string]string {
 }
 
 func (r *ReconcilePerconaXtraDBCluster) createOrUpdateService(cr *api.PerconaXtraDBCluster, svc *corev1.Service, saveOldMeta bool) error {
+	err := setControllerReference(cr, svc, r.scheme)
+	if err != nil {
+		return errors.Wrap(err, "set controller reference")
+	}
 	if !saveOldMeta && len(cr.Spec.IgnoreAnnotations) == 0 && len(cr.Spec.IgnoreLabels) == 0 {
 		return r.createOrUpdate(cr, svc)
 	}
 	oldSvc := new(corev1.Service)
-	err := r.client.Get(context.TODO(), types.NamespacedName{
+	err = r.client.Get(context.TODO(), types.NamespacedName{
 		Name:      svc.GetName(),
 		Namespace: svc.GetNamespace(),
 	}, oldSvc)

pkg/controller/pxc/replication.go

@@ -53,11 +53,6 @@ func (r *ReconcilePerconaXtraDBCluster) ensurePxcPodServices(cr *api.PerconaXtra
 		svcName := fmt.Sprintf("%s-pxc-%d", cr.Name, i)
 		svc := NewExposedPXCService(svcName, cr)
 
-		err := setControllerReference(cr, svc, r.scheme)
-		if err != nil {
-			return errors.Wrap(err, "failed to set owner to external service")
-		}
-
 		err = r.createOrUpdateService(cr, svc, len(cr.Spec.PXC.Expose.Annotations) == 0)
 		if err != nil {
 			return errors.Wrap(err, "failed to ensure pxc service")

pkg/controller/pxc/service_test.go

@@ -0,0 +1,213 @@
+package pxc
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/percona/percona-xtradb-cluster-operator/pkg/pxc"
+)
+
+var _ = Describe("Service labels and annotations", Ordered, func() {
+	ctx := context.Background()
+	const ns = "svc-ls-an"
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      ns,
+			Namespace: ns,
+		},
+	}
+	crName := ns + "-reconciler"
+	crNamespacedName := types.NamespacedName{Name: crName, Namespace: ns}
+	cr, err := readDefaultCR(crName, ns)
+	It("should read default cr.yaml", func() {
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	BeforeAll(func() {
+		By("Creating the Namespace to perform the tests")
+		err := k8sClient.Create(ctx, namespace)
+		Expect(err).To(Not(HaveOccurred()))
+	})
+
+	AfterAll(func() {
+		// TODO(user): Attention if you improve this code by adding other context test you MUST
+		// be aware of the current delete namespace limitations. More info: https://book.kubebuilder.io/reference/envtest.html#testing-considerations
+		By("Deleting the Namespace to perform the tests")
+		_ = k8sClient.Delete(ctx, namespace)
+	})
+
+	Context("Create Percona XtraDB cluster", func() {
+		It("Should create PerconaXtraDBCluster", func() {
+			Expect(k8sClient.Create(ctx, cr)).Should(Succeed())
+		})
+	})
+
+	It("should reconcile PerconaXtraDBCluster", func() {
+		_, err := reconciler().Reconcile(ctx, reconcile.Request{
+			NamespacedName: crNamespacedName,
+		})
+		Expect(err).To(Succeed())
+	})
+
+	checkLabelsAndAnnotations := func(services []*corev1.Service) {
+		Context("update service labels manually", func() {
+			It("should update service labels manually", func() {
+				for i := range services {
+					svc := new(corev1.Service)
+
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(services[i]), svc)).To(Succeed())
+
+					svc.Labels["manual-label"] = "test"
+					svc.Labels["ignored-label"] = "test"
+					svc.Annotations["manual-annotation"] = "test"
+					svc.Annotations["ignored-annotation"] = "test"
+					Expect(k8sClient.Update(ctx, svc)).To(Succeed())
+				}
+			})
+
+			It("should reconcile PerconaXtraDBCluster", func() {
+				_, err := reconciler().Reconcile(ctx, reconcile.Request{
+					NamespacedName: crNamespacedName,
+				})
+				Expect(err).To(Succeed())
+			})
+			It("should check if manual labels and annotations are still there", func() {
+				for i := range services {
+					svc := new(corev1.Service)
+
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(services[i]), svc)).To(Succeed())
+
+					Expect(svc.Labels["manual-label"]).To(Equal("test"))
+					Expect(svc.Annotations["manual-annotation"]).To(Equal("test"))
+					Expect(svc.Labels["ignored-label"]).To(Equal("test"))
+					Expect(svc.Annotations["ignored-annotation"]).To(Equal("test"))
+				}
+			})
+		})
+
+		Context("set service labels and annotations", func() {
+			It("should update cr", func() {
+				Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed())
+				cr.Spec.IgnoreAnnotations = []string{"ignored-annotation"}
+				cr.Spec.IgnoreLabels = []string{"ignored-label"}
+				cr.Spec.PXC.ServiceLabels = map[string]string{"cr-label": "test"}
+				cr.Spec.PXC.ServiceAnnotations = map[string]string{"cr-annotation": "test"}
+				cr.Spec.HAProxy.ServiceLabels = map[string]string{"cr-label": "test"}
+				cr.Spec.HAProxy.ServiceAnnotations = map[string]string{"cr-annotation": "test"}
+				cr.Spec.HAProxy.ReplicasServiceLabels = map[string]string{"cr-label": "test"}
+				cr.Spec.HAProxy.ReplicasServiceAnnotations = map[string]string{"cr-annotation": "test"}
+				cr.Spec.ProxySQL.ServiceLabels = map[string]string{"cr-label": "test"}
+				cr.Spec.ProxySQL.ServiceAnnotations = map[string]string{"cr-annotation": "test"}
+				Expect(k8sClient.Update(ctx, cr)).Should(Succeed())
+			})
+			It("should reconcile PerconaXtraDBCluster", func() {
+				_, err := reconciler().Reconcile(ctx, reconcile.Request{
+					NamespacedName: crNamespacedName,
+				})
+				Expect(err).To(Succeed())
+			})
+			It("check labels and annotations", func() {
+				for i := range services {
+					svc := new(corev1.Service)
+
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(services[i]), svc)).To(Succeed())
+
+					Expect(svc.Labels["manual-label"]).To(Equal(""))
+					Expect(svc.Annotations["manual-annotation"]).To(Equal(""))
+					Expect(svc.Labels["ignored-label"]).To(Equal("test"))
+					Expect(svc.Annotations["ignored-annotation"]).To(Equal("test"))
+					Expect(svc.Labels["cr-label"]).To(Equal("test"))
+					Expect(svc.Annotations["cr-annotation"]).To(Equal("test"))
+				}
+			})
+		})
+		Context("remove ignored labels and annotations", func() {
+			It("should update cr", func() {
+				Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed())
+				cr.Spec.IgnoreAnnotations = []string{}
+				cr.Spec.IgnoreLabels = []string{}
+				Expect(k8sClient.Update(ctx, cr)).Should(Succeed())
+			})
+			It("should reconcile PerconaXtraDBCluster", func() {
+				_, err := reconciler().Reconcile(ctx, reconcile.Request{
+					NamespacedName: crNamespacedName,
+				})
+				Expect(err).To(Succeed())
+			})
+			It("should check if there are no ignored labels and annotations", func() {
+				for i := range services {
+					svc := new(corev1.Service)
+
+					Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(services[i]), svc)).To(Succeed())
+
+					Expect(svc.Labels["ignored-label"]).To(Equal(""))
+					Expect(svc.Annotations["ignored-annotation"]).To(Equal(""))
+					Expect(svc.Labels["cr-label"]).To(Equal("test"))
+					Expect(svc.Annotations["cr-annotation"]).To(Equal("test"))
+				}
+			})
+		})
+	}
+
+	services := []*corev1.Service{
+		pxc.NewServicePXC(cr),
+		pxc.NewServiceHAProxy(cr),
+		pxc.NewServiceHAProxyReplicas(cr),
+	}
+
+	Context("check haproxy cluster", func() {
+		checkLabelsAndAnnotations(services)
+	})
+
+	It("should delete services", func() {
+		for _, svc := range services {
+			Expect(k8sClient.Delete(ctx, svc)).To(Succeed())
+		}
+	})
+
+	It("should switch to ProxySQL and remove serviceLabels, serviceAnnotations", func() {
+		haproxySts := &appsv1.StatefulSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      cr.Name + "-haproxy",
+				Namespace: cr.Namespace,
+			},
+		}
+		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(haproxySts), haproxySts)).To(Succeed())
+		Expect(k8sClient.Delete(ctx, haproxySts)).To(Succeed())
+
+		Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed())
+		cr.Spec.HAProxy.Enabled = false
+		cr.Spec.ProxySQL.Enabled = true
+
+		cr.Spec.PXC.ServiceLabels = nil
+		cr.Spec.PXC.ServiceAnnotations = nil
+		cr.Spec.HAProxy.ServiceLabels = nil
+		cr.Spec.HAProxy.ServiceAnnotations = nil
+		cr.Spec.HAProxy.ReplicasServiceLabels = nil
+		cr.Spec.HAProxy.ReplicasServiceAnnotations = nil
+		cr.Spec.ProxySQL.ServiceLabels = nil
+		cr.Spec.ProxySQL.ServiceAnnotations = nil
+		Expect(k8sClient.Update(ctx, cr)).To(Succeed())
+	})
+	It("should reconcile PerconaXtraDBCluster", func() {
+		_, err := reconciler().Reconcile(ctx, reconcile.Request{
+			NamespacedName: crNamespacedName,
+		})
+		Expect(err).To(Succeed())
+	})
+
+	Context("check proxysql cluster", func() {
+		checkLabelsAndAnnotations([]*corev1.Service{
+			pxc.NewServicePXC(cr),
+			pxc.NewServiceProxySQL(cr),
+		})
+	})
+})