github/flagger
1
0
Fork 0
mirror of https://github.com/fluxcd/flagger.git synced 2026-04-15 06:57:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

finalizer: refactoring

Browse Source
This commit is contained in:
mathetake
2020-03-29 17:07:18 +09:00
parent 7676918184
commit ef8b6fe9b8
21 changed files with 238 additions and 375 deletions
Download Patch File Download Diff File Expand all files Collapse all files
go.mod
-1
View File
@@ -7,7 +7,6 @@ require (
github.com/aws/aws-sdk-go v1.29.29
github.com/davecgh/go-spew v1.1.1
github.com/google/go-cmp v0.4.0
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.5.1
github.com/stretchr/testify v1.5.1
go.uber.org/zap v1.14.1
pkg/canary/daemonset_controller.go
+1 -2
View File
@@ -299,9 +299,8 @@ func (c *DaemonSetController) HaveDependenciesChanged(cd *flaggerv1.Canary) (boo
//Finalize scale the reference instance from zero
func (c *DaemonSetController) Finalize(cd *flaggerv1.Canary) error {
if err := c.ScaleFromZero(cd); err != nil {
return err
return fmt.Errorf("ScaleFromZero failed: %w", err)
}
return nil
}
pkg/canary/daemonset_controller_test.go
-1
View File
@@ -207,6 +207,5 @@ func TestDaemonSetController_Finalize(t *testing.T) {
require.NoError(t, err)
_, ok := dep.Spec.Template.Spec.NodeSelector["flagger.app/scale-to-zero"]
assert.False(t, ok)
}
pkg/canary/deployment_controller.go
+26 -32
View File
@@ -179,27 +179,6 @@ 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
@@ -403,33 +382,48 @@ func (c *DeploymentController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bo
// update the reference deployment replicas to the primary replicas
func (c *DeploymentController) Finalize(cd *flaggerv1.Canary) error {
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
//Get ref deployment
// get ref deployment
refDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(cd.Spec.TargetRef.Name, metav1.GetOptions{})
if err != nil {
return err
return fmt.Errorf("deplyoment %s.%s get query error: %w", cd.Spec.TargetRef.Name, cd.Namespace, err)
}
//2. Get primary if possible, if not scale from zero
// get primary if possible, if not scale from zero
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
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 fmt.Errorf("ScaleFromZero failed: %w", err)
}
return nil
}
return err
return fmt.Errorf("deplyoment %s.%s get query error: %w", primaryName, cd.Namespace, err)
}
//3. If both ref and primary present update the replicas of the ref to match the primary
// 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
if err := c.Scale(cd, int32Default(primaryDep.Spec.Replicas)); err != nil {
return err
// set the replicas value on the original reference deployment
if err := c.scale(cd, int32Default(primaryDep.Spec.Replicas)); err != nil {
return fmt.Errorf("scale failed: %w", err)
}
}
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 {
return fmt.Errorf("deployment %s.%s query error: %w", 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: %w", depCopy.GetName(), depCopy.Namespace, replicas, err)
}
return nil
}
pkg/canary/deployment_controller_test.go
+9 -17
View File
@@ -187,32 +187,24 @@ func TestDeploymentController_Finalize(t *testing.T) {
mocks := newDeploymentFixture()
for _, tc := range []struct {
mocks deploymentControllerFixture
callInitialize bool
shouldError bool
expectedReplicas int32
canary *flaggerv1.Canary
mocks deploymentControllerFixture
callInitialize bool
canary *flaggerv1.Canary
}{
// primary not found returns error
{mocks, false, false, 1, mocks.canary},
{mocks, false, mocks.canary},
// happy path
{mocks, true, false, 1, mocks.canary},
{mocks, true, mocks.canary},
} {
if tc.callInitialize {
mocks.initializeCanary(t)
}
err := mocks.controller.Finalize(tc.canary)
if tc.shouldError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
require.NoError(t, err)
if tc.expectedReplicas > 0 {
c, err := mocks.kubeClient.AppsV1().Deployments(mocks.canary.Namespace).Get(mocks.canary.Name, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, tc.expectedReplicas, *c.Spec.Replicas)
}
c, err := mocks.kubeClient.AppsV1().Deployments(mocks.canary.Namespace).Get(mocks.canary.Name, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, int32(1), *c.Spec.Replicas)
}
}
pkg/canary/service_controller.go
+1 -1
View File
@@ -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(_ *flaggerv1.Canary) error {
return nil
}
pkg/canary/status.go
+4 -4
View File
@@ -56,7 +56,7 @@ func setStatusFailedChecks(flaggerClient clientset.Interface, cd *flaggerv1.Cana
name, ns := cd.GetName(), cd.GetNamespace()
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
cd, err = flaggerClient.FlaggerV1beta1().Canaries(name).Get(ns, metav1.GetOptions{})
cd, err = flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
@@ -82,7 +82,7 @@ func setStatusWeight(flaggerClient clientset.Interface, cd *flaggerv1.Canary, va
name, ns := cd.GetName(), cd.GetNamespace()
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
cd, err = flaggerClient.FlaggerV1beta1().Canaries(cd.Namespace).Get(cd.GetName(), metav1.GetOptions{})
cd, err = flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
@@ -108,7 +108,7 @@ func setStatusIterations(flaggerClient clientset.Interface, cd *flaggerv1.Canary
name, ns := cd.GetName(), cd.GetNamespace()
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
cd, err = flaggerClient.FlaggerV1beta1().Canaries(cd.Namespace).Get(cd.GetName(), metav1.GetOptions{})
cd, err = flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
@@ -136,7 +136,7 @@ func setStatusPhase(flaggerClient clientset.Interface, cd *flaggerv1.Canary, pha
name, ns := cd.GetName(), cd.GetNamespace()
err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if !firstTry {
cd, err = flaggerClient.FlaggerV1beta1().Canaries(cd.Namespace).Get(cd.GetName(), metav1.GetOptions{})
cd, err = flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
pkg/controller/controller.go
+19 -19
View File
@@ -128,19 +128,19 @@ 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
} else if !newCanary.DeletionTimestamp.IsZero() && hasFinalizer(&newCanary) ||
!hasFinalizer(&newCanary) && 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 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)
err := ctrl.removeFinalizer(&newCanary)
if err != nil {
ctrl.logger.Warnf("Failed to remove finalizers for %s.%s", oldCanary.Name, oldCanary.Namespace)
ctrl.logger.Warnf("Failed to remove finalizers for %s.%s: %v", oldCanary.Name, oldCanary.Namespace, err)
return
}
}
@@ -232,27 +232,27 @@ func (c *Controller) syncHandler(key string) error {
return nil
}
//Finalize if canary has been marked for deletion and revert is desired
// 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) {
// If finalizers have been previously removed proceed
if !hasFinalizer(cd) {
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)
return fmt.Errorf("unable to finalize to canary %s.%s error: %w", 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)
// Remove finalizer from Canary
if err := c.removeFinalizer(cd); err != nil {
return fmt.Errorf("unable to remove finalizer for canary %s.%s: %w", cd.Name, cd.Namespace, err)
}
//record event
// 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)
@@ -276,10 +276,10 @@ 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)
// If opt in for revertOnDeletion add finalizer if not present
if cd.Spec.RevertOnDeletion && !hasFinalizer(cd) {
if err := c.addFinalizer(cd); err != nil {
return fmt.Errorf("unable to add finalizer to canary %s.%s: %w", cd.Name, cd.Namespace, err)
}
}
pkg/controller/finalizer.go
+72 -122
View File
@@ -3,215 +3,165 @@ package controller
import (
"fmt"
ex "github.com/pkg/errors"
flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1"
"github.com/weaveworks/flagger/pkg/canary"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/util/retry"
flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1"
)
const finalizer = "finalizer.flagger.app"
func (c *Controller) finalize(old interface{}) error {
var r *flaggerv1.Canary
var ok bool
//Ensure interface is a canary
if r, ok = old.(*flaggerv1.Canary); !ok {
c.logger.Warnf("Received unexpected object: %v", old)
return nil
r, ok := old.(*flaggerv1.Canary)
if !ok {
return fmt.Errorf("received unexpected object: %v", old)
}
_, err := c.flaggerClient.FlaggerV1beta1().Canaries(r.Namespace).Get(r.Name, metav1.GetOptions{})
if err != nil {
c.logger.With("canary", fmt.Sprintf("%s.%s", r.Name, r.Namespace)).
Errorf("Canary %s.%s not found nothing to finalize", r.Name, r.Namespace)
return nil
return fmt.Errorf("get query error: %w", err)
}
//Retrieve a controller
// Retrieve a controller
canaryController := c.canaryFactory.Controller(r.Spec.TargetRef.Kind)
//Set the status to terminating if not already in that state
// Set the status to terminating if not already in that state
if r.Status.Phase != flaggerv1.CanaryPhaseTerminating {
if err := canaryController.SetStatusPhase(r, flaggerv1.CanaryPhaseTerminating); err != nil {
c.logger.Infof("Failed to update status to finalizing %s", err)
return err
return fmt.Errorf("failed to update status: %w", err)
}
//record event
// record event
c.recordEventInfof(r, "Terminating canary %s.%s", r.Name, r.Namespace)
}
err = c.revertTargetRef(canaryController, r)
err = canaryController.Finalize(r)
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.Spec.TargetRef.Kind)
return nil
}
c.logger.Debugf("%s.%s failed due to %s", r.Name, r.Namespace, err)
return err
} else {
//Ensure that targetRef has met a ready state
c.logger.Infof("Checking is canary is ready %s.%s", r.Name, r.Namespace)
ready, err := canaryController.IsCanaryReady(r)
if err != nil && ready {
return fmt.Errorf("%s.%s has not reached ready state during finalizing", r.Name, r.Namespace)
}
return fmt.Errorf("failed to revert target: %w", err)
}
c.logger.Infof("%s.%s kind %s reverted", r.Name, r.Namespace, r.Spec.TargetRef.Kind)
// Ensure that targetRef has met a ready state
c.logger.Infof("Checking is canary is ready %s.%s", r.Name, r.Namespace)
_, err = canaryController.IsCanaryReady(r)
if err != nil {
return fmt.Errorf("canary not ready during finalizing: %w", err)
}
c.logger.Infof("%s.%s moving forward with router finalizing", r.Name, r.Namespace)
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)
return err
}
//Revert the router
if err := c.revertRouter(r, labelSelector, ports); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
return fmt.Errorf("failed to get metadata for router finalizing: %w", err)
}
c.logger.Infof("%s.%s moving forward with mesh finalizing", r.Name, r.Namespace)
//TODO if I can't revert the mesh continue on?
//Revert the Mesh
// Revert the router
router := c.routerFactory.KubernetesRouter(r.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports)
if err := router.Finalize(r); err != nil {
return fmt.Errorf("failed revert router: %w", err)
}
c.logger.Infof("%s.%s router reverted", r.Name, r.Namespace)
// TODO if I can't revert the mesh continue on?
// Revert the Mesh
if err := c.revertMesh(r); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
return fmt.Errorf("failed to revert mesh: %w", err)
}
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
}
c.logger.Infof("%s.%s kind %s reverted", r.Name, r.Namespace, r.Spec.TargetRef.Kind)
return nil
}
//revertRouter
func (c *Controller) revertRouter(r *flaggerv1.Canary, labelSelector string, ports map[string]int32) error {
router := c.routerFactory.KubernetesRouter(r.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports)
if err := router.Finalize(r); err != nil {
c.logger.Errorf("%s.%s router failed with error %s", r.Name, r.Namespace, err)
return err
}
c.logger.Infof("Service %s.%s reverted", r.Name, r.Namespace)
return nil
}
//revertMesh reverts defined mesh provider based upon the implementation's respective Finalize method.
//If the Finalize method encounters and error that is returned, else revert is considered successful.
// revertMesh reverts defined mesh provider based upon the implementation's respective Finalize method.
// If the Finalize method encounters and error that is returned, else revert is considered successful.
func (c *Controller) revertMesh(r *flaggerv1.Canary) error {
provider := c.meshProvider
if r.Spec.Provider != "" {
provider = r.Spec.Provider
}
//Establish provider
meshRouter := c.routerFactory.MeshRouter(provider)
//Finalize mesh
err := meshRouter.Finalize(r)
if err != nil {
c.logger.Errorf("%s.%s mesh failed with error %s", r.Name, r.Namespace, err)
return err
if err := meshRouter.Finalize(r); err != nil {
return fmt.Errorf("meshRouter.Finlize failed: %w", err)
}
c.logger.Infof("%s.%s mesh provider %s reverted", r.Name, r.Namespace, provider)
return nil
}
//hasFinalizer evaluates the finalizers of a given canary for for existence of a provide finalizer string.
//It returns a boolean, true if the finalizer is found false otherwise.
func hasFinalizer(canary *flaggerv1.Canary, finalizerString string) bool {
currentFinalizers := canary.ObjectMeta.Finalizers
for _, f := range currentFinalizers {
if f == finalizerString {
// hasFinalizer evaluates the finalizers of a given canary for for existence of a provide finalizer string.
// It returns a boolean, true if the finalizer is found false otherwise.
func hasFinalizer(canary *flaggerv1.Canary) bool {
for _, f := range canary.ObjectMeta.Finalizers {
if f == finalizer {
return true
}
}
return false
}
//addFinalizer adds 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) addFinalizer(canary *flaggerv1.Canary, finalizerString string) error {
// addFinalizer adds 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) addFinalizer(canary *flaggerv1.Canary) error {
firstTry := true
name, ns := canary.GetName(), canary.GetNamespace()
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{})
if selErr != nil {
return selErr
canary, err = c.flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
}
copy := canary.DeepCopy()
copy.ObjectMeta.Finalizers = append(copy.ObjectMeta.Finalizers, finalizerString)
_, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(copy)
cCopy := canary.DeepCopy()
cCopy.ObjectMeta.Finalizers = append(cCopy.ObjectMeta.Finalizers, finalizer)
_, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(cCopy)
firstTry = false
return
})
if err != nil {
c.logger.Errorf("Failed to add finalizer %s", err)
return ex.Wrap(err, "Add finalizer failed")
return fmt.Errorf("failed after retries: %w", err)
}
return nil
}
//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 {
// 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) error {
firstTry := true
name, ns := canary.GetName(), canary.GetNamespace()
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{})
if selErr != nil {
return selErr
canary, err = c.flaggerClient.FlaggerV1beta1().Canaries(ns).Get(name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("canary %s.%s get query failed: %w", name, ns, err)
}
}
copy := canary.DeepCopy()
newSlice := make([]string, 0)
for _, item := range copy.ObjectMeta.Finalizers {
if item == finalizerString {
continue
cCopy := canary.DeepCopy()
nfs := make([]string, 0, len(cCopy.ObjectMeta.Finalizers))
for _, item := range cCopy.ObjectMeta.Finalizers {
if item != finalizer {
nfs = append(nfs, item)
}
newSlice = append(newSlice, item)
}
if len(newSlice) == 0 {
newSlice = nil
if len(nfs) == 0 {
nfs = nil
}
copy.ObjectMeta.Finalizers = newSlice
_, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(copy)
cCopy.ObjectMeta.Finalizers = nfs
_, err = c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Update(cCopy)
firstTry = false
return
})
if err != nil {
return ex.Wrap(err, "Remove finalizer failed")
return fmt.Errorf("failed after retries: %w", err)
}
return nil
}
pkg/controller/finalizer_test.go
+29 -53
View File
@@ -2,77 +2,56 @@ package controller
import (
"fmt"
"testing"
flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1"
fakeFlagger "github.com/weaveworks/flagger/pkg/client/clientset/versioned/fake"
"github.com/weaveworks/flagger/pkg/logger"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"
k8sTesting "k8s.io/client-go/testing"
"testing"
flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1"
fakeFlagger "github.com/weaveworks/flagger/pkg/client/clientset/versioned/fake"
)
//Test has finalizers
func TestFinalizer_hasFinalizer(t *testing.T) {
c := newDeploymentTestCanary()
require.False(t, hasFinalizer(c))
withFinalizer := newDeploymentTestCanary()
withFinalizer.Finalizers = append(withFinalizer.Finalizers, finalizer)
tables := []struct {
canary *flaggerv1.Canary
result bool
}{
{newDeploymentTestCanary(), false},
{withFinalizer, true},
}
for _, table := range tables {
isPresent := hasFinalizer(table.canary, finalizer)
if isPresent != table.result {
t.Errorf("Result of hasFinalizer returned [%t], but expected [%t]", isPresent, table.result)
}
}
c.Finalizers = append(c.Finalizers, finalizer)
require.True(t, hasFinalizer(c))
}
func TestFinalizer_addFinalizer(t *testing.T) {
mockError := fmt.Errorf("failed to add finalizer to canary %s", "testCanary")
cs := fakeFlagger.NewSimpleClientset(newDeploymentTestCanary())
//prepend so it is evaluated over the catch all *
// prepend so it is evaluated over the catch all *
cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, mockError
return true, nil, fmt.Errorf("failed to add finalizer to canary %s", "testCanary")
})
logger, _ := logger.NewLogger("debug")
m := fixture{
canary: newDeploymentTestCanary(),
flaggerClient: cs,
ctrl: &Controller{
flaggerClient: cs,
logger: logger,
},
logger: logger,
ctrl: &Controller{flaggerClient: cs},
}
tables := []struct {
mock fixture
canary *flaggerv1.Canary
error error
expErr bool
}{
{newDeploymentFixture(nil), newDeploymentTestCanary(), nil},
{m, m.canary, mockError},
{newDeploymentFixture(nil), newDeploymentTestCanary(), false},
{m, m.canary, true},
}
for _, table := range tables {
response := table.mock.ctrl.addFinalizer(table.canary, finalizer)
err := table.mock.ctrl.addFinalizer(table.canary)
if table.error != nil && response == nil {
t.Errorf("Expected an error from addFinalizer, but wasn't present")
} else if table.error == nil && response != nil {
t.Errorf("Expected no error from addFinalizer, but returned error %s", response)
if table.expErr {
require.NotNil(t, err)
} else {
require.Nil(t, err)
}
}
}
func TestFinalizer_removeFinalizer(t *testing.T) {
@@ -80,11 +59,10 @@ func TestFinalizer_removeFinalizer(t *testing.T) {
withFinalizer := newDeploymentTestCanary()
withFinalizer.Finalizers = append(withFinalizer.Finalizers, finalizer)
mockError := fmt.Errorf("failed to add finalizer to canary %s", "testCanary")
cs := fakeFlagger.NewSimpleClientset(newDeploymentTestCanary())
//prepend so it is evaluated over the catch all *
// prepend so it is evaluated over the catch all *
cs.PrependReactor("update", "canaries", func(action k8sTesting.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, mockError
return true, nil, fmt.Errorf("failed to add finalizer to canary %s", "testCanary")
})
m := fixture{
canary: withFinalizer,
@@ -95,20 +73,18 @@ func TestFinalizer_removeFinalizer(t *testing.T) {
tables := []struct {
mock fixture
canary *flaggerv1.Canary
error error
expErr bool
}{
{newDeploymentFixture(nil), withFinalizer, nil},
{m, m.canary, mockError},
{newDeploymentFixture(nil), withFinalizer, false},
{m, m.canary, true},
}
for _, table := range tables {
response := table.mock.ctrl.removeFinalizer(table.canary, finalizer)
if table.error != nil && response == nil {
t.Errorf("Expected an error from addFinalizer, but wasn't present")
} else if table.error == nil && response != nil {
t.Errorf("Expected no error from addFinalizer, but returned error %s", response)
err := table.mock.ctrl.removeFinalizer(table.canary)
if table.expErr {
require.NotNil(t, err)
} else {
require.Nil(t, err)
}
}
}
pkg/router/appmesh.go
+1 -1
View File
@@ -483,7 +483,7 @@ func (ar *AppMeshRouter) gatewayAnnotations(canary *flaggerv1.Canary) map[string
return a
}
func (ar *AppMeshRouter) Finalize(canary *flaggerv1.Canary) error {
func (ar *AppMeshRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/contour.go
+1 -1
View File
@@ -418,6 +418,6 @@ func (cr *ContourRouter) makeLinkerdHeaderValue(canary *flaggerv1.Canary, servic
}
func (cr *ContourRouter) Finalize(canary *flaggerv1.Canary) error {
func (cr *ContourRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/gloo.go
+1 -1
View File
@@ -186,6 +186,6 @@ func (gr *GlooRouter) SetRoutes(
return nil
}
func (gr *GlooRouter) Finalize(canary *flaggerv1.Canary) error {
func (gr *GlooRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/ingress.go
+1 -1
View File
@@ -238,6 +238,6 @@ func (i *IngressRouter) GetAnnotationWithPrefix(suffix string) string {
return fmt.Sprintf("%v/%v", i.annotationsPrefix, suffix)
}
func (i *IngressRouter) Finalize(canary *flaggerv1.Canary) error {
func (i *IngressRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/istio.go
+7 -14
View File
@@ -366,33 +366,27 @@ func (ir *IstioRouter) SetRoutes(
}
func (ir *IstioRouter) Finalize(canary *flaggerv1.Canary) error {
//Need to see if I can get the annotation orig-configuration
// 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
return fmt.Errorf("VirtualService %s.%s get query error: %w", apexName, canary.Namespace, 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",
if err := json.Unmarshal([]byte(a), &storedVS); err != nil {
return fmt.Errorf("VirtualService %s.%s failed to unMarshal annotation %s",
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",
if err := json.Unmarshal([]byte(a), &storedSpec); err != nil {
return fmt.Errorf("VirtualService %s.%s failed to unMarshal annotation %s",
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
@@ -403,9 +397,8 @@ func (ir *IstioRouter) Finalize(canary *flaggerv1.Canary) error {
_, 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 fmt.Errorf("VirtualService %s.%s update error: %w", apexName, canary.Namespace, err)
}
return nil
}
pkg/router/istio_test.go
+20 -40
View File
@@ -5,8 +5,6 @@ import (
"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"
@@ -379,72 +377,54 @@ func TestIstioRouter_Finalize(t *testing.T) {
callReconcile bool
annotation string
}{
//VS not found
// VS not found
{router: router, spec: nil, shouldError: true, createVS: false, canary: mocks.canary, callReconcile: false, annotation: ""},
//No annotation found but still finalizes
// 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
// 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
// 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())
}
require.NoError(t, err)
if vs.Annotations == nil {
vs.Annotations = make(map[string]string)
}
if table.annotation == "flagger" {
switch table.annotation {
case "flagger":
b, err := json.Marshal(table.spec)
if err != nil {
t.Fatal(err.Error())
}
require.NoError(t, err)
vs.Annotations[configAnnotation] = string(b)
} else if table.annotation == "kubectl" {
case "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())
}
require.NoError(t, err)
}
if table.callReconcile {
err = router.Reconcile(table.canary)
if err != nil {
t.Fatal(err.Error())
}
require.NoError(t, err)
}
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)
}
if table.shouldError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
if table.spec != nil {
vs, err := router.istioClient.NetworkingV1alpha3().VirtualServices(table.canary.Namespace).Get(table.canary.Name, metav1.GetOptions{})
require.NoError(t, err)
require.Equal(t, *table.spec, vs.Spec)
}
}
}
pkg/router/kubernetes_default.go
+25 -28
View File
@@ -178,60 +178,57 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam
return nil
}
//Finalize reverts the apex router if not owned by the Flagger controller.
// 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{})
if err != nil {
return err
return fmt.Errorf("service %s.%s get query error: %w", apexName, canary.Namespace, err)
}
//No need to do any reconciliation if the router is owned by the controller
// No need to do any reconciliation if the router is owned by the controller
if hasCanaryOwnerRef, isOwned := c.isOwnedByCanary(svc, canary.Name); !hasCanaryOwnerRef && !isOwned {
//If kubectl annotation is present that will be utilized, else reconcile
// 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",
if err := json.Unmarshal([]byte(a), &storedSvc); err != nil {
return fmt.Errorf("router %s.%s failed to unMarshal annotation %s",
svc.Name, svc.Namespace, kubectlAnnotation)
}
clone := svc.DeepCopy()
clone.Spec.Selector = storedSvc.Spec.Selector
_, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(clone)
if err != nil {
if _, err := c.kubeClient.CoreV1().Services(canary.Namespace).Update(clone); 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
return fmt.Errorf("reconcileService failed: %w", err)
}
}
}
return nil
}
//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.
// 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 true if it is owned by the supplied name.
func (c KubernetesDefaultRouter) isOwnedByCanary(obj interface{}, name string) (bool, bool) {
var object metav1.Object
var ok bool
if object, ok = obj.(metav1.Object); ok {
if ownerRef := metav1.GetControllerOf(object); ownerRef != nil {
if ownerRef.Kind == flaggerv1.CanaryKind {
//And the name exists return true
if name == ownerRef.Name {
return true, true
}
return true, false
}
}
object, ok := obj.(metav1.Object)
if !ok {
return false, false
}
return false, false
ownerRef := metav1.GetControllerOf(object)
if ownerRef == nil {
return false, false
}
if ownerRef.Kind != flaggerv1.CanaryKind {
return false, false
}
return true, ownerRef.Name == name
}
pkg/router/kubernetes_default_test.go
+18 -34
View File
@@ -127,7 +127,7 @@ func TestServiceRouter_isOwnedByCanary(t *testing.T) {
isOwned bool
hasOwnerRef bool
}{
//owned
// owned
{
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
@@ -152,7 +152,7 @@ func TestServiceRouter_isOwnedByCanary(t *testing.T) {
},
}, isOwned: true, hasOwnerRef: true,
},
//Owner ref but kind not Canary
// Owner ref but kind not Canary
{
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
@@ -177,7 +177,7 @@ func TestServiceRouter_isOwnedByCanary(t *testing.T) {
},
}, isOwned: false, hasOwnerRef: false,
},
//Owner ref but name doesn't match
// Owner ref but name doesn't match
{
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
@@ -202,7 +202,7 @@ func TestServiceRouter_isOwnedByCanary(t *testing.T) {
},
}, isOwned: false, hasOwnerRef: true,
},
//No ownerRef
// No ownerRef
{
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
@@ -305,13 +305,13 @@ func TestServiceRouter_Finalize(t *testing.T) {
canary *v1beta1.Canary
shouldMutate bool
}{
//Won't reconcile since it is owned and would be garbage collected
// 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
// Service not found
{router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(), logger: mocks.logger}, callSetupMethods: false, shouldError: true, canary: mocks.canary, shouldMutate: false},
//Not owned
// Not owned
{router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(svc), logger: mocks.logger}, callSetupMethods: false, shouldError: false, canary: mocks.canary, shouldMutate: true},
//Kubectl annotation
// Kubectl annotation
{router: &KubernetesDefaultRouter{kubeClient: fake.NewSimpleClientset(kubectlSvc), logger: mocks.logger}, callSetupMethods: false, shouldError: false, canary: mocks.canary, shouldMutate: true},
}
@@ -319,46 +319,30 @@ func TestServiceRouter_Finalize(t *testing.T) {
if table.callSetupMethods {
err := table.router.Initialize(table.canary)
if err != nil {
t.Fatal(err.Error())
}
require.NoError(t, err)
err = table.router.Reconcile(table.canary)
if err != nil {
t.Fatal(err.Error())
}
require.NoError(t, err)
}
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)
if table.shouldError {
require.Error(t, err)
} else {
require.NoError(t, 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)
}
require.Equal(t, "http", svc.Spec.Ports[0].Name)
require.Equal(t, 9898, svc.Spec.Ports[0].Port)
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)
}
require.Equal(t, table.canary.Name, svc.Spec.Selector["app"])
} 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))
}
require.Equal(t, fmt.Sprintf("%s-primary", table.canary.Name), svc.Spec.Selector["app"])
}
}
}
}
}
pkg/router/kubernetes_noop.go
+1 -1
View File
@@ -17,6 +17,6 @@ func (c *KubernetesNoopRouter) Reconcile(_ *flaggerv1.Canary) error {
return nil
}
func (c *KubernetesNoopRouter) Finalize(canary *flaggerv1.Canary) error {
func (c *KubernetesNoopRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/nop.go
+1 -1
View File
@@ -23,6 +23,6 @@ func (*NopRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight int, canary
return 100, 0, false, nil
}
func (c *NopRouter) Finalize(canary *flaggerv1.Canary) error {
func (c *NopRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}
pkg/router/smi.go
+1 -1
View File
@@ -225,6 +225,6 @@ func (sr *SmiRouter) getWithConvert(canary *flaggerv1.Canary, host string) (*smi
return ts, nil
}
func (sr *SmiRouter) Finalize(canary *flaggerv1.Canary) error {
func (sr *SmiRouter) Finalize(_ *flaggerv1.Canary) error {
return nil
}