fix migrations to stackset (#191)

* fix: have only one place for the const definition, such that overrides also work as expected

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

* fix: generate traffic settings on stackset if we do --migrate-to=stackset

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

* pass the decision who manages the traffic via UpdateFromResources() to omit exported member of internal struct as commented

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

* refactor BackendWeightsAnnotationKey to non public field

Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

Author: szuecs

controller/stackset.go

@@ -892,7 +892,7 @@ func (c *StackSetController) ReconcileStackSet(container *core.StackSetContainer
 	}
 
 	// Update statuses from external resources (ingresses, deployments, etc). Abort on errors.
-	err = container.UpdateFromResources(c.backendWeightsAnnotationKey)
+	err = container.UpdateFromResources(c.migrateTo == "stackset", c.backendWeightsAnnotationKey)
 	if err != nil {
 		return err
 	}

pkg/core/stackset.go

@@ -7,6 +7,7 @@ import (
 	"strconv"
 
 	zv1 "github.com/zalando-incubator/stackset-controller/pkg/apis/zalando.org/v1"
+	"github.com/zalando-incubator/stackset-controller/pkg/traffic"
 	corev1 "k8s.io/api/core/v1"
 	extensions "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -56,7 +57,6 @@ func (ssc *StackSetContainer) NewStack() (*StackContainer, string) {
 	observedStackVersion := stackset.Status.ObservedStackVersion
 	stackVersion := currentStackVersion(stackset)
 	stackName := generateStackName(stackset, stackVersion)
-
 	stack := ssc.stackByName(stackName)
 
 	// If the current stack doesn't exist, check that we haven't created it before. We shouldn't recreate
@@ -223,11 +223,11 @@ func (ssc *StackSetContainer) GenerateIngress() (*extensions.Ingress, error) {
 		return nil, err
 	}
 
-	result.Annotations[BackendWeightsAnnotationKey] = string(actualWeightsData)
+	result.Annotations[ssc.backendWeightsAnnotationKey] = string(actualWeightsData)
 	if ssc.stacksetManagesTraffic {
-		delete(result.Annotations, stackTrafficWeightsAnnotationKey)
+		delete(result.Annotations, traffic.StackTrafficWeightsAnnotationKey)
 	} else {
-		result.Annotations[stackTrafficWeightsAnnotationKey] = string(desiredWeightData)
+		result.Annotations[traffic.StackTrafficWeightsAnnotationKey] = string(desiredWeightData)
 	}
 
 	return result, nil

pkg/core/stackset_test.go

@@ -101,7 +101,8 @@ func TestExpiredStacks(t *testing.T) {
 						},
 					},
 				},
-				StackContainers: map[types.UID]*StackContainer{},
+				StackContainers:             map[types.UID]*StackContainer{},
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 			c.StackSet.Spec.StackLifecycle.Limit = &tc.limit
 			for _, stack := range tc.stacks {
@@ -294,8 +295,9 @@ func TestStackSetNewStack(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			stackset := &StackSetContainer{
-				StackSet:        tc.stackset,
-				StackContainers: tc.stacks,
+				StackSet:                    tc.stackset,
+				StackContainers:             tc.stacks,
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 			newStack, newStackName := stackset.NewStack()
 			require.EqualValues(t, tc.expectedStack, newStack)
@@ -324,6 +326,7 @@ func dummyStacksetContainer() *StackSetContainer {
 				Stack: &zv1.Stack{},
 			},
 		},
+		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 }
 
@@ -350,7 +353,7 @@ func TestStackSetUpdateFromResourcesPopulatesIngress(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			c := dummyStacksetContainer()
 			c.StackSet.Spec.Ingress = tc.ingress
-			err := c.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -394,7 +397,7 @@ func TestStackSetUpdateFromResourcesPopulatesBackendPort(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			c := dummyStacksetContainer()
 			c.StackSet.Spec = tc.spec
-			err := c.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -448,7 +451,7 @@ func TestStackSetUpdateFromResourcesScaleDown(t *testing.T) {
 				c.StackSet.Spec.Ingress = tc.ingress
 			}
 
-			err := c.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -789,9 +792,10 @@ func TestUpdateTrafficFromStackSet(t *testing.T) {
 					"v2": stack2,
 					"v3": stack3,
 				},
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 
-			err := ssc.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+			err := ssc.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
 			require.NoError(t, err)
 			require.True(t, ssc.stacksetManagesTraffic)
 
@@ -820,9 +824,10 @@ func TestStackSetExternalIngressForcesTrafficManagement(t *testing.T) {
 		StackContainers: map[types.UID]*StackContainer{
 			"v1": testStack("foo-v1").stack(),
 		},
+		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 
-	err := ssc.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+	err := ssc.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
 	require.NoError(t, err)
 	require.True(t, ssc.stacksetManagesTraffic)
 	require.EqualValues(t, &backendPort, ssc.externalIngressBackendPort)
@@ -879,16 +884,17 @@ func TestUpdateTrafficFromIngress(t *testing.T) {
 					"v2": stack2,
 					"v3": stack3,
 				},
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 
 			if tc.desiredWeights != "" {
-				ssc.Ingress.Annotations[stackTrafficWeightsAnnotationKey] = tc.desiredWeights
+				ssc.Ingress.Annotations[traffic.StackTrafficWeightsAnnotationKey] = tc.desiredWeights
 			}
 			if tc.actualWeights != "" {
-				ssc.Ingress.Annotations[BackendWeightsAnnotationKey] = tc.actualWeights
+				ssc.Ingress.Annotations[traffic.DefaultBackendWeightsAnnotationKey] = tc.actualWeights
 			}
 
-			err := ssc.UpdateFromResources(traffic.DefaultBackendWeightsAnnotationKey)
+			err := ssc.UpdateFromResources(false, traffic.DefaultBackendWeightsAnnotationKey)
 			require.NoError(t, err)
 			require.False(t, ssc.stacksetManagesTraffic)
 
@@ -915,6 +921,7 @@ func TestGenerateStackSetStatus(t *testing.T) {
 			"v4": testStack("v4").stack(),
 			"v5": testStack("v5").ready(3).traffic(20, 10).stack(),
 		},
+		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 
 	expected := &zv1.StackSetStatus{
@@ -998,7 +1005,8 @@ func TestGenerateStackSetTraffic(t *testing.T) {
 					"v4": testStack("v4").stack(),
 					"v5": testStack("v5").ready(3).traffic(20, 10).stack(),
 				},
-				stacksetManagesTraffic: tc.managesTraffic,
+				stacksetManagesTraffic:      tc.managesTraffic,
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 
 			require.Equal(t, tc.expected, c.GenerateStackSetTraffic())
@@ -1059,7 +1067,8 @@ func TestStackSetGenerateIngress(t *testing.T) {
 					"v3": testStack("foo-v3").traffic(0.625, 0.625).stack(),
 					"v4": testStack("foo-v4").traffic(0, 0).stack(),
 				},
-				stacksetManagesTraffic: tc.stacksetManagesTraffic,
+				stacksetManagesTraffic:      tc.stacksetManagesTraffic,
+				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 			ingress, err := c.GenerateIngress()
 			require.NoError(t, err)
@@ -1164,7 +1173,8 @@ func TestStackSetGenerateIngress(t *testing.T) {
 
 func TestStackSetGenerateIngressNone(t *testing.T) {
 	c := &StackSetContainer{
-		StackSet: &zv1.StackSet{},
+		StackSet:                    &zv1.StackSet{},
+		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 	ingress, err := c.GenerateIngress()
 	require.NoError(t, err)

pkg/core/traffic.go

@@ -4,11 +4,6 @@ import (
 	"time"
 )
 
-const (
-	stackTrafficWeightsAnnotationKey = "zalando.org/stack-traffic-weights"
-	BackendWeightsAnnotationKey      = "zalando.org/backend-weights"
-)
-
 type TrafficReconciler interface {
 	// Handle the traffic switching and/or scaling logic.
 	Reconcile(stacks map[string]*StackContainer, currentTimestamp time.Time) error

pkg/core/types.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	zv1 "github.com/zalando-incubator/stackset-controller/pkg/apis/zalando.org/v1"
+	"github.com/zalando-incubator/stackset-controller/pkg/traffic"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscaling "k8s.io/api/autoscaling/v2beta1"
 	v1 "k8s.io/api/core/v1"
@@ -53,6 +54,11 @@ type StackSetContainer struct {
 
 	// Whether the stackset should be authoritative for the traffic, and not the ingress
 	stacksetManagesTraffic bool
+
+	// backendWeightsAnnotationKey to store the runtime decision
+	// which annotation is used, defaults to
+	// traffic.DefaultBackendWeightsAnnotationKey
+	backendWeightsAnnotationKey string
 }
 
 // StackContainer is a container for storing the full state of a Stack
@@ -180,7 +186,7 @@ func (ssc *StackSetContainer) stackByName(name string) *StackContainer {
 
 // updateDesiredTrafficFromIngress updates traffic weights of stack containers from the ingress object
 func (ssc *StackSetContainer) updateDesiredTrafficFromIngress() error {
-	desired, err := ssc.getNormalizedTrafficFromIngress(stackTrafficWeightsAnnotationKey)
+	desired, err := ssc.getNormalizedTrafficFromIngress(traffic.StackTrafficWeightsAnnotationKey)
 	if err != nil {
 		return fmt.Errorf("failed to get current actual Stack traffic weights: %v", err)
 	}
@@ -192,8 +198,8 @@ func (ssc *StackSetContainer) updateDesiredTrafficFromIngress() error {
 	return nil
 }
 
-func (ssc *StackSetContainer) updateActualTrafficFromIngress(backendWeightsAnnotationKey string) error {
-	actual, err := ssc.getNormalizedTrafficFromIngress(backendWeightsAnnotationKey)
+func (ssc *StackSetContainer) updateActualTrafficFromIngress() error {
+	actual, err := ssc.getNormalizedTrafficFromIngress(ssc.backendWeightsAnnotationKey)
 	if err != nil {
 		return fmt.Errorf("failed to get current actual Stack traffic weights: %v", err)
 	}
@@ -318,7 +324,10 @@ func (ssc *StackSetContainer) updateActualTrafficFromStackSet() error {
 }
 
 // UpdateFromResources populates stack state information (e.g. replica counts or traffic) from related resources
-func (ssc *StackSetContainer) UpdateFromResources(backendWeightsAnnotationKey string) error {
+func (ssc *StackSetContainer) UpdateFromResources(stacksetManageTraffic bool, backendWeightsAnnotationKey string) error {
+	ssc.stacksetManagesTraffic = stacksetManageTraffic
+	ssc.backendWeightsAnnotationKey = backendWeightsAnnotationKey
+
 	if len(ssc.StackContainers) == 0 {
 		return nil
 	}
@@ -370,7 +379,7 @@ func (ssc *StackSetContainer) UpdateFromResources(backendWeightsAnnotationKey st
 		}
 
 		// TODO(sszuecs): delete until end of function, if we drop ingress based desired traffic. For step1 we need the fallback but update the stackset status, too
-		return ssc.updateActualTrafficFromIngress(backendWeightsAnnotationKey)
+		return ssc.updateActualTrafficFromIngress()
 	}
 
 	return nil

pkg/traffic/traffic.go

@@ -13,7 +13,7 @@ import (
 
 const (
 	stacksetHeritageLabelKey           = "stackset"
-	stackTrafficWeightsAnnotationKey   = "zalando.org/stack-traffic-weights"
+	StackTrafficWeightsAnnotationKey   = "zalando.org/stack-traffic-weights"
 	DefaultBackendWeightsAnnotationKey = "zalando.org/backend-weights"
 )
 
@@ -62,7 +62,7 @@ func (t *Switcher) Switch(stackset, stack, namespace string, weight float64) ([]
 		annotation := map[string]map[string]map[string]string{
 			"metadata": map[string]map[string]string{
 				"annotations": map[string]string{
-					stackTrafficWeightsAnnotationKey: string(stackWeightsData),
+					StackTrafficWeightsAnnotationKey: string(stackWeightsData),
 				},
 			},
 		}
@@ -139,7 +139,7 @@ func (t *Switcher) getIngressTraffic(name, namespace string, stacks []zv1.Stack)
 	}
 
 	desiredTraffic := make(map[string]float64, len(stacks))
-	if weights, ok := ingress.Annotations[stackTrafficWeightsAnnotationKey]; ok {
+	if weights, ok := ingress.Annotations[StackTrafficWeightsAnnotationKey]; ok {
 		err := json.Unmarshal([]byte(weights), &desiredTraffic)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to get current desired Stack traffic weights: %v", err)