From 92937a8f48e6f284fe1d5ec7aad362edcb39e260 Mon Sep 17 00:00:00 2001 From: Tanner Altares Date: Tue, 10 Mar 2020 10:11:47 -0500 Subject: [PATCH] kubectl annotation support rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller add unit tests for finalizing introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller run fmt to clean up formatting review changes add kubectl annotation add kubectl annotation support introduction of finalizer introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller add unit tests for finalizing introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller run fmt to clean up formatting review changes introduction of finalizer introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller add unit tests for finalizing introduction of finalizer rebase and squash fix fmt issues revert Dockerfile revert go.mod and go.sum introduction of finalizer introduction of finalizer remove test for finalizer add istio tests fix fmt issues revert go.mod and go.sum revert Dockerfile and main.go fmt deployment controller run fmt to clean up formatting review changes --- Dockerfile | 13 +- artifacts/flagger/crd.yaml | 5 + charts/flagger/crds/crd.yaml | 5 + go.mod | 2 +- go.sum | 14 +- kustomize/base/flagger/crd.yaml | 3 + pkg/apis/flagger/v1beta1/canary.go | 4 + pkg/canary/daemonset_controller.go | 2 +- pkg/canary/deployment_controller.go | 56 +++-- pkg/canary/deployment_controller_test.go | 45 ++++ pkg/canary/service_controller.go | 2 +- pkg/controller/controller.go | 50 +++++ pkg/controller/finalizer.go | 28 +-- pkg/controller/finalizer_test.go | 50 ++--- pkg/router/appmesh.go | 4 + pkg/router/contour.go | 4 + pkg/router/gloo.go | 4 + pkg/router/ingress.go | 4 + pkg/router/istio.go | 61 ++++++ pkg/router/istio_test.go | 119 +++++++++++ pkg/router/kubernetes_default.go | 33 ++- pkg/router/kubernetes_default_test.go | 258 +++++++++++++++++++++++ pkg/router/kubernetes_noop.go | 4 + pkg/router/nop.go | 4 + pkg/router/router.go | 3 + pkg/router/smi.go | 4 + 26 files changed, 685 insertions(+), 96 deletions(-) diff --git a/Dockerfile b/Dockerfile index 55734152..8f693b77 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,17 +1,8 @@ FROM alpine:3.11 - -RUN echo 'https://repository.walmart.com/content/repositories/alpine-v38/community' > /etc/apk/repositories \ - && echo 'https://repository.walmart.com/content/repositories/alpine-v38/main' >> /etc/apk/repositories \ - && apk update && apk upgrade && apk --no-cache add \ - ca-certificates - RUN addgroup -S flagger \ - && adduser -S -g flagger flagger - -#RUN addgroup -S flagger \ -# && adduser -S -g flagger flagger \ -# && apk --no-cache add ca-certificates + && adduser -S -g flagger flagger \ + && apk --no-cache add ca-certificates WORKDIR /home/flagger diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 3ae25fa8..a39bb578 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -502,6 +502,9 @@ spec: skipAnalysis: description: Skip analysis and promote canary type: boolean + revertOnDeletion: + description: Revert mutated resources to original spec on deletion + type: boolean analysis: description: Canary analysis for this canary type: object @@ -650,6 +653,8 @@ spec: - Finalising - Succeeded - Failed + - Terminating + - Terminated canaryWeight: description: Traffic weight percentage routed to canary type: number diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 3ae25fa8..a39bb578 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -502,6 +502,9 @@ spec: skipAnalysis: description: Skip analysis and promote canary type: boolean + revertOnDeletion: + description: Revert mutated resources to original spec on deletion + type: boolean analysis: description: Canary analysis for this canary type: object @@ -650,6 +653,8 @@ spec: - Finalising - Succeeded - Failed + - Terminating + - Terminated canaryWeight: description: Traffic weight percentage routed to canary type: number diff --git a/go.mod b/go.mod index ed7aaf92..75f58ddb 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( k8s.io/api v0.17.2 k8s.io/apimachinery v0.17.2 k8s.io/client-go v0.17.2 - k8s.io/code-generator v0.17.3 + k8s.io/code-generator v0.17.2 k8s.io/utils v0.0.0-20191114184206-e782cd3c129f ) diff --git a/go.sum b/go.sum index b4359eb6..dc82318c 100644 --- a/go.sum +++ b/go.sum @@ -15,10 +15,8 @@ github.com/Masterminds/semver/v3 v3.0.3 h1:znjIyLfpXEDQjOIEWh+ehwpTU14UzUPub3c3s github.com/Masterminds/semver/v3 v3.0.3/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= github.com/PuerkitoBio/purell v1.0.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= -github.com/PuerkitoBio/purell v1.1.1 h1:WEQqlqaGbrPkxLJWfBwQmfEAE1Z7ONdDLqrN38tNFfI= github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= -github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M= github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= @@ -36,7 +34,6 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= github.com/elazarl/goproxy v0.0.0-20170405201442-c4fc26588b6e/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc= github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= -github.com/emicklei/go-restful v2.9.5+incompatible h1:spTtZBk5DYEvbxMVutUuTyh1Ao2r4iyvLdACqsl/Ljk= github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= github.com/evanphx/json-patch v4.2.0+incompatible h1:fUDGZCv/7iAN7u0puUVhvKCcsR6vRfwrJatElLBEf0I= github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= @@ -49,18 +46,14 @@ github.com/go-logfmt/logfmt v0.3.0 h1:8HUsc87TaSWLKwrnumgC8/YconD2fJQsRJAsWaPg2i github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+35s3my2LFTysnkMfxsJBAMHj/DoqoB9knIWoYG/Vk0= github.com/go-openapi/jsonpointer v0.19.2/go.mod h1:3akKfEdA7DF1sugOqz1dVQHBcuDBPKZGEoHC/NkiQRg= -github.com/go-openapi/jsonpointer v0.19.3 h1:gihV7YNZK1iK6Tgwwsxo2rJbD1GTbdm72325Bq8FI3w= github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/jsonreference v0.0.0-20160704190145-13c6e3589ad9/go.mod h1:W3Z9FmVs9qj+KR4zFKmDPGiLdk1D9Rlm7cyMvf57TTg= github.com/go-openapi/jsonreference v0.19.2/go.mod h1:jMjeRr2HHw6nAVajTXJ4eiUwohSTlpa0o73RUL1owJc= -github.com/go-openapi/jsonreference v0.19.3 h1:5cxNfTy0UVC3X8JL5ymxzyoUZmo8iZb+jeTWn7tUa8o= github.com/go-openapi/jsonreference v0.19.3/go.mod h1:rjx6GuL8TTa9VaixXglHmQmIL98+wF9xc8zWvFonSJ8= github.com/go-openapi/spec v0.0.0-20160808142527-6aced65f8501/go.mod h1:J8+jY1nAiCcj+friV/PDoE1/3eeccG9LYBs0tYvLOWc= -github.com/go-openapi/spec v0.19.3 h1:0XRyw8kguri6Yw4SxhsQA/atC88yqrk0+G4YhI2wabc= github.com/go-openapi/spec v0.19.3/go.mod h1:FpwSN1ksY1eteniUU7X0N/BgJ7a4WvBFVA8Lj9mJglo= github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dpr1UfpPtxFw+EFuQ41HhCWZfha5jSVRG7C7I= github.com/go-openapi/swag v0.19.2/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= -github.com/go-openapi/swag v0.19.5 h1:lTz6Ys4CmqqCQmZPBlbQENR1/GucA2bzYTE12Pw4tFY= github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= @@ -130,7 +123,6 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= -github.com/mailru/easyjson v0.7.0 h1:aizVhC/NAAcKWb+5QsU1iNOZb4Yws5UO2I+aIprQITM= github.com/mailru/easyjson v0.7.0/go.mod h1:KAzv3t3aY1NaHWoQz1+4F1ccyAH66Jk7yos7ldAVICs= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= @@ -305,8 +297,6 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= -gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= k8s.io/api v0.17.2 h1:NF1UFXcKN7/OOv1uxdRz3qfra8AHsPav5M93hlV9+Dc= @@ -317,8 +307,6 @@ k8s.io/client-go v0.17.2 h1:ndIfkfXEGrNhLIgkr0+qhRguSD3u6DCmonepn1O6NYc= k8s.io/client-go v0.17.2/go.mod h1:QAzRgsa0C2xl4/eVpeVAZMvikCn8Nm81yqVx3Kk9XYI= k8s.io/code-generator v0.17.2 h1:pTwl3rLB1fUyxmvEzmVPMM0tBSdUehd7z+bDzpj4lPE= k8s.io/code-generator v0.17.2/go.mod h1:DVmfPQgxQENqDIzVR2ddLXMH34qeszkKSdH/N+s+38s= -k8s.io/code-generator v0.17.3 h1:q/hDMk2cvFzSxol7k/VA1qCssR7VSMXHQHhzuX29VJ8= -k8s.io/code-generator v0.17.3/go.mod h1:l8BLVwASXQZTo2xamW5mQNFCe1XPiAesVq7Y1t7PiQQ= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6 h1:4s3/R4+OYYYUKptXPhZKjQ04WJ6EhQQVFdjOFvCazDk= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20190822140433-26a664648505 h1:ZY6yclUKVbZ+SdWnkfY+Je5vrMpKOxmGeKRbsXVmqYM= @@ -334,4 +322,4 @@ modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I= sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI= sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs= -sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o= +sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o= \ No newline at end of file diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 3ae25fa8..5d6ef4db 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -502,6 +502,9 @@ spec: skipAnalysis: description: Skip analysis and promote canary type: boolean + revertOnDeletion: + description: Revert mutated resources to original spec on deletion + type: boolean analysis: description: Canary analysis for this canary type: object diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index a68a0a43..24a643df 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -94,6 +94,10 @@ type CanarySpec struct { // SkipAnalysis promotes the canary without analysing it // +optional SkipAnalysis bool `json:"skipAnalysis,omitempty"` + + // revert canary mutation on deletion of canary resource + // +optional + RevertOnDeletion bool `json:"revertOnDeletion,omitempty"` } // CanaryService defines how ClusterIP services, service mesh or ingress routing objects are generated diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index e0e80d7b..d52f5d34 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -297,6 +297,6 @@ func (c *DaemonSetController) HaveDependenciesChanged(cd *flaggerv1.Canary) (boo return c.configTracker.HasConfigChanged(cd) } -func (c *DaemonSetController) Finalize(cd *flaggerv1.Canary) error { +func (c *DaemonSetController) Finalize(cd *flaggerv1.Canary) error { return nil } diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 19bc874e..9eae0cc7 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -179,6 +179,27 @@ func (c *DeploymentController) ScaleFromZero(cd *flaggerv1.Canary) error { return nil } +// Scale sets the canary deployment replicas +func (c *DeploymentController) Scale(cd *flaggerv1.Canary, replicas int32) error { + targetName := cd.Spec.TargetRef.Name + dep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(targetName, metav1.GetOptions{}) + if err != nil { + if errors.IsNotFound(err) { + return fmt.Errorf("deployment %s.%s not found", targetName, cd.Namespace) + } + return fmt.Errorf("deployment %s.%s query error %v", targetName, cd.Namespace, err) + } + + depCopy := dep.DeepCopy() + depCopy.Spec.Replicas = int32p(replicas) + + _, err = c.kubeClient.AppsV1().Deployments(dep.Namespace).Update(depCopy) + if err != nil { + return fmt.Errorf("scaling %s.%s to %v failed: %v", depCopy.GetName(), depCopy.Namespace, replicas, err) + } + return nil +} + // GetMetadata returns the pod label selector and svc ports func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (string, map[string]int32, error) { targetName := cd.Spec.TargetRef.Name @@ -377,36 +398,35 @@ func (c *DeploymentController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bo return c.configTracker.HasConfigChanged(cd) } - // revertDeployment will set the replica count from the primary to the reference instance. This method is used // during a delete to attempt to revert the deployment back to the original state. Error is returned if unable // update the reference deployment replicas to the primary replicas -func (c *DeploymentController) Finalize(cd *flaggerv1.Canary) error { +func (c *DeploymentController) Finalize(cd *flaggerv1.Canary) error { - //1. Get the Primary deployment if possible primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name) - //2. Get the replicas value - primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(primaryName, metav1.GetOptions{}) - if err != nil { - /*if errors.IsNotFound(err) { - c.logger.Warnf("deployment %s.%s not found while finalizing", primaryName, cd.Namespace) - return nil - }*/ - return err - } + + //Get ref deployment refDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(cd.Spec.TargetRef.Name, metav1.GetOptions{}) if err != nil { - /*if errors.IsNotFound(err) { - c.logger.Warnf("deployment %s.%s not found while finalizing", cd.Spec.TargetRef.Name, cd.Namespace) - return nil - }*/ return err } + //2. Get primary if possible, if not scale from zero + primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(primaryName, metav1.GetOptions{}) + if err != nil { + if errors.IsNotFound(err) { + if err := c.ScaleFromZero(cd); err != nil { + return err + } + return nil + } + return err + } + + //3. If both ref and primary present update the replicas of the ref to match the primary if refDep.Spec.Replicas != primaryDep.Spec.Replicas { //3. Set the replicas value on the original reference deployment - desiredReplicas := primaryDep.Spec.Replicas - if err := c.Scale(cd, int32Default(desiredReplicas)); err != nil { + if err := c.Scale(cd, int32Default(primaryDep.Spec.Replicas)); err != nil { return err } } diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 0c24d8e7..9ef5e312 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -188,3 +188,48 @@ func TestDeploymentController_HasTargetChanged(t *testing.T) { require.NoError(t, err) assert.True(t, isNew) } + +func TestCanaryDeployer_Finalize(t *testing.T) { + + mocks := newDeploymentFixture() + + tables := []struct { + mocks deploymentControllerFixture + callInitialize bool + shouldError bool + expectedReplicas int32 + canary *flaggerv1.Canary + }{ + //Primary not found returns error + {mocks, false, false, 1, mocks.canary}, + //Happy path + {mocks, true, false, 1, mocks.canary}, + } + + for _, table := range tables { + if table.callInitialize { + err := mocks.controller.Initialize(table.canary, true) + if err != nil { + t.Fatal(err.Error()) + } + } + + err := mocks.controller.Finalize(table.canary) + + if table.shouldError && err == nil { + t.Error("Expected error while calling Finalize, but none was returned") + } else if !table.shouldError && err != nil { + t.Errorf("Expected no error would be returned while calling Finalize, but returned %s", err) + } + + if table.expectedReplicas > 0 { + c, err := mocks.kubeClient.AppsV1().Deployments(mocks.canary.Namespace).Get(mocks.canary.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + if int32Default(c.Spec.Replicas) != table.expectedReplicas { + t.Errorf("Expected replicas %d recieved replicas %d", table.expectedReplicas, c.Spec.Replicas) + } + } + } +} diff --git a/pkg/canary/service_controller.go b/pkg/canary/service_controller.go index fa7ff8ef..94e14c73 100644 --- a/pkg/canary/service_controller.go +++ b/pkg/canary/service_controller.go @@ -222,6 +222,6 @@ func (c *ServiceController) IsCanaryReady(_ *flaggerv1.Canary) (bool, error) { return true, nil } -func (c *ServiceController) Finalize(cd *flaggerv1.Canary) error { +func (c *ServiceController) Finalize(cd *flaggerv1.Canary) error { return nil } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 0b92c458..37a2779a 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -128,6 +128,21 @@ func NewController( } ctrl.enqueue(new) + } else if !newCanary.DeletionTimestamp.IsZero() && hasFinalizer(&newCanary, finalizer) || + !hasFinalizer(&newCanary, finalizer) && newCanary.Spec.RevertOnDeletion { + //If this was marked for deletion and has finalizers enqueue for finalizing or + //If this canary doesn't have finalizers and RevertOnDeletion is true updated speck enqueue + ctrl.enqueue(new) + } + + //If canary no longer desires reverting, finalizers should be removed + if oldCanary.Spec.RevertOnDeletion && !newCanary.Spec.RevertOnDeletion { + ctrl.logger.Infof("%s.%s opting out, deleting finalizers", newCanary.Name, newCanary.Namespace) + err := ctrl.removeFinalizer(&newCanary, finalizer) + if err != nil { + ctrl.logger.Warnf("Failed to finalizers for %s.%s", oldCanary.Name, oldCanary.Namespace) + return + } } }, DeleteFunc: func(old interface{}) { @@ -217,6 +232,33 @@ func (c *Controller) syncHandler(key string) error { return nil } + //Finalize if canary has been marked for deletion and revert is desired + if cd.Spec.RevertOnDeletion && cd.ObjectMeta.DeletionTimestamp != nil { + + //If finalizers have been previously removed proceed + if !hasFinalizer(cd, finalizer) { + c.logger.Infof("Canary %s.%s has been finalized", cd.Name, cd.Namespace) + return nil + } + + if cd.Status.Phase != flaggerv1.CanaryPhaseTerminated { + if err := c.finalize(cd); err != nil { + return fmt.Errorf("unable to finalize to canary %s.%s error %s", cd.Name, cd.Namespace, err) + } + } + + //Remove finalizer from Canary + if err := c.removeFinalizer(cd, finalizer); err != nil { + return fmt.Errorf("unable to remove finalizer for canary %s.%s", cd.Name, cd.Namespace) + } + + //record event + c.recordEventInfof(cd, "Terminated canary %s.%s", cd.Name, cd.Namespace) + + c.logger.Infof("Canary %s.%s has been successfully processed and marked for deletion", cd.Name, cd.Namespace) + return nil + } + // set status condition for new canaries if cd.Status.Conditions == nil { if ok, conditions := canary.MakeStatusConditions(cd, flaggerv1.CanaryPhaseInitializing); ok { @@ -233,6 +275,14 @@ func (c *Controller) syncHandler(key string) error { } c.canaries.Store(fmt.Sprintf("%s.%s", cd.Name, cd.Namespace), cd) + + //If opt in for revertOnDeletion add finaliers if not present + if cd.Spec.RevertOnDeletion && !hasFinalizer(cd, finalizer) { + if err := c.addFinalizer(cd, finalizer); err != nil { + return fmt.Errorf("unable to add finalizer to canary %s.%s", cd.Name, cd.Namespace) + } + + } c.logger.Infof("Synced %s", key) return nil diff --git a/pkg/controller/finalizer.go b/pkg/controller/finalizer.go index 3f7f136b..49301ba8 100644 --- a/pkg/controller/finalizer.go +++ b/pkg/controller/finalizer.go @@ -2,6 +2,7 @@ package controller import ( "fmt" + ex "github.com/pkg/errors" flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" "github.com/weaveworks/flagger/pkg/canary" @@ -10,7 +11,7 @@ import ( "k8s.io/client-go/util/retry" ) -const finalizer = "finalizer.flagger.com" +const finalizer = "finalizer.flagger.app" func (c *Controller) finalize(old interface{}) error { var r *flaggerv1.Canary @@ -39,7 +40,7 @@ func (c *Controller) finalize(old interface{}) error { if err != nil { if errors.IsNotFound(err) { //No reason to wait not found - c.logger.Warnf("%s.%s failed due to %s not found", r.Name, r.Namespace, r.Kind) + c.logger.Warnf("%s.%s failed due to %s not found", r.Name, r.Namespace, r.Spec.TargetRef.Kind) return nil } c.logger.Errorf("%s.%s failed due to %s", r.Name, r.Namespace, err) @@ -55,7 +56,6 @@ func (c *Controller) finalize(old interface{}) error { } c.logger.Infof("%s.%s moving forward with router finalizing", r.Name, r.Namespace) - //TODO if I can't revert continue on? labelSelector, ports, err := canaryController.GetMetadata(r) if err != nil { c.logger.Errorf("%s.%s failed to get metadata for router finalizing", r.Name, r.Namespace) @@ -82,20 +82,12 @@ func (c *Controller) finalize(old interface{}) error { c.logger.Infof("Finalization complete for %s.%s", r.Name, r.Namespace) return nil - - } func (c *Controller) revertTargetRef(ctrl canary.Controller, r *flaggerv1.Canary) error { if err := ctrl.Finalize(r); err != nil { return err } - /*if err != nil { - if errors.IsNotFound(err) { - return false, err - } - return true, fmt.Errorf("%s.%s failed to revert deployment to original replicas", r.Name, r.Namespace) - }*/ c.logger.Infof("%s.%s kind %s reverted", r.Name, r.Namespace, r.Spec.TargetRef.Kind) return nil } @@ -107,9 +99,6 @@ func (c *Controller) revertRouter(r *flaggerv1.Canary, labelSelector string, por c.logger.Errorf("%s.%s router failed with error %s", r.Name, r.Namespace, err) return err } - /*if err != nil { - return fmt.Errorf("%s.%s failed to revert service to original state", r.Name, r.Namespace) - }*/ c.logger.Infof("Service %s.%s reverted", r.Name, r.Namespace) return nil } @@ -158,7 +147,7 @@ func (c *Controller) addFinalizer(canary *flaggerv1.Canary, finalizerString stri var selErr error if !firstTry { - canary, selErr = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Get(canary.GetName(),metav1.GetOptions{}) + canary, selErr = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Get(canary.GetName(), metav1.GetOptions{}) if selErr != nil { return selErr } @@ -167,7 +156,7 @@ func (c *Controller) addFinalizer(canary *flaggerv1.Canary, finalizerString stri copy := canary.DeepCopy() copy.ObjectMeta.Finalizers = append(copy.ObjectMeta.Finalizers, finalizerString) - _, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(copy) + _, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(copy) firstTry = false return @@ -182,13 +171,13 @@ func (c *Controller) addFinalizer(canary *flaggerv1.Canary, finalizerString stri //removeFinalizer removes a provided finalizer to the specified canary resource. //If failures occur the error will be returned otherwise the action is deemed successful //and error will be nil. -func (c *Controller) removeFinalizer(canary *flaggerv1.Canary, finalizerString string) error { +func (c *Controller) removeFinalizer(canary *flaggerv1.Canary, finalizerString string) error { firstTry := true err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { var selErr error if !firstTry { - canary, selErr = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Get(canary.GetName(),metav1.GetOptions{}) + canary, selErr = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Get(canary.GetName(), metav1.GetOptions{}) if selErr != nil { return selErr } @@ -217,5 +206,4 @@ func (c *Controller) removeFinalizer(canary *flaggerv1.Canary, finalizerString s return ex.Wrap(err, "Remove finalizer failed") } return nil - -} \ No newline at end of file +} diff --git a/pkg/controller/finalizer_test.go b/pkg/controller/finalizer_test.go index b5ed4d3c..f5861934 100644 --- a/pkg/controller/finalizer_test.go +++ b/pkg/controller/finalizer_test.go @@ -2,6 +2,7 @@ package controller import ( "fmt" + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" fakeFlagger "github.com/weaveworks/flagger/pkg/client/clientset/versioned/fake" "k8s.io/apimachinery/pkg/runtime" @@ -13,15 +14,15 @@ import ( //Test has finalizers func TestFinalizer_hasFinalizer(t *testing.T) { - withFinalizer := newTestCanary() + withFinalizer := newDeploymentTestCanary() withFinalizer.Finalizers = append(withFinalizer.Finalizers, finalizer) - tables := []struct{ + tables := []struct { canary *flaggerv1.Canary result bool }{ - {newTestCanary(), false}, - { withFinalizer, true}, + {newDeploymentTestCanary(), false}, + {withFinalizer, true}, } for _, table := range tables { @@ -35,24 +36,23 @@ func TestFinalizer_hasFinalizer(t *testing.T) { func TestFinalizer_addFinalizer(t *testing.T) { mockError := fmt.Errorf("failed to add finalizer to canary %s", "testCanary") - cs := fakeFlagger.NewSimpleClientset(newTestCanary()) + cs := fakeFlagger.NewSimpleClientset(newDeploymentTestCanary()) //prepend so it is evaluated over the catch all * - cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error){ + cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, mockError }) - m := Mocks{ - canary:newTestCanary(), + m := fixture{ + canary: newDeploymentTestCanary(), flaggerClient: cs, - ctrl:&Controller{flaggerClient:cs}, + ctrl: &Controller{flaggerClient: cs}, } - - tables := []struct{ - mock Mocks + tables := []struct { + mock fixture canary *flaggerv1.Canary - error error + error error }{ - {SetupMocks(nil), newTestCanary(), nil}, + {newDeploymentFixture(nil), newDeploymentTestCanary(), nil}, {m, m.canary, mockError}, } @@ -70,27 +70,27 @@ func TestFinalizer_addFinalizer(t *testing.T) { func TestFinalizer_removeFinalizer(t *testing.T) { - withFinalizer := newTestCanary() + withFinalizer := newDeploymentTestCanary() withFinalizer.Finalizers = append(withFinalizer.Finalizers, finalizer) mockError := fmt.Errorf("failed to add finalizer to canary %s", "testCanary") - cs := fakeFlagger.NewSimpleClientset(newTestCanary()) + cs := fakeFlagger.NewSimpleClientset(newDeploymentTestCanary()) //prepend so it is evaluated over the catch all * - cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error){ + cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, mockError }) - m := Mocks{ - canary:withFinalizer, + m := fixture{ + canary: withFinalizer, flaggerClient: cs, - ctrl:&Controller{flaggerClient:cs}, + ctrl: &Controller{flaggerClient: cs}, } - tables := []struct{ - mock Mocks + tables := []struct { + mock fixture canary *flaggerv1.Canary - error error + error error }{ - {SetupMocks(nil), withFinalizer, nil}, + {newDeploymentFixture(nil), withFinalizer, nil}, {m, m.canary, mockError}, } @@ -104,4 +104,4 @@ func TestFinalizer_removeFinalizer(t *testing.T) { } } -} \ No newline at end of file +} diff --git a/pkg/router/appmesh.go b/pkg/router/appmesh.go index 83a32cf3..ac7335dc 100644 --- a/pkg/router/appmesh.go +++ b/pkg/router/appmesh.go @@ -483,6 +483,10 @@ func (ar *AppMeshRouter) gatewayAnnotations(canary *flaggerv1.Canary) map[string return a } +func (ar *AppMeshRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} + func int64p(i int64) *int64 { return &i } diff --git a/pkg/router/contour.go b/pkg/router/contour.go index dfd7dd11..0b362cb9 100644 --- a/pkg/router/contour.go +++ b/pkg/router/contour.go @@ -417,3 +417,7 @@ func (cr *ContourRouter) makeLinkerdHeaderValue(canary *flaggerv1.Canary, servic } } + +func (cr *ContourRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} diff --git a/pkg/router/gloo.go b/pkg/router/gloo.go index c2401e04..3eab5f0a 100644 --- a/pkg/router/gloo.go +++ b/pkg/router/gloo.go @@ -185,3 +185,7 @@ func (gr *GlooRouter) SetRoutes( } return nil } + +func (gr *GlooRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go index 70682aac..14d2b03f 100644 --- a/pkg/router/ingress.go +++ b/pkg/router/ingress.go @@ -237,3 +237,7 @@ func (i *IngressRouter) makeHeaderAnnotations(annotations map[string]string, func (i *IngressRouter) GetAnnotationWithPrefix(suffix string) string { return fmt.Sprintf("%v/%v", i.annotationsPrefix, suffix) } + +func (i *IngressRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} diff --git a/pkg/router/istio.go b/pkg/router/istio.go index 3cdfd220..613d4513 100644 --- a/pkg/router/istio.go +++ b/pkg/router/istio.go @@ -1,6 +1,7 @@ package router import ( + "encoding/json" "fmt" "github.com/google/go-cmp/cmp" @@ -211,6 +212,22 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error { vtClone := virtualService.DeepCopy() vtClone.Spec = newSpec + //If annotation kubectl.kubernetes.io/last-applied-configuration is present no need to duplicate + //serialization. If not present store the serialized object in annotation + //flagger.kubernetes.io/original-configuration + if _, ok := vtClone.Annotations[kubectlAnnotation]; !ok { + b, err := json.Marshal(virtualService.Spec) + if err != nil { + ir.logger.Warnf("Unable to marshal VS %s for orig-configuration annotation", virtualService.Name) + } + + if vtClone.ObjectMeta.Annotations == nil { + vtClone.ObjectMeta.Annotations = make(map[string]string) + } + + vtClone.ObjectMeta.Annotations[configAnnotation] = string(b) + } + _, err = ir.istioClient.NetworkingV1alpha3().VirtualServices(canary.Namespace).Update(vtClone) if err != nil { return fmt.Errorf("VirtualService %s.%s update error: %w", apexName, canary.Namespace, err) @@ -348,6 +365,50 @@ func (ir *IstioRouter) SetRoutes( return nil } +func (ir *IstioRouter) Finalize(canary *flaggerv1.Canary) error { + + //Need to see if I can get the annotation orig-configuration + apexName, _, _ := canary.GetServiceNames() + + vs, err := ir.istioClient.NetworkingV1alpha3().VirtualServices(canary.Namespace).Get(apexName, metav1.GetOptions{}) + if err != nil { + return err + } + + var storedSpec istiov1alpha3.VirtualServiceSpec + //If able to get and unMarshal update the spec + if a, ok := vs.ObjectMeta.Annotations[kubectlAnnotation]; ok { + var storedVS istiov1alpha3.VirtualService + err := json.Unmarshal([]byte(a), &storedVS) + if err != nil { + return fmt.Errorf("VirtualService %s.%s failed to unMarshal annotation %s, unable to revert", + apexName, canary.Namespace, kubectlAnnotation) + } + storedSpec = storedVS.Spec + } else if a, ok := vs.ObjectMeta.Annotations[configAnnotation]; ok { + var spec istiov1alpha3.VirtualServiceSpec + err := json.Unmarshal([]byte(a), &spec) + if err != nil { + return fmt.Errorf("VirtualService %s.%s failed to unMarshal annotation %s, unable to revert", + apexName, canary.Namespace, configAnnotation) + } + storedSpec = spec + } else { + ir.logger.Warnf("VirtualService %s.%s original configuration not found, unable to revert", apexName, canary.Namespace) + return nil + } + + clone := vs.DeepCopy() + clone.Spec = storedSpec + + _, err = ir.istioClient.NetworkingV1alpha3().VirtualServices(canary.Namespace).Update(clone) + if err != nil { + return fmt.Errorf("VirtualService %s.%s update error %v, unable to revert", apexName, canary.Namespace, err) + } + + return nil +} + // mergeMatchConditions appends the URI match rules to canary conditions func mergeMatchConditions(canary, defaults []istiov1alpha3.HTTPMatchRequest) []istiov1alpha3.HTTPMatchRequest { for i := range canary { diff --git a/pkg/router/istio_test.go b/pkg/router/istio_test.go index 0b9c3591..3e592630 100644 --- a/pkg/router/istio_test.go +++ b/pkg/router/istio_test.go @@ -1,13 +1,17 @@ package router import ( + "encoding/json" "fmt" "testing" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" istiov1alpha3 "github.com/weaveworks/flagger/pkg/apis/istio/v1alpha3" ) @@ -329,3 +333,118 @@ func TestIstioRouter_GatewayPort(t *testing.T) { port := vs.Spec.Http[0].Route[0].Destination.Port.Number assert.Equal(t, uint32(mocks.canary.Spec.Service.Port), port) } + +func TestIstioRouter_Finalize(t *testing.T) { + mocks := newFixture(nil) + router := &IstioRouter{ + logger: mocks.logger, + flaggerClient: mocks.flaggerClient, + istioClient: mocks.meshClient, + kubeClient: mocks.kubeClient, + } + + flaggerSpec := &istiov1alpha3.VirtualServiceSpec{ + Http: []istiov1alpha3.HTTPRoute{ + { + Match: mocks.canary.Spec.Service.Match, + Rewrite: mocks.canary.Spec.Service.Rewrite, + Timeout: mocks.canary.Spec.Service.Timeout, + Retries: mocks.canary.Spec.Service.Retries, + CorsPolicy: mocks.canary.Spec.Service.CorsPolicy, + }, + }, + } + + kubectlSpec := &istiov1alpha3.VirtualServiceSpec{ + Hosts: []string{"podinfo"}, + Gateways: []string{"ingressgateway.istio-system.svc.cluster.local"}, + Http: []istiov1alpha3.HTTPRoute{ + { + Match: nil, + Route: []istiov1alpha3.DestinationWeight{ + { + Destination: istiov1alpha3.Destination{Host: "podinfo"}, + }, + }, + }, + }, + } + + tables := []struct { + router *IstioRouter + spec *istiov1alpha3.VirtualServiceSpec + shouldError bool + createVS bool + canary *v1beta1.Canary + callReconcile bool + annotation string + }{ + //VS not found + {router: router, spec: nil, shouldError: true, createVS: false, canary: mocks.canary, callReconcile: false, annotation: ""}, + //No annotation found but still finalizes + {router: router, spec: nil, shouldError: false, createVS: false, canary: mocks.canary, callReconcile: true, annotation: ""}, + //Spec should match annotation after finalize + {router: router, spec: flaggerSpec, shouldError: false, createVS: true, canary: mocks.canary, callReconcile: true, annotation: "flagger"}, + //Need to test kubectl annotation + {router: router, spec: kubectlSpec, shouldError: false, createVS: true, canary: mocks.canary, callReconcile: true, annotation: "kubectl"}, + } + + for _, table := range tables { + + var err error + + if table.createVS { + vs, err := router.istioClient.NetworkingV1alpha3().VirtualServices(table.canary.Namespace).Get(table.canary.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + if vs.Annotations == nil { + vs.Annotations = make(map[string]string) + } + + if table.annotation == "flagger" { + b, err := json.Marshal(table.spec) + if err != nil { + t.Fatal(err.Error()) + } + + vs.Annotations[configAnnotation] = string(b) + } else if table.annotation == "kubectl" { + vs.Annotations[kubectlAnnotation] = `{"apiVersion": "networking.istio.io/v1alpha3","kind": "VirtualService","metadata": {"annotations": {},"name": "podinfo","namespace": "test"}, "spec": {"gateways": ["ingressgateway.istio-system.svc.cluster.local"],"hosts": ["podinfo"],"http": [{"route": [{"destination": {"host": "podinfo"}}]}]}}` + + } + _, err = router.istioClient.NetworkingV1alpha3().VirtualServices(table.canary.Namespace).Update(vs) + if err != nil { + t.Fatal(err.Error()) + } + } + + if table.callReconcile { + err = router.Reconcile(table.canary) + if err != nil { + t.Fatal(err.Error()) + + } + } + + err = router.Finalize(table.canary) + + if table.shouldError && err == nil { + t.Errorf("Expected error from Finalize but error was not returned") + } else if !table.shouldError && err != nil { + t.Errorf("Expected no error from Finalize but error was returned %s", err) + } else if table.spec != nil { + vs, err := router.istioClient.NetworkingV1alpha3().VirtualServices(table.canary.Namespace).Get(table.canary.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + + } + if cmp.Diff(vs.Spec, *table.spec) != "" { + + t.Errorf("Expected spec %+v but recieved %+v", table.spec, vs.Spec) + } + } + + } +} diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go index 2b5f6bdf..4c23f0fb 100644 --- a/pkg/router/kubernetes_default.go +++ b/pkg/router/kubernetes_default.go @@ -1,6 +1,7 @@ package router import ( + "encoding/json" "fmt" "github.com/google/go-cmp/cmp" @@ -163,7 +164,8 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam return nil } -func (c *KubernetesDeploymentRouter) Finalize(canary *flaggerv1.Canary) error { +//Finalize reverts the apex router if not owned by the Flagger controller. +func (c *KubernetesDefaultRouter) Finalize(canary *flaggerv1.Canary) error { apexName, _, _ := canary.GetServiceNames() svc, err := c.kubeClient.CoreV1().Services(canary.Namespace).Get(apexName, metav1.GetOptions{}) @@ -171,10 +173,29 @@ func (c *KubernetesDeploymentRouter) Finalize(canary *flaggerv1.Canary) error { return err } + //No need to do any reconciliation if the router is owned by the controller if hasCanaryOwnerRef, isOwned := c.isOwnedByCanary(svc, canary.Name); !hasCanaryOwnerRef && !isOwned { - err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name) - if err != nil { - return err + //If kubectl annotation is present that will be utilized, else reconcile + if a, ok := svc.Annotations[kubectlAnnotation]; ok { + var storedSvc corev1.Service + err := json.Unmarshal([]byte(a), &storedSvc) + if err != nil { + return fmt.Errorf("router %s.%s failed to unMarshal annotation %s, unable to revert", + svc.Name, svc.Namespace, kubectlAnnotation) + } + clone := svc.DeepCopy() + clone.Spec = storedSvc.Spec + + _, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(clone) + if err != nil { + return fmt.Errorf("service %s update error: %w", clone.Name, err) + } + + } else { + err = c.reconcileService(canary, apexName, canary.Spec.TargetRef.Name) + if err != nil { + return err + } } } @@ -184,7 +205,7 @@ func (c *KubernetesDeploymentRouter) Finalize(canary *flaggerv1.Canary) error { //isOwnedByCanary evaluates if an object contains an OwnerReference declaration, that is of kind Canary and //has the same ref name as the Canary under evaluation. It returns two bool the first returns true if //an OwnerReference is present and the second, returns if it is owned by the supplied name. -func (c KubernetesDeploymentRouter) isOwnedByCanary(obj interface{}, name string) (bool, bool) { +func (c KubernetesDefaultRouter) isOwnedByCanary(obj interface{}, name string) (bool, bool) { var object metav1.Object var ok bool if object, ok = obj.(metav1.Object); ok { @@ -199,4 +220,4 @@ func (c KubernetesDeploymentRouter) isOwnedByCanary(obj interface{}, name string } } return false, false -} \ No newline at end of file +} diff --git a/pkg/router/kubernetes_default_test.go b/pkg/router/kubernetes_default_test.go index 89002885..a5efd24a 100644 --- a/pkg/router/kubernetes_default_test.go +++ b/pkg/router/kubernetes_default_test.go @@ -1,11 +1,17 @@ package router import ( + "fmt" "testing" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/client-go/kubernetes/fake" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -104,3 +110,255 @@ func TestServiceRouter_Undo(t *testing.T) { assert.Equal(t, "http", canarySvc.Spec.Ports[0].Name) assert.Equal(t, int32(9898), canarySvc.Spec.Ports[0].Port) } + +func TestServiceRouter_isOwnedByCanary(t *testing.T) { + mocks := newFixture(nil) + router := &KubernetesDefaultRouter{ + kubeClient: mocks.kubeClient, + flaggerClient: mocks.flaggerClient, + logger: mocks.logger, + } + + isController := new(bool) + *isController = true + + tables := []struct { + svc *corev1.Service + isOwned bool + hasOwnerRef bool + }{ + //owned + { + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "flagger.app/v1alpha3", + Kind: "Canary", + Name: "podinfo", + Controller: isController, + }, + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + }, isOwned: true, hasOwnerRef: true, + }, + //Owner ref but kind not Canary + { + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "flagger.app/v1alpha3", + Kind: "Deployment", + Name: "podinfo", + Controller: isController, + }, + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + }, isOwned: false, hasOwnerRef: false, + }, + //Owner ref but name doesn't match + { + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "flagger.app/v1alpha3", + Kind: "Canary", + Name: "notpodinfo", + Controller: isController, + }, + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + }, isOwned: false, hasOwnerRef: true, + }, + //No ownerRef + { + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + }, isOwned: false, hasOwnerRef: false, + }, + } + + for _, table := range tables { + hasOwnerRef, wasOwned := router.isOwnedByCanary(table.svc, mocks.canary.Name) + if table.isOwned && !wasOwned { + t.Error("Expected to be owned, but was not") + } else if !table.isOwned && wasOwned { + t.Error("Expected not to be owned but was") + } else if table.hasOwnerRef && !hasOwnerRef { + t.Error("Expected to contain OwnerReference but not present") + } else if !table.hasOwnerRef && hasOwnerRef { + t.Error("Expected not to have an OwnerReference but present") + } + } + +} + +func TestServiceRouter_Finalize(t *testing.T) { + + mocks := newFixture(nil) + router := &KubernetesDefaultRouter{ + kubeClient: mocks.kubeClient, + flaggerClient: mocks.flaggerClient, + logger: mocks.logger, + labelSelector: "app", + } + + isController := new(bool) + *isController = true + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "flagger.app/v1alpha3", + Kind: "Canary", + Name: "NotOwned", + Controller: isController, + }, + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 9898, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + } + + kubectlSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podinfo", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "flagger.app/v1alpha3", + Kind: "Canary", + Name: "NotOwned", + Controller: isController, + }, + }, + Annotations: map[string]string{ + kubectlAnnotation: `{"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"labels":{"app":"podinfo"},"name":"podinfo","namespace":"test"},"spec":{"ports":[{"name":"http","port":9898,"protocol":"TCP","targetPort":9898}],"selector":{"app":"podinfo"},"type":"ClusterIP"}}`, + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 9898, + }}, + Selector: map[string]string{"app": "podinfo"}, + }, + } + + tables := []struct { + router *KubernetesDefaultRouter + callSetupMethods bool + shouldError bool + canary *v1beta1.Canary + shouldMutate bool + }{ + //Won't reconcile since it is owned and would be garbage collected + {router: router, callSetupMethods: true, shouldError: false, canary: mocks.canary, shouldMutate: false}, + //Service not found + {router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(), logger: mocks.logger}, callSetupMethods: false, shouldError: true, canary: mocks.canary, shouldMutate: false}, + //Not owned + {router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(svc), logger: mocks.logger}, callSetupMethods: false, shouldError: false, canary: mocks.canary, shouldMutate: true}, + //Kubectl annotation + {router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(kubectlSvc), logger: mocks.logger}, callSetupMethods: false, shouldError: false, canary: mocks.canary, shouldMutate: true}, + } + + for _, table := range tables { + + if table.callSetupMethods { + err := table.router.Initialize(table.canary) + if err != nil { + t.Fatal(err.Error()) + } + + err = table.router.Reconcile(table.canary) + if err != nil { + t.Fatal(err.Error()) + } + } + + err := table.router.Finalize(table.canary) + + if table.shouldError && err == nil { + t.Error("Should have errored") + } else if !table.shouldError && err != nil { + t.Errorf("Shouldn't error but did %s", err) + } + + svc, err := table.router.kubeClient.CoreV1().Services(table.canary.Namespace).Get(table.canary.Name, metav1.GetOptions{}) + if err != nil { + if !errors.IsNotFound(err) { + if svc.Spec.Ports[0].Name != "http" { + t.Errorf("Got svc port name %s wanted %s", svc.Spec.Ports[0].Name, "http") + } + + if svc.Spec.Ports[0].Port != 9898 { + t.Errorf("Got svc port %v wanted %v", svc.Spec.Ports[0].Port, 9898) + } + + if table.shouldMutate { + if svc.Spec.Selector["app"] != table.canary.Name { + t.Errorf("Got svc selector %v wanted %v", svc.Spec.Selector["app"], table.canary.Name) + } + } else { + if svc.Spec.Selector["app"] != fmt.Sprintf("%s-primary", table.canary.Name) { + t.Errorf("Got svc selector %v wanted %v", svc.Spec.Selector["app"], fmt.Sprintf("%s-primary", table.canary.Name)) + } + } + } + } + } + +} diff --git a/pkg/router/kubernetes_noop.go b/pkg/router/kubernetes_noop.go index b516cee8..b561c686 100644 --- a/pkg/router/kubernetes_noop.go +++ b/pkg/router/kubernetes_noop.go @@ -16,3 +16,7 @@ func (c *KubernetesNoopRouter) Initialize(_ *flaggerv1.Canary) error { func (c *KubernetesNoopRouter) Reconcile(_ *flaggerv1.Canary) error { return nil } + +func (c *KubernetesNoopRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} diff --git a/pkg/router/nop.go b/pkg/router/nop.go index 4775cfda..91d468b3 100644 --- a/pkg/router/nop.go +++ b/pkg/router/nop.go @@ -22,3 +22,7 @@ func (*NopRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canary } return 100, 0, false, nil } + +func (c *NopRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +} diff --git a/pkg/router/router.go b/pkg/router/router.go index 5a3a414a..91bd63ef 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -2,6 +2,9 @@ package router import flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" +const configAnnotation = "flagger.kubernetes.io/original-configuration" +const kubectlAnnotation = "kubectl.kubernetes.io/last-applied-configuration" + type Interface interface { Reconcile(canary *flaggerv1.Canary) error SetRoutes(canary *flaggerv1.Canary, primaryWeight int, canaryWeight int, mirrored bool) error diff --git a/pkg/router/smi.go b/pkg/router/smi.go index 33282207..a20e4528 100644 --- a/pkg/router/smi.go +++ b/pkg/router/smi.go @@ -224,3 +224,7 @@ func (sr *SmiRouter) getWithConvert(canary *flaggerv1.Canary, host string) (*smi } return ts, nil } + +func (sr *SmiRouter) Finalize(canary *flaggerv1.Canary) error { + return nil +}