Improve forward feature

cmd/stackset-controller/main.go

@@ -48,6 +48,7 @@ var (
 		ConfigMapSupportEnabled     bool
 		SecretSupportEnabled        bool
 		PCSSupportEnabled           bool
+		ForwardSupportEnabled       bool
 	}
 )
 
@@ -71,6 +72,7 @@ func main() {
 	kingpin.Flag("enable-configmap-support", "Enable support for ConfigMaps on StackSets.").Default("false").BoolVar(&config.ConfigMapSupportEnabled)
 	kingpin.Flag("enable-secret-support", "Enable support for Secrets on StackSets.").Default("false").BoolVar(&config.SecretSupportEnabled)
 	kingpin.Flag("enable-pcs-support", "Enable support for PlatformCredentialsSet on StackSets.").Default("false").BoolVar(&config.PCSSupportEnabled)
+	kingpin.Flag("enable-forward-support", "Enable support for skipper traffic forwarding.").Default("false").BoolVar(&config.ForwardSupportEnabled)
 	kingpin.Parse()
 
 	if config.Debug {
@@ -92,6 +94,7 @@ func main() {
 		ConfigMapSupportEnabled:  config.ConfigMapSupportEnabled,
 		SecretSupportEnabled:     config.SecretSupportEnabled,
 		PcsSupportEnabled:        config.PCSSupportEnabled,
+		ForwardSupportEnabled:    config.ForwardSupportEnabled,
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())

controller/stack_resources.go

@@ -44,6 +44,23 @@ func isOwned(ownerReferences []metav1.OwnerReference) (bool, types.UID) {
 func (c *StackSetController) ReconcileStackDeployment(ctx context.Context, stack *zv1.Stack, existing *apps.Deployment, generateUpdated func() *apps.Deployment) error {
 	deployment := generateUpdated()
 
+	// no deployment
+	if deployment == nil {
+		if existing != nil {
+			err := c.client.AppsV1().Deployments(existing.Namespace).Delete(ctx, existing.Name, metav1.DeleteOptions{})
+			if err != nil {
+				return err
+			}
+			c.recorder.Eventf(
+				stack,
+				apiv1.EventTypeNormal,
+				"DeletedDeployment",
+				"Deleted Deployment %s",
+				existing.Name)
+		}
+		return nil
+	}
+
 	// Create new deployment
 	if existing == nil {
 		_, err := c.client.AppsV1().Deployments(deployment.Namespace).Create(ctx, deployment, metav1.CreateOptions{})
@@ -88,7 +105,7 @@ func (c *StackSetController) ReconcileStackHPA(ctx context.Context, stack *zv1.S
 		return err
 	}
 
-	// HPA removed
+	// no HPA
 	if hpa == nil {
 		if existing != nil {
 			err := c.client.AutoscalingV2().HorizontalPodAutoscalers(existing.Namespace).Delete(ctx, existing.Name, metav1.DeleteOptions{})
@@ -100,7 +117,7 @@ func (c *StackSetController) ReconcileStackHPA(ctx context.Context, stack *zv1.S
 				apiv1.EventTypeNormal,
 				"DeletedHPA",
 				"Deleted HPA %s",
-				existing.Namespace)
+				existing.Name)
 		}
 		return nil
 	}

controller/stackset.go

@@ -77,6 +77,7 @@ type StackSetConfig struct {
 	ConfigMapSupportEnabled  bool
 	SecretSupportEnabled     bool
 	PcsSupportEnabled        bool
+	ForwardSupportEnabled    bool
 }
 
 type stacksetEvent struct {
@@ -758,7 +759,7 @@ func (c *StackSetController) ReconcileTrafficSegments(
 
 // CreateCurrentStack creates a new Stack object for the current stack, if needed
 func (c *StackSetController) CreateCurrentStack(ctx context.Context, ssc *core.StackSetContainer) error {
-	newStack, newStackVersion := ssc.NewStack()
+	newStack, newStackVersion := ssc.NewStack(c.config.ForwardSupportEnabled)
 	if newStack == nil {
 		return nil
 	}

pkg/core/stack_resources.go

@@ -196,6 +196,10 @@ func (sc *StackContainer) selector() map[string]string {
 // "zalando.org/forward-backend", the deployment will be set to
 // replicas 1.
 func (sc *StackContainer) GenerateDeployment() *appsv1.Deployment {
+	if sc.TrafficForward() {
+		// during traffic forwarding we do not need a deployment
+		return nil
+	}
 
 	stack := sc.Stack
 
@@ -234,12 +238,6 @@ func (sc *StackContainer) GenerateDeployment() *appsv1.Deployment {
 		Labels:      embeddedCopy.Labels,
 	}
 
-	if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration && *updatedReplicas != 0 {
-		updatedReplicas = wrapReplicas(1)
-		sc.deploymentReplicas = 1
-		sc.stackReplicas = 1
-	}
-
 	deployment := &appsv1.Deployment{
 		ObjectMeta: sc.resourceMeta(),
 		Spec: appsv1.DeploymentSpec{
@@ -270,7 +268,7 @@ func (sc *StackContainer) GenerateHPA() (
 	*autoscaling.HorizontalPodAutoscaler,
 	error,
 ) {
-	if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration {
+	if sc.TrafficForward() {
 		return nil, nil
 	}
 
@@ -477,7 +475,7 @@ func (sc *StackContainer) generateIngress(segment bool) (
 			Rules: rules,
 		},
 	}
-	if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration {
+	if sc.TrafficForward() {
 		// see https://opensource.zalando.com/skipper/kubernetes/ingress-usage/#skipper-ingress-annotations
 		result.Annotations["zalando.org/skipper-backend"] = "forward"
 	}

pkg/core/stack_resources_test.go

@@ -1226,6 +1226,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 		deploymentReplicas int32
 		noTrafficSince     time.Time
 		expectedReplicas   int32
+		expectedDeployment bool
 		maxUnavailable     int
 		maxSurge           int
 		stackAnnotations   map[string]string
@@ -1235,64 +1236,74 @@ func TestStackGenerateDeployment(t *testing.T) {
 			stackReplicas:      0,
 			deploymentReplicas: 3,
 			expectedReplicas:   0,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack scaled down to zero, deployment already scaled down",
 			stackReplicas:      0,
 			deploymentReplicas: 0,
 			expectedReplicas:   0,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack scaled down because it doesn't have traffic, deployment still running",
 			stackReplicas:      3,
 			deploymentReplicas: 3,
 			noTrafficSince:     time.Now().Add(-time.Hour),
 			expectedReplicas:   0,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack scaled down because it doesn't have traffic, deployment already scaled down",
 			stackReplicas:      3,
 			deploymentReplicas: 0,
 			noTrafficSince:     time.Now().Add(-time.Hour),
 			expectedReplicas:   0,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack scaled down to zero, deployment already scaled down",
 			stackReplicas:      0,
 			deploymentReplicas: 0,
 			expectedReplicas:   0,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has zero replicas",
 			stackReplicas:      3,
 			deploymentReplicas: 0,
 			expectedReplicas:   3,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has zero replicas, hpa enabled",
 			hpaEnabled:         true,
 			stackReplicas:      3,
 			deploymentReplicas: 0,
 			expectedReplicas:   3,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has the same amount replicas",
 			stackReplicas:      3,
 			deploymentReplicas: 3,
 			expectedReplicas:   3,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has a different amount of replicas",
 			stackReplicas:      3,
 			deploymentReplicas: 5,
 			expectedReplicas:   3,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has a different amount of replicas, hpa enabled",
 			hpaEnabled:         true,
 			stackReplicas:      3,
 			deploymentReplicas: 5,
 			expectedReplicas:   5,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has zero replicas, prescaling enabled",
@@ -1301,6 +1312,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 			prescalingReplicas: 7,
 			deploymentReplicas: 0,
 			expectedReplicas:   7,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has zero replicas, hpa enabled, prescaling enabled",
@@ -1310,6 +1322,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 			stackReplicas:      3,
 			deploymentReplicas: 0,
 			expectedReplicas:   7,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has the same amount replicas, prescaling enabled",
@@ -1318,6 +1331,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 			prescalingReplicas: 7,
 			deploymentReplicas: 7,
 			expectedReplicas:   7,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has a different amount of replicas, prescaling enabled",
@@ -1326,6 +1340,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 			prescalingReplicas: 7,
 			deploymentReplicas: 5,
 			expectedReplicas:   7,
+			expectedDeployment: true,
 		},
 		{
 			name:               "stack running, deployment has a different amount of replicas, hpa enabled, prescaling enabled",
@@ -1335,20 +1350,23 @@ func TestStackGenerateDeployment(t *testing.T) {
 			stackReplicas:      3,
 			deploymentReplicas: 5,
 			expectedReplicas:   5,
+			expectedDeployment: true,
 		},
 		{
 			name:               "max surge is specified",
 			stackReplicas:      3,
 			deploymentReplicas: 3,
 			expectedReplicas:   3,
 			maxSurge:           10,
+			expectedDeployment: true,
 		},
 		{
 			name:               "max unavailable is specified",
 			stackReplicas:      3,
 			deploymentReplicas: 3,
 			expectedReplicas:   3,
 			maxUnavailable:     10,
+			expectedDeployment: true,
 		},
 		{
 			name:               "max surge and max unavailable are specified",
@@ -1357,10 +1375,12 @@ func TestStackGenerateDeployment(t *testing.T) {
 			expectedReplicas:   3,
 			maxSurge:           1,
 			maxUnavailable:     10,
+			expectedDeployment: true,
 		},
 		{
-			name:            "minReadySeconds should be set",
-			minReadySeconds: 5,
+			name:               "minReadySeconds should be set",
+			minReadySeconds:    5,
+			expectedDeployment: true,
 		},
 		{
 			name:               "cluster migration should scale down deployment to 1",
@@ -1369,7 +1389,7 @@ func TestStackGenerateDeployment(t *testing.T) {
 			stackAnnotations: map[string]string{
 				forwardBackendAnnotation: "fwd-deployment",
 			},
-			expectedReplicas: 1,
+			expectedDeployment: false,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -1431,7 +1451,13 @@ func TestStackGenerateDeployment(t *testing.T) {
 				}
 			}
 			deployment := c.GenerateDeployment()
-			expected := &apps.Deployment{
+			var expected *apps.Deployment
+			if !tc.expectedDeployment {
+				require.Nil(t, deployment)
+				return
+			}
+
+			expected = &apps.Deployment{
 				ObjectMeta: testResourceMeta,
 				Spec: apps.DeploymentSpec{
 					Replicas:        wrapReplicas(tc.expectedReplicas),

pkg/core/stackset.go

@@ -53,8 +53,11 @@ func sanitizeServicePorts(service *zv1.StackServiceSpec) *zv1.StackServiceSpec {
 }
 
 // NewStack returns an (optional) stack that should be created
-func (ssc *StackSetContainer) NewStack() (*StackContainer, string) {
+func (ssc *StackSetContainer) NewStack(forwardSupportEnabled bool) (*StackContainer, string) {
 	_, forwardMigration := ssc.StackSet.ObjectMeta.Annotations[forwardBackendAnnotation]
+	if !forwardSupportEnabled {
+		forwardMigration = false
+	}
 	observedStackVersion := ssc.StackSet.Status.ObservedStackVersion
 	stackVersion := currentStackVersion(ssc.StackSet)
 	stackName := generateStackName(ssc.StackSet, stackVersion)

pkg/core/stackset_test.go

@@ -371,7 +371,7 @@ func TestStackSetNewStack(t *testing.T) {
 				StackContainers:             tc.stacks,
 				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
-			newStack, newStackName := stackset.NewStack()
+			newStack, newStackName := stackset.NewStack(false)
 			require.EqualValues(t, tc.expectedStack, newStack)
 			require.EqualValues(t, tc.expectedStackName, newStackName)
 		})

pkg/core/types.go

@@ -159,6 +159,12 @@ func (sc *StackContainer) HasTraffic() bool {
 }
 
 func (sc *StackContainer) IsReady() bool {
+	if sc.TrafficForward() {
+		// if stack is configured to forward traffic, consider it
+		// ready.
+		return true
+	}
+
 	// Calculate minimum required replicas for the Deployment to be considered ready
 	minRequiredReplicas := int32(math.Ceil(float64(sc.deploymentReplicas) * sc.minReadyPercent))
 
@@ -187,6 +193,11 @@ func (sc *StackContainer) ScaledDown() bool {
 	return !sc.noTrafficSince.IsZero() && time.Since(sc.noTrafficSince) > sc.scaledownTTL
 }
 
+func (sc *StackContainer) TrafficForward() bool {
+	_, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]
+	return clusterMigration
+}
+
 func (sc *StackContainer) Name() string {
 	return sc.Stack.Name
 }