diff --git a/deploy/cr.yaml b/deploy/cr.yaml index 3e6d093fbc..11c97fb0f7 100644 --- a/deploy/cr.yaml +++ b/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 diff --git a/pkg/controller/pxc/controller.go b/pkg/controller/pxc/controller.go index 07c7d7a0b1..fed25d5493 100644 --- a/pkg/controller/pxc/controller.go +++ b/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,7 +1296,13 @@ 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 @@ -1319,7 +1310,13 @@ func setIgnoredAnnotationsAndLabels(cr *api.PerconaXtraDBCluster, obj, oldObject } 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) diff --git a/pkg/controller/pxc/replication.go b/pkg/controller/pxc/replication.go index 1f91786d8f..2d81904ae6 100644 --- a/pkg/controller/pxc/replication.go +++ b/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") diff --git a/pkg/controller/pxc/service_test.go b/pkg/controller/pxc/service_test.go new file mode 100644 index 0000000000..80460424fa --- /dev/null +++ b/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), + }) + }) +}) diff --git a/pkg/pxc/service.go b/pkg/pxc/service.go index fa69f3b4e3..abfdfef39c 100644 --- a/pkg/pxc/service.go +++ b/pkg/pxc/service.go @@ -48,7 +48,8 @@ func NewServicePXC(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33062, - Name: "mysql-admin"}, + Name: "mysql-admin", + }, ) } @@ -61,10 +62,18 @@ func NewServicePXC(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33060, - Name: "mysqlx"}, + Name: "mysqlx", + }, ) } + if cr.CompareVersionWith("1.14.0") >= 0 { + if cr.Spec.PXC != nil { + obj.Annotations = cr.Spec.PXC.ServiceAnnotations + obj.Labels = fillServiceLabels(obj.Labels, cr.Spec.PXC.ServiceLabels) + } + } + return obj } @@ -106,7 +115,8 @@ func NewServicePXCUnready(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33062, - Name: "mysql-admin"}, + Name: "mysql-admin", + }, ) } @@ -119,7 +129,8 @@ func NewServicePXCUnready(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33060, - Name: "mysqlx"}, + Name: "mysqlx", + }, ) } @@ -128,6 +139,13 @@ func NewServicePXCUnready(cr *api.PerconaXtraDBCluster) *corev1.Service { delete(obj.ObjectMeta.Annotations, "service.alpha.kubernetes.io/tolerate-unready-endpoints") } + if cr.CompareVersionWith("1.14.0") >= 0 { + if cr.Spec.PXC != nil { + obj.Annotations = cr.Spec.PXC.ServiceAnnotations + obj.Labels = fillServiceLabels(obj.Labels, cr.Spec.PXC.ServiceLabels) + } + } + return obj } @@ -173,7 +191,8 @@ func NewServiceProxySQLUnready(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33062, - Name: "mysql-admin"}, + Name: "mysql-admin", + }, ) } @@ -259,7 +278,8 @@ func NewServiceProxySQL(cr *api.PerconaXtraDBCluster) *corev1.Service { obj.Spec.Ports, corev1.ServicePort{ Port: 33062, - Name: "mysql-admin"}, + Name: "mysql-admin", + }, ) } @@ -382,7 +402,8 @@ func NewServiceHAProxyReplicas(cr *api.PerconaXtraDBCluster) *corev1.Service { "app.kubernetes.io/instance": cr.Name, "app.kubernetes.io/component": "haproxy", "app.kubernetes.io/managed-by": "percona-xtradb-cluster-operator", - "app.kubernetes.io/part-of": "percona-xtradb-cluster"} + "app.kubernetes.io/part-of": "percona-xtradb-cluster", + } loadBalancerSourceRanges := []string{} loadBalancerIP := "" if cr.Spec.HAProxy != nil {