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 +}