Decouple segment computing and generation; configure options to compl…

…etely disable segments.

cmd/e2e/basic_test.go

@@ -862,7 +862,7 @@ func testStacksetUpdate(
 				t,
 				stacksetName,
 				updatedVersion,
-				"TrafficSegment(0.0, 0.0)",
+				"TrafficSegment(0.00, 0.00)",
 				newSubResourceAnnotations,
 			)
 		default:

cmd/stackset-controller/main.go

@@ -41,6 +41,7 @@ var (
 		BackendWeightsAnnotationKey string
 		RouteGroupSupportEnabled    bool
 		TrafficSegmentsEnabled      bool
+		AnnotatedTrafficSegments    bool
 		IngressSourceSwitchTTL      time.Duration
 		ReconcileWorkers            int
 		ConfigMapSupportEnabled     bool
@@ -63,6 +64,10 @@ func main() {
 		"enable-traffic-segments",
 		"Enable support for traffic segments.",
 	).Default("false").BoolVar(&config.TrafficSegmentsEnabled)
+	kingpin.Flag(
+		"annotated-traffic-segments",
+		"Only support traffic segments when annotated. Requires --enable-traffic-segments.",
+	).Default("false").BoolVar(&config.AnnotatedTrafficSegments)
 	kingpin.Flag("ingress-source-switch-ttl", "The ttl before an ingress source is deleted when replaced with another one e.g. switching from RouteGroup to Ingress or vice versa.").
 		Default(defaultIngressSourceSwitchTTL).DurationVar(&config.IngressSourceSwitchTTL)
 	kingpin.Flag("enable-configmap-support", "Enable support for ConfigMaps on StackSets.").Default("false").BoolVar(&config.ConfigMapSupportEnabled)
@@ -93,6 +98,7 @@ func main() {
 		config.Interval,
 		config.RouteGroupSupportEnabled,
 		config.TrafficSegmentsEnabled,
+		config.AnnotatedTrafficSegments,
 		config.ConfigMapSupportEnabled,
 		config.IngressSourceSwitchTTL,
 	)

controller/stackset.go

@@ -62,6 +62,7 @@ type StackSetController struct {
 	HealthReporter              healthcheck.Handler
 	routeGroupSupportEnabled    bool
 	trafficSegmentsEnabled      bool
+	annotatedTrafficSegments    bool
 	ingressSourceSwitchTTL      time.Duration
 	now                         func() string
 	reconcileWorkers            int
@@ -98,6 +99,7 @@ func NewStackSetController(
 	interval time.Duration,
 	routeGroupSupportEnabled bool,
 	trafficSegmentsEnabled bool,
+	annotatedTrafficSegments bool,
 	configMapSupportEnabled bool,
 	ingressSourceSwitchTTL time.Duration,
 ) (*StackSetController, error) {
@@ -120,6 +122,7 @@ func NewStackSetController(
 		HealthReporter:              healthcheck.NewHandler(),
 		routeGroupSupportEnabled:    routeGroupSupportEnabled,
 		trafficSegmentsEnabled:      trafficSegmentsEnabled,
+		annotatedTrafficSegments:    annotatedTrafficSegments,
 		ingressSourceSwitchTTL:      ingressSourceSwitchTTL,
 		configMapSupportEnabled:     configMapSupportEnabled,
 		now:                         now,
@@ -234,7 +237,8 @@ func (c *StackSetController) Run(ctx context.Context) {
 // already present.
 //
 // Only inject the traffic segment annotation if the controller has traffic
-// segments enabled.
+// segments enabled by default, i.e. doesn't matter if the StackSet has the
+// segment annotation.
 func (c *StackSetController) injectSegmentAnnotation(
 	ctx context.Context,
 	stackSet *zv1.StackSet,
@@ -243,6 +247,10 @@ func (c *StackSetController) injectSegmentAnnotation(
 		return false
 	}
 
+	if c.annotatedTrafficSegments {
+		return false
+	}
+
 	if stackSet.Annotations[TrafficSegmentsAnnotationKey] == "true" {
 		return false
 	}
@@ -295,7 +303,9 @@ func (c *StackSetController) collectResources(ctx context.Context) (map[types.UI
 		}
 
 		stacksetContainer := core.NewContainer(&stackset, reconciler, c.backendWeightsAnnotationKey, c.clusterDomains)
-		if stackset.Annotations[TrafficSegmentsAnnotationKey] == "true" {
+		if c.trafficSegmentsEnabled &&
+			stackset.Annotations[TrafficSegmentsAnnotationKey] == "true" {
+
 			stacksetContainer.EnableSegmentTraffic()
 		}
 		stacksets[uid] = stacksetContainer
@@ -712,17 +722,11 @@ func (c *StackSetController) ReconcileStatuses(ctx context.Context, ssc *core.St
 func (c *StackSetController) ReconcileTrafficSegments(
 	ctx context.Context,
 	ssc *core.StackSetContainer,
-) ([]core.TrafficSegment, error) {
+) ([]types.UID, error) {
 	// Compute segments
 	toUpdate, err := ssc.ComputeTrafficSegments()
 	if err != nil {
-		return []core.TrafficSegment{},
-			c.errorEventf(ssc.StackSet, "FailedManageSegments", err)
-	}
-
-	for _, ts := range toUpdate {
-		ssc.StackContainers[ts.GetID()].IngressSegmentToUpdate = ts.IngressSegment
-		ssc.StackContainers[ts.GetID()].RouteGroupSegmentToUpdate = ts.RouteGroupSegment
+		return nil, c.errorEventf(ssc.StackSet, "FailedManageSegments", err)
 	}
 
 	return toUpdate, nil
@@ -1248,8 +1252,6 @@ func (c *StackSetController) ReconcileStackResources(ctx context.Context, ssc *c
 		if err != nil {
 			return c.errorEventf(sc.Stack, "FailedManageIngressSegment", err)
 		}
-
-		sc.IngressSegmentToUpdate = nil
 	}
 
 	if c.routeGroupSupportEnabled {
@@ -1273,8 +1275,6 @@ func (c *StackSetController) ReconcileStackResources(ctx context.Context, ssc *c
 					err,
 				)
 			}
-
-			sc.RouteGroupSegmentToUpdate = nil
 		}
 	}
 	return nil
@@ -1322,7 +1322,7 @@ func (c *StackSetController) ReconcileStackSet(ctx context.Context, container *c
 	// Mark stacks that should be removed
 	container.MarkExpiredStacks()
 
-	segsInOrder := []core.TrafficSegment{}
+	segsInOrder := []types.UID{}
 	// This is to support both central and segment-based traffic.
 	if container.SupportsSegmentTraffic() {
 		// Update traffic segments. Proceed on errors.
@@ -1342,9 +1342,9 @@ func (c *StackSetController) ReconcileStackSet(ctx context.Context, container *c
 
 	// Reconcile stack resources. Proceed on errors.
 	reconciledStacks := map[types.UID]bool{}
-	for _, ts := range segsInOrder {
-		reconciledStacks[ts.GetID()] = true
-		sc := container.StackContainers[ts.GetID()]
+	for _, id := range segsInOrder {
+		reconciledStacks[id] = true
+		sc := container.StackContainers[id]
 		err = c.ReconcileStackResources(ctx, container, sc)
 		if err != nil {
 			err = c.errorEventf(sc.Stack, "FailedManageStack", err)

controller/test_helpers.go

@@ -69,6 +69,7 @@ func NewTestEnvironment() *testEnvironment {
 		true,
 		true,
 		true,
+		true,
 		time.Minute,
 	)
 

e2e/apply/deployment.yaml

@@ -27,6 +27,8 @@ spec:
           - "--cluster-domain={{{CLUSTER_DOMAIN}}}"
           - "--cluster-domain={{{CLUSTER_DOMAIN_INTERNAL}}}"
           - "--enable-routegroup-support"
+          - "--enable-traffic-segments"
+          - "--annotated-traffic-segments"
           - "--ingress-source-switch-ttl=1m"
         resources:
           limits:

pkg/core/stack_resources.go

@@ -24,7 +24,6 @@ const (
 	kindDeployment   = "Deployment"
 
 	SegmentSuffix       = "-traffic-segment"
-	InitialSegment      = "TrafficSegment(0.0, 0.0)"
 	IngressPredicateKey = "zalando.org/skipper-predicate"
 )
 
@@ -34,10 +33,6 @@ type ingressOrRouteGroupSpec interface {
 }
 
 var (
-	initialIngressSegment = map[string]string{
-		IngressPredicateKey: InitialSegment,
-	}
-
 	// set implementation with 0 Byte value
 	selectorLabels = map[string]struct{}{
 		StacksetHeritageLabelKey: {},
@@ -311,12 +306,10 @@ func (sc *StackContainer) stackHostnames(
 	spec ingressOrRouteGroupSpec,
 	segment bool,
 ) ([]string, error) {
-
 	// The Ingress segment uses the original hostnames
 	if segment {
 		return spec.GetHosts(), nil
 	}
-
 	result := sets.NewString()
 
 	// Old-style autogenerated hostnames
@@ -333,7 +326,6 @@ func (sc *StackContainer) stackHostnames(
 			}
 		}
 	}
-
 	return result.List(), nil
 }
 
@@ -345,14 +337,26 @@ func (sc *StackContainer) GenerateIngressSegment() (
 	*networking.Ingress,
 	error,
 ) {
-	switch {
-	case sc.IngressSegmentToUpdate != nil:
-		return sc.IngressSegmentToUpdate, nil
-	case sc.Resources.IngressSegment != nil:
-		return sc.Resources.IngressSegment.DeepCopy(), nil
-	default:
-		return sc.generateIngress(true)
+	res, err := sc.generateIngress(true)
+	if err != nil || res == nil {
+		return res, err
 	}
+
+	if predVal, ok := res.Annotations[IngressPredicateKey]; !ok {
+		res.Annotations = mergeLabels(
+			res.Annotations,
+			map[string]string{IngressPredicateKey: sc.trafficSegment()},
+		)
+	} else {
+		res.Annotations = mergeLabels(
+			res.Annotations,
+			map[string]string{
+				IngressPredicateKey: sc.trafficSegment() + " && " + predVal,
+			},
+		)
+	}
+
+	return res, nil
 }
 
 func (sc *StackContainer) generateIngress(segment bool) (
@@ -416,13 +420,6 @@ func (sc *StackContainer) generateIngress(segment bool) (
 		sc.ingressSpec.GetAnnotations(),
 	)
 
-	if segment {
-		result.Annotations = mergeLabels(
-			result.Annotations,
-			initialIngressSegment,
-		)
-	}
-
 	return result, nil
 }
 
@@ -434,14 +431,19 @@ func (sc *StackContainer) GenerateRouteGroupSegment() (
 	*rgv1.RouteGroup,
 	error,
 ) {
-	switch {
-	case sc.RouteGroupSegmentToUpdate != nil:
-		return sc.RouteGroupSegmentToUpdate, nil
-	case sc.Resources.RouteGroupSegment != nil:
-		return sc.Resources.RouteGroupSegment.DeepCopy(), nil
-	default:
-		return sc.generateRouteGroup(true)
+	res, err := sc.generateRouteGroup(true)
+	if err != nil || res == nil {
+		return res, err
 	}
+
+	segmentedRoutes := []rgv1.RouteGroupRouteSpec{}
+	for _, r := range res.Spec.Routes {
+		r.Predicates = append(r.Predicates, sc.trafficSegment())
+		segmentedRoutes = append(segmentedRoutes, r)
+	}
+	res.Spec.Routes = segmentedRoutes
+
+	return res, nil
 }
 
 func (sc *StackContainer) generateRouteGroup(segment bool) (
@@ -482,16 +484,7 @@ func (sc *StackContainer) generateRouteGroup(segment bool) (
 		},
 	}
 
-	if !segment {
-		result.Spec.Routes = sc.routeGroupSpec.Routes
-	} else {
-		routesWithSegment := []rgv1.RouteGroupRouteSpec{}
-		for _, r := range sc.routeGroupSpec.Routes {
-			r.Predicates = append(r.Predicates, InitialSegment)
-			routesWithSegment = append(routesWithSegment, r)
-		}
-		result.Spec.Routes = routesWithSegment
-	}
+	result.Spec.Routes = sc.routeGroupSpec.Routes
 
 	// validate not overlapping with main backend
 	for _, backend := range sc.routeGroupSpec.AdditionalBackends {
@@ -504,7 +497,7 @@ func (sc *StackContainer) generateRouteGroup(segment bool) (
 		result.Spec.Backends = append(result.Spec.Backends, backend)
 	}
 
-	// sort backends to ensure have a consistent generated RoutGroup resource
+	// sort backends to ensure have a consistent generated RouteGroup resource
 	sort.Slice(result.Spec.Backends, func(i, j int) bool {
 		return result.Spec.Backends[i].Name < result.Spec.Backends[j].Name
 	})
@@ -518,6 +511,14 @@ func (sc *StackContainer) generateRouteGroup(segment bool) (
 	return result, nil
 }
 
+func (sc *StackContainer) trafficSegment() string {
+	return fmt.Sprintf(
+		segmentString,
+		sc.segmentLowerLimit,
+		sc.segmentUpperLimit,
+	)
+}
+
 func (sc *StackContainer) UpdateObjectMeta(objMeta *metav1.ObjectMeta) *metav1.ObjectMeta {
 	metaObj := sc.resourceMeta()
 	objMeta.OwnerReferences = metaObj.OwnerReferences