From 9d907deece337297d6c771ef28b78452ee1d863a Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Tue, 24 Mar 2020 11:38:55 -0700 Subject: [PATCH 01/11] Initial support for custom labels and annotations --- artifacts/flagger/crd.yaml | 36 +++++++++++++++++++ charts/flagger/crds/crd.yaml | 36 +++++++++++++++++++ kustomize/base/flagger/crd.yaml | 36 +++++++++++++++++++ pkg/apis/flagger/v1beta1/canary.go | 18 ++++++++++ .../flagger/v1beta1/zz_generated.deepcopy.go | 33 +++++++++++++++++ pkg/router/kubernetes_default.go | 17 +++++---- 6 files changed, 170 insertions(+), 6 deletions(-) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index a39bb578..0fc6b2ac 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -499,6 +499,42 @@ spec: format: string type: string type: array + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index a39bb578..0fc6b2ac 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -499,6 +499,42 @@ spec: format: string type: string type: array + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index a39bb578..0fc6b2ac 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -499,6 +499,42 @@ spec: format: string type: string type: array + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 25bafe0d..5251fe82 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -168,6 +168,18 @@ type CanaryService struct { // Backends of the generated App Mesh virtual nodes // +optional Backends []string `json:"backends,omitempty"` + + // Apex is metadata to add to the apex service + // +optional + Apex CustomMetadata `json:"apex,omitempty"` + + // Primary is the metadata to add to the primary service + // +optional + Primary CustomMetadata `json:"primary,omitempty"` + + // Canary is the metadata to add to the canary service + // +optional + Canary CustomMetadata `json:"canary,omitempty"` } // CanaryAnalysis is used to describe how the analysis should be done @@ -343,6 +355,12 @@ type CrossNamespaceObjectReference struct { Namespace string `json:"namespace,omitempty"` } +// CustomMetadata holds labels and annotations to set on generated objects. +type CustomMetadata struct { + Labels map[string]string `json:"labels,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` +} + // GetServiceNames returns the apex, primary and canary Kubernetes service names func (c *Canary) GetServiceNames() (apexName, primaryName, canaryName string) { apexName = c.Spec.TargetRef.Name diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index 86fb45e7..c1a7d350 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go @@ -364,6 +364,9 @@ func (in *CanaryService) DeepCopyInto(out *CanaryService) { *out = make([]string, len(*in)) copy(*out, *in) } + in.Apex.DeepCopyInto(&out.Apex) + in.Primary.DeepCopyInto(&out.Primary) + in.Canary.DeepCopyInto(&out.Canary) return } @@ -547,6 +550,36 @@ func (in *CrossNamespaceObjectReference) DeepCopy() *CrossNamespaceObjectReferen return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomMetadata) DeepCopyInto(out *CustomMetadata) { + *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomMetadata. +func (in *CustomMetadata) DeepCopy() *CustomMetadata { + if in == nil { + return nil + } + out := new(CustomMetadata) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MetricTemplate) DeepCopyInto(out *MetricTemplate) { *out = *in diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 2d1c556e..63c006a3 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -34,13 +34,13 @@ func (c *KubernetesDefaultRouter) Initialize(canary *flaggerv1.Canary) error { _, primaryName, canaryName := canary.GetServiceNames() // canary svc - err := c.reconcileService(canary, canaryName, canary.Spec.TargetRef.Name) + err := c.reconcileService(canary, canaryName, canary.Spec.TargetRef.Name, canary.Spec.Service.Canary) if err != nil { return fmt.Errorf("reconcileService failed: %w", err) } // primary svc - err = c.reconcileService(canary, primaryName, fmt.Sprintf("%s-primary", canary.Spec.TargetRef.Name)) + err = c.reconcileService(canary, primaryName, fmt.Sprintf("%s-primary", canary.Spec.TargetRef.Name), canary.Spec.Service.Primary) if err != nil { return fmt.Errorf("reconcileService failed: %w", err) } @@ -53,7 +53,7 @@ func (c *KubernetesDefaultRouter) Reconcile(canary *flaggerv1.Canary) error { apexName, _, _ := canary.GetServiceNames() // main svc - err := c.reconcileService(canary, apexName, fmt.Sprintf("%s-primary", canary.Spec.TargetRef.Name)) + err := c.reconcileService(canary, apexName, fmt.Sprintf("%s-primary", canary.Spec.TargetRef.Name), canary.Spec.Service.Apex) if err != nil { return fmt.Errorf("reconcileService failed: %w", err) } @@ -69,7 +69,7 @@ func (c *KubernetesDefaultRouter) GetRoutes(_ *flaggerv1.Canary) (primaryRoute i return 0, 0, nil } -func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, name string, podSelector string) error { +func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, name string, podSelector string, metadata flaggerv1.CustomMetadata) error { portName := canary.Spec.Service.PortName if portName == "" { portName = "http" @@ -113,6 +113,11 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam svcSpec.Ports = append(svcSpec.Ports, cp) } + metadata.Labels[c.labelSelector] = name + for k, v := range c.annotations { + metadata.Annotations[k] = v + } + // create service if it doesn't exists svc, err := c.kubeClient.CoreV1().Services(canary.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) if errors.IsNotFound(err) { @@ -120,8 +125,8 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: canary.Namespace, - Labels: map[string]string{c.labelSelector: name}, - Annotations: c.annotations, + Labels: metadata.Labels, + Annotations: metadata.Annotations, OwnerReferences: []metav1.OwnerReference{ *metav1.NewControllerRef(canary, schema.GroupVersionKind{ Group: flaggerv1.SchemeGroupVersion.Group, From 5c48430ed27effe7570889ca11dd64697faf665c Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Tue, 24 Mar 2020 16:04:33 -0700 Subject: [PATCH 02/11] Initialize the label and annotation maps if they are nil --- pkg/router/kubernetes_default.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 63c006a3..100f0bf2 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -113,6 +113,13 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam svcSpec.Ports = append(svcSpec.Ports, cp) } + if metadata.Labels == nil { + metadata.Labels = make(map[string]string) + } + if metadata.Annotations == nil { + metadata.Annotations = make(map[string]string) + } + metadata.Labels[c.labelSelector] = name for k, v := range c.annotations { metadata.Annotations[k] = v From 0aee385145bf3eb07013e17b576630197b34fdae Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Fri, 27 Mar 2020 15:38:09 -0700 Subject: [PATCH 03/11] log canary.spec.service for debugging purposes --- pkg/router/kubernetes_default.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 100f0bf2..b34046c3 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -125,6 +125,9 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam metadata.Annotations[k] = v } + c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)). + Debugw(fmt.Sprintf("Creating Service %s", name), "metadata", metadata, "service_configuration", canary.Spec.Service) + // create service if it doesn't exists svc, err := c.kubeClient.CoreV1().Services(canary.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) if errors.IsNotFound(err) { From b7441a7ce72b69e6c0d55f66dc26b8555b950876 Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Mon, 30 Mar 2020 14:57:52 -0700 Subject: [PATCH 04/11] Fix call to reconcileService --- pkg/router/kubernetes_default.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index b34046c3..93410f74 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -219,7 +219,7 @@ func (c *KubernetesDefaultRouter) Finalize(canary *flaggerv1.Canary) error { return fmt.Errorf("service %s update error: %w", clone.Name, err) } } else { - err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name) + err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name, canary.Spec.Service.Apex) if err != nil { return fmt.Errorf("reconcileService failed: %w", err) } From 2657e135b8d97320a836d4abd384af0c39cb0648 Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Tue, 31 Mar 2020 12:33:51 -0700 Subject: [PATCH 05/11] Use pointers for metadata because it is optional and metadata parameter is nil on finalize. in response to PR feedback --- pkg/apis/flagger/v1beta1/canary.go | 6 +++--- .../flagger/v1beta1/zz_generated.deepcopy.go | 18 +++++++++++++++--- pkg/router/kubernetes_default.go | 8 ++++++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 5251fe82..6106d143 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -171,15 +171,15 @@ type CanaryService struct { // Apex is metadata to add to the apex service // +optional - Apex CustomMetadata `json:"apex,omitempty"` + Apex *CustomMetadata `json:"apex,omitempty"` // Primary is the metadata to add to the primary service // +optional - Primary CustomMetadata `json:"primary,omitempty"` + Primary *CustomMetadata `json:"primary,omitempty"` // Canary is the metadata to add to the canary service // +optional - Canary CustomMetadata `json:"canary,omitempty"` + Canary *CustomMetadata `json:"canary,omitempty"` } // CanaryAnalysis is used to describe how the analysis should be done diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index c1a7d350..31ec068e 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go @@ -364,9 +364,21 @@ func (in *CanaryService) DeepCopyInto(out *CanaryService) { *out = make([]string, len(*in)) copy(*out, *in) } - in.Apex.DeepCopyInto(&out.Apex) - in.Primary.DeepCopyInto(&out.Primary) - in.Canary.DeepCopyInto(&out.Canary) + if in.Apex != nil { + in, out := &in.Apex, &out.Apex + *out = new(CustomMetadata) + (*in).DeepCopyInto(*out) + } + if in.Primary != nil { + in, out := &in.Primary, &out.Primary + *out = new(CustomMetadata) + (*in).DeepCopyInto(*out) + } + if in.Canary != nil { + in, out := &in.Canary, &out.Canary + *out = new(CustomMetadata) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 93410f74..0d58e49f 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -69,7 +69,7 @@ func (c *KubernetesDefaultRouter) GetRoutes(_ *flaggerv1.Canary) (primaryRoute i return 0, 0, nil } -func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, name string, podSelector string, metadata flaggerv1.CustomMetadata) error { +func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, name string, podSelector string, metadata *flaggerv1.CustomMetadata) error { portName := canary.Spec.Service.PortName if portName == "" { portName = "http" @@ -113,6 +113,10 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam svcSpec.Ports = append(svcSpec.Ports, cp) } + if metadata == nil { + metadata = &flaggerv1.CustomMetadata{} + } + if metadata.Labels == nil { metadata.Labels = make(map[string]string) } @@ -219,7 +223,7 @@ func (c *KubernetesDefaultRouter) Finalize(canary *flaggerv1.Canary) error { return fmt.Errorf("service %s update error: %w", clone.Name, err) } } else { - err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name, canary.Spec.Service.Apex) + err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name, nil) if err != nil { return fmt.Errorf("reconcileService failed: %w", err) } From c609a90959b0153d5df51f091119111e62446562 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Wed, 1 Apr 2020 21:48:01 +0300 Subject: [PATCH 06/11] Add unit tests for service custom metadata --- pkg/router/kubernetes_default_test.go | 72 ++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/pkg/router/kubernetes_default_test.go b/pkg/router/kubernetes_default_test.go index e43d2797..14f874fe 100644 --- a/pkg/router/kubernetes_default_test.go +++ b/pkg/router/kubernetes_default_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -303,7 +303,7 @@ func TestServiceRouter_Finalize(t *testing.T) { router *KubernetesDefaultRouter callSetupMethods bool shouldError bool - canary *v1beta1.Canary + canary *flaggerv1.Canary shouldMutate bool }{ // Won't reconcile since it is owned and would be garbage collected @@ -347,3 +347,71 @@ func TestServiceRouter_Finalize(t *testing.T) { } } } + +func TestServiceRouter_InitializeMetadata(t *testing.T) { + mocks := newFixture(nil) + router := &KubernetesDefaultRouter{ + kubeClient: mocks.kubeClient, + flaggerClient: mocks.flaggerClient, + logger: mocks.logger, + labelSelector: "app", + } + + metadata := &flaggerv1.CustomMetadata{ + Labels: map[string]string{"test": "test"}, + Annotations: map[string]string{"test": "test"}, + } + + mocks.canary.Spec.Service.Canary = metadata + + err := router.Initialize(mocks.canary) + require.NoError(t, err) + + canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-canary", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "test", canarySvc.Annotations["test"]) + assert.Equal(t, "test", canarySvc.Labels["test"]) + assert.Equal(t, "podinfo-canary", canarySvc.Labels["app"]) + + primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-primary", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, 0, len(primarySvc.Annotations)) + assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"]) +} + +func TestServiceRouter_ReconcileMetadata(t *testing.T) { + mocks := newFixture(nil) + router := &KubernetesDefaultRouter{ + kubeClient: mocks.kubeClient, + flaggerClient: mocks.flaggerClient, + logger: mocks.logger, + labelSelector: "app", + } + + mocks.canary.Spec.Service.Apex = &flaggerv1.CustomMetadata{ + Labels: map[string]string{"test": "test"}, + Annotations: map[string]string{"test": "test"}, + } + + err := router.Initialize(mocks.canary) + require.NoError(t, err) + + err = router.Reconcile(mocks.canary) + require.NoError(t, err) + + apexSvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "test", apexSvc.Annotations["test"]) + assert.Equal(t, "test", apexSvc.Labels["test"]) + assert.Equal(t, "podinfo", apexSvc.Labels["app"]) + + canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-canary", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, 0, len(canarySvc.Annotations)) + assert.Equal(t, "podinfo-canary", canarySvc.Labels["app"]) + + primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-primary", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, 0, len(primarySvc.Annotations)) + assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"]) +} From 37527854d2c13aa1d6eff1903e2523fe99e629bb Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Wed, 1 Apr 2020 22:25:32 +0300 Subject: [PATCH 07/11] Add e2e test for apex service custom metadata --- test/e2e-istio-tests.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/e2e-istio-tests.sh b/test/e2e-istio-tests.sh index 53231439..5dc4732c 100755 --- a/test/e2e-istio-tests.sh +++ b/test/e2e-istio-tests.sh @@ -60,6 +60,11 @@ spec: service: port: 9898 portDiscovery: true + apex: + annotations: + test: "annotations-test" + labels: + test: "labels-test" headers: request: add: @@ -110,6 +115,11 @@ done echo '✔ Canary initialization test passed' +kubectl -n test get svc/podinfo -oyaml | grep annotations-test +kubectl -n test get svc/podinfo -oyaml | grep labels-test + +echo '✔ Canary service custom metadata test passed' + echo '>>> Triggering canary deployment' kubectl -n test set image deployment/podinfo podinfod=stefanprodan/podinfo:3.1.1 From 4460cb73851950f25e59596fd1722e9ada3d81c9 Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Wed, 1 Apr 2020 15:43:54 -0700 Subject: [PATCH 08/11] Fix CRD indentation --- artifacts/flagger/crd.yaml | 66 ++++++++++++++++----------------- charts/flagger/crds/crd.yaml | 66 ++++++++++++++++----------------- kustomize/base/flagger/crd.yaml | 66 ++++++++++++++++----------------- 3 files changed, 99 insertions(+), 99 deletions(-) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 0fc6b2ac..68cabd96 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -499,42 +499,42 @@ spec: format: string type: string type: array - apex: - description: Metadata to add to the apex service + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - primary: - description: Metadata to add to the primary service + additionalProperties: + type: string + annotations: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - canary: - description: Metadata to add to the canary service + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 0fc6b2ac..68cabd96 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -499,42 +499,42 @@ spec: format: string type: string type: array - apex: - description: Metadata to add to the apex service + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - primary: - description: Metadata to add to the primary service + additionalProperties: + type: string + annotations: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - canary: - description: Metadata to add to the canary service + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 0fc6b2ac..68cabd96 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -499,42 +499,42 @@ spec: format: string type: string type: array - apex: - description: Metadata to add to the apex service + apex: + description: Metadata to add to the apex service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - primary: - description: Metadata to add to the primary service + additionalProperties: + type: string + annotations: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - canary: - description: Metadata to add to the canary service + additionalProperties: + type: string + primary: + description: Metadata to add to the primary service + type: object + properties: + labels: type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + canary: + description: Metadata to add to the canary service + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string skipAnalysis: description: Skip analysis and promote canary type: boolean From 3f961ae73f1ee2645bf651de1bbd0232276c9067 Mon Sep 17 00:00:00 2001 From: Finn Herzfeld Date: Fri, 3 Apr 2020 13:55:58 -0700 Subject: [PATCH 09/11] Handle annotations/labels update --- pkg/router/kubernetes_default.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 0d58e49f..8189c191 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -180,12 +180,38 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam } } + updateService := false + svcClone := svc.DeepCopy() + portsDiff := cmp.Diff(svcSpec.Ports, svc.Spec.Ports, cmpopts.SortSlices(sortPorts)) selectorsDiff := cmp.Diff(svcSpec.Selector, svc.Spec.Selector) + if portsDiff != "" || selectorsDiff != "" { svcClone := svc.DeepCopy() svcClone.Spec.Ports = svcSpec.Ports svcClone.Spec.Selector = svcSpec.Selector + _, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(context.TODO(), svcClone, metav1.UpdateOptions{}) + updateService = true + } + + // TODO: Handle annotation/label removal + annotationsDiff := cmp.Diff(metadata.Annotations, svc.ObjectMeta.Annotations) + if annotationsDiff != "" { + for k, v := range metadata.Annotations { + svcClone.ObjectMeta.Annotations[k] = v + } + updateService = true + } + + labelsDiff := cmp.Diff(metadata.Labels, svc.ObjectMeta.Labels) + if labelsDiff != "" { + for k, v := range metadata.Labels { + svcClone.ObjectMeta.Labels[k] = v + } + updateService = true + } + + if updateService { _, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(context.TODO(), svcClone, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("service %s update error: %w", name, err) From 8d37b7b20b069570bc651836d622a78f3e62ff78 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 4 Apr 2020 10:00:00 +0300 Subject: [PATCH 10/11] Update service metadata if ownerRef kind is canary --- pkg/router/kubernetes_default.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 8189c191..a9a2ee2a 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -194,21 +194,16 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam updateService = true } - // TODO: Handle annotation/label removal - annotationsDiff := cmp.Diff(metadata.Annotations, svc.ObjectMeta.Annotations) - if annotationsDiff != "" { - for k, v := range metadata.Annotations { - svcClone.ObjectMeta.Annotations[k] = v + // update annotations and labels only if the service has been created by Flagger + if _, owned := c.isOwnedByCanary(svc, canary.Name); owned { + if cmp.Diff(metadata.Annotations, svc.ObjectMeta.Annotations) != "" { + svcClone.ObjectMeta.Annotations = metadata.Annotations + updateService = true } - updateService = true - } - - labelsDiff := cmp.Diff(metadata.Labels, svc.ObjectMeta.Labels) - if labelsDiff != "" { - for k, v := range metadata.Labels { - svcClone.ObjectMeta.Labels[k] = v + if cmp.Diff(metadata.Labels, svc.ObjectMeta.Labels) != "" { + svcClone.ObjectMeta.Labels = metadata.Labels + updateService = true } - updateService = true } if updateService { From 84dd0006ca4a3410ebde59d3d92485c49c63c42e Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Sat, 4 Apr 2020 10:16:00 +0300 Subject: [PATCH 11/11] Add service metadata update unit test --- pkg/controller/finalizer.go | 2 +- pkg/controller/scheduler.go | 2 +- pkg/router/factory.go | 3 +-- pkg/router/kubernetes_default.go | 9 ++------- pkg/router/kubernetes_default_test.go | 24 +++++++++++++++++++----- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/pkg/controller/finalizer.go b/pkg/controller/finalizer.go index 7c4a4be7..50596272 100644 --- a/pkg/controller/finalizer.go +++ b/pkg/controller/finalizer.go @@ -56,7 +56,7 @@ func (c *Controller) finalize(old interface{}) error { } // Revert the Kubernetes service - router := c.routerFactory.KubernetesRouter(canary.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports) + router := c.routerFactory.KubernetesRouter(canary.Spec.TargetRef.Kind, labelSelector, ports) if err := router.Finalize(canary); err != nil { return fmt.Errorf("failed revert router: %w", err) } diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 7de4bb8a..a9ca1af7 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -109,7 +109,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { } // init Kubernetes router - kubeRouter := c.routerFactory.KubernetesRouter(cd.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports) + kubeRouter := c.routerFactory.KubernetesRouter(cd.Spec.TargetRef.Kind, labelSelector, ports) if err := kubeRouter.Initialize(cd); err != nil { c.recordEventWarningf(cd, "%v", err) return diff --git a/pkg/router/factory.go b/pkg/router/factory.go index 4356fd74..5e28868a 100644 --- a/pkg/router/factory.go +++ b/pkg/router/factory.go @@ -35,7 +35,7 @@ func NewFactory(kubeConfig *restclient.Config, kubeClient kubernetes.Interface, } // KubernetesRouter returns a KubernetesRouter interface implementation -func (factory *Factory) KubernetesRouter(kind string, labelSelector string, annotations map[string]string, ports map[string]int32) KubernetesRouter { +func (factory *Factory) KubernetesRouter(kind string, labelSelector string, ports map[string]int32) KubernetesRouter { switch kind { case "Service": return &KubernetesNoopRouter{} @@ -45,7 +45,6 @@ func (factory *Factory) KubernetesRouter(kind string, labelSelector string, anno flaggerClient: factory.flaggerClient, kubeClient: factory.kubeClient, labelSelector: labelSelector, - annotations: annotations, ports: ports, } } diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index a9a2ee2a..a8d4a9c9 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -25,7 +25,6 @@ type KubernetesDefaultRouter struct { flaggerClient clientset.Interface logger *zap.SugaredLogger labelSelector string - annotations map[string]string ports map[string]int32 } @@ -120,15 +119,12 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam if metadata.Labels == nil { metadata.Labels = make(map[string]string) } + metadata.Labels[c.labelSelector] = name + if metadata.Annotations == nil { metadata.Annotations = make(map[string]string) } - metadata.Labels[c.labelSelector] = name - for k, v := range c.annotations { - metadata.Annotations[k] = v - } - c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)). Debugw(fmt.Sprintf("Creating Service %s", name), "metadata", metadata, "service_configuration", canary.Spec.Service) @@ -187,7 +183,6 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam selectorsDiff := cmp.Diff(svcSpec.Selector, svc.Spec.Selector) if portsDiff != "" || selectorsDiff != "" { - svcClone := svc.DeepCopy() svcClone.Spec.Ports = svcSpec.Ports svcClone.Spec.Selector = svcSpec.Selector _, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(context.TODO(), svcClone, metav1.UpdateOptions{}) diff --git a/pkg/router/kubernetes_default_test.go b/pkg/router/kubernetes_default_test.go index 14f874fe..21b51712 100644 --- a/pkg/router/kubernetes_default_test.go +++ b/pkg/router/kubernetes_default_test.go @@ -367,13 +367,13 @@ func TestServiceRouter_InitializeMetadata(t *testing.T) { err := router.Initialize(mocks.canary) require.NoError(t, err) - canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-canary", metav1.GetOptions{}) + canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-canary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, "test", canarySvc.Annotations["test"]) assert.Equal(t, "test", canarySvc.Labels["test"]) assert.Equal(t, "podinfo-canary", canarySvc.Labels["app"]) - primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-primary", metav1.GetOptions{}) + primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, 0, len(primarySvc.Annotations)) assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"]) @@ -399,19 +399,33 @@ func TestServiceRouter_ReconcileMetadata(t *testing.T) { err = router.Reconcile(mocks.canary) require.NoError(t, err) - apexSvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo", metav1.GetOptions{}) + apexSvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, "test", apexSvc.Annotations["test"]) assert.Equal(t, "test", apexSvc.Labels["test"]) assert.Equal(t, "podinfo", apexSvc.Labels["app"]) - canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-canary", metav1.GetOptions{}) + canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-canary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, 0, len(canarySvc.Annotations)) assert.Equal(t, "podinfo-canary", canarySvc.Labels["app"]) - primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get("podinfo-primary", metav1.GetOptions{}) + primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{}) require.NoError(t, err) assert.Equal(t, 0, len(primarySvc.Annotations)) assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"]) + + mocks.canary.Spec.Service.Apex = &flaggerv1.CustomMetadata{ + Labels: map[string]string{"test": "test1"}, + Annotations: map[string]string{"test1": "test"}, + } + + err = router.Reconcile(mocks.canary) + require.NoError(t, err) + + apexSvc, err = mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "test", apexSvc.Annotations["test1"]) + assert.Equal(t, "test1", apexSvc.Labels["test"]) + assert.Equal(t, "podinfo", apexSvc.Labels["app"]) }