Stack ingresses: use an explicit cluster domain (#202)

* StackSetContainer: refactor to provide an explicit constructor Signed-off-by: Alexey Ermakov <[email protected]> * Per-stack ingresses: generate using explicitly passed cluster domain Signed-off-by: Alexey Ermakov <[email protected]> * Mark --cluster-domain as required Signed-off-by: Alexey Ermakov <[email protected]> * Allow --cluster-domain to be configured with an envar Signed-off-by: Alexey Ermakov <[email protected]> * Add a test for propagation of clusterDomain to SCs Signed-off-by: Alexey Ermakov <[email protected]> * Fix e2e Signed-off-by: Alexey Ermakov <[email protected]>
zalando-incubator · Feb 26, 2020 · 2baddca · 2baddca
1 parent 2f1a617
commit 2baddca
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 90 deletions.
diff --git a/cmd/stackset-controller/main.go b/cmd/stackset-controller/main.go
@@ -33,6 +33,7 @@ var (
 		Interval                    time.Duration
 		APIServer                   *url.URL
 		MetricsAddress              string
+		ClusterDomain               string
 		NoTrafficScaledownTTL       time.Duration
 		ControllerID                string
 		MigrateTo                   string
@@ -49,6 +50,7 @@ func main() {
 	kingpin.Flag("controller-id", "ID of the controller used to determine ownership of StackSet resources").StringVar(&config.ControllerID)
 	kingpin.Flag("backend-weights-key", "Backend weights annotation key the controller will use to set current traffic values").Default(traffic.DefaultBackendWeightsAnnotationKey).StringVar(&config.BackendWeightsAnnotationKey)
 	kingpin.Flag("migrate-to", "Migrate desired traffic setting from Ingress to StackSet or from StackSet to Ingress").EnumVar(&config.MigrateTo, "ingress", "stackset")
+	kingpin.Flag("cluster-domain", "Main domain of the cluster, used for generating Stack Ingress hostnames").Envar("CLUSTER_DOMAIN").Required().StringVar(&config.ClusterDomain)
 	kingpin.Parse()
 
 	if config.Debug {
@@ -71,6 +73,7 @@ func main() {
 		config.ControllerID,
 		config.MigrateTo,
 		config.BackendWeightsAnnotationKey,
+		config.ClusterDomain,
 		prometheus.DefaultRegisterer,
 		config.Interval,
 	)

diff --git a/controller/stackset.go b/controller/stackset.go
@@ -53,6 +53,7 @@ type StackSetController struct {
 	controllerID                string
 	migrateTo                   string
 	backendWeightsAnnotationKey string
+	clusterDomain               string
 	interval                    time.Duration
 	stacksetEvents              chan stacksetEvent
 	stacksetStore               map[types.UID]zv1.StackSet
@@ -76,7 +77,7 @@ func (ee *eventedError) Error() string {
 }
 
 // NewStackSetController initializes a new StackSetController.
-func NewStackSetController(client clientset.Interface, controllerID, migrateTo, backendWeightsAnnotationKey string, registry prometheus.Registerer, interval time.Duration) (*StackSetController, error) {
+func NewStackSetController(client clientset.Interface, controllerID, migrateTo, backendWeightsAnnotationKey, clusterDomain string, registry prometheus.Registerer, interval time.Duration) (*StackSetController, error) {
 	metricsReporter, err := core.NewMetricsReporter(registry)
 	if err != nil {
 		return nil, err
@@ -88,6 +89,7 @@ func NewStackSetController(client clientset.Interface, controllerID, migrateTo,
 		controllerID:                controllerID,
 		migrateTo:                   migrateTo,
 		backendWeightsAnnotationKey: backendWeightsAnnotationKey,
+		clusterDomain:               clusterDomain,
 		interval:                    interval,
 		stacksetEvents:              make(chan stacksetEvent, 1),
 		stacksetStore:               make(map[types.UID]zv1.StackSet),
@@ -344,23 +346,21 @@ func (c *StackSetController) collectResources() (map[types.UID]*core.StackSetCon
 	stacksets := make(map[types.UID]*core.StackSetContainer, len(c.stacksetStore))
 	for uid, stackset := range c.stacksetStore {
 		stackset := stackset
-		stacksetContainer := &core.StackSetContainer{
-			StackSet:          &stackset,
-			StackContainers:   map[types.UID]*core.StackContainer{},
-			TrafficReconciler: &core.SimpleTrafficReconciler{},
-		}
+
+		reconciler := core.TrafficReconciler(&core.SimpleTrafficReconciler{})
 
 		// use prescaling logic if enabled with an annotation
 		if _, ok := stackset.Annotations[PrescaleStacksAnnotationKey]; ok {
 			resetDelay := defaultResetMinReplicasDelay
 			if resetDelayValue, ok := getResetMinReplicasDelay(stackset.Annotations); ok {
 				resetDelay = resetDelayValue
 			}
-			stacksetContainer.TrafficReconciler = &core.PrescalingTrafficReconciler{
+			reconciler = &core.PrescalingTrafficReconciler{
 				ResetHPAMinReplicasTimeout: resetDelay,
 			}
 		}
 
+		stacksetContainer := core.NewContainer(&stackset, reconciler, c.migrateTo == "stackset", c.backendWeightsAnnotationKey, c.clusterDomain)
 		stacksets[uid] = stacksetContainer
 	}
 
@@ -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.migrateTo == "stackset", c.backendWeightsAnnotationKey)
+	err = container.UpdateFromResources()
 	if err != nil {
 		return err
 	}

diff --git a/controller/test_helpers.go b/controller/test_helpers.go
@@ -9,7 +9,6 @@ import (
 	ssfake "github.com/zalando-incubator/stackset-controller/pkg/client/clientset/versioned/fake"
 	zi "github.com/zalando-incubator/stackset-controller/pkg/client/clientset/versioned/typed/zalando.org/v1"
 	ssunified "github.com/zalando-incubator/stackset-controller/pkg/clientset"
-	"github.com/zalando-incubator/stackset-controller/pkg/traffic"
 	apps "k8s.io/api/apps/v1"
 	autoscaling "k8s.io/api/autoscaling/v2beta1"
 	v1 "k8s.io/api/core/v1"
@@ -40,7 +39,7 @@ func NewTestEnvironment() *testEnvironment {
 		ssClient:  ssfake.NewSimpleClientset(),
 	}
 
-	controller, err := NewStackSetController(client, "", "", traffic.DefaultBackendWeightsAnnotationKey, prometheus.NewPedanticRegistry(), time.Minute)
+	controller, err := NewStackSetController(client, "", "", "", "", prometheus.NewPedanticRegistry(), time.Minute)
 	if err != nil {
 		panic(err)
 	}

diff --git a/e2e/apply/deployment.yaml b/e2e/apply/deployment.yaml
@@ -24,6 +24,7 @@ spec:
         image: "{{{IMAGE}}}"
         args:
           - "--controller-id={{{CONTROLLER_ID}}}"
+          - "--cluster-domain={{{CLUSTER_DOMAIN}}}"
         resources:
           limits:
             cpu: 10m

diff --git a/pkg/core/helpers.go b/pkg/core/helpers.go
@@ -1,9 +1,7 @@
 package core
 
 import (
-	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	zv1 "github.com/zalando-incubator/stackset-controller/pkg/apis/zalando.org/v1"
@@ -50,21 +48,6 @@ func getStackGeneration(resource metav1.ObjectMeta) int64 {
 	return decodedGeneration
 }
 
-// createSubdomain creates a subdomain giving an existing domain by replacing
-// the first section of the domain. E.g. given the domain: my-app.example.org
-// and the subdomain part my-new-app the resulting domain will be
-// my-new-app.example.org.
-func createSubdomain(domain, subdomain string) (string, error) {
-	names := strings.SplitN(domain, ".", 2)
-	if len(names) != 2 {
-		return "", fmt.Errorf("unexpected domain format: %s", domain)
-	}
-
-	names[0] = subdomain
-
-	return strings.Join(names, "."), nil
-}
-
 func wrapTime(time time.Time) *metav1.Time {
 	if time.IsZero() {
 		return nil

diff --git a/pkg/core/stack_resources.go b/pkg/core/stack_resources.go
@@ -266,41 +266,29 @@ func (sc *StackContainer) GenerateIngress() (*extensions.Ingress, error) {
 	result := &extensions.Ingress{
 		ObjectMeta: sc.resourceMeta(),
 		Spec: extensions.IngressSpec{
-			Rules: make([]extensions.IngressRule, 0),
+			Rules: []extensions.IngressRule{
+				{
+					IngressRuleValue: extensions.IngressRuleValue{
+						HTTP: &extensions.HTTPIngressRuleValue{
+							Paths: []extensions.HTTPIngressPath{
+								{
+									Path: sc.ingressSpec.Path,
+									Backend: extensions.IngressBackend{
+										ServiceName: sc.Name(),
+										ServicePort: *sc.backendPort,
+									},
+								},
+							},
+						},
+					},
+					Host: fmt.Sprintf("%s.%s", sc.Name(), sc.clusterDomain),
+				},
+			},
 		},
 	}
 
 	// insert annotations
 	result.Annotations = mergeLabels(result.Annotations, sc.ingressSpec.Annotations)
-
-	rule := extensions.IngressRule{
-		IngressRuleValue: extensions.IngressRuleValue{
-			HTTP: &extensions.HTTPIngressRuleValue{
-				Paths: make([]extensions.HTTPIngressPath, 0),
-			},
-		},
-	}
-
-	path := extensions.HTTPIngressPath{
-		Path: sc.ingressSpec.Path,
-		Backend: extensions.IngressBackend{
-			ServiceName: sc.Name(),
-			ServicePort: *sc.backendPort,
-		},
-	}
-	rule.IngressRuleValue.HTTP.Paths = append(rule.IngressRuleValue.HTTP.Paths, path)
-
-	// create rule per hostname
-	for _, host := range sc.ingressSpec.Hosts {
-		r := rule
-		newHost, err := createSubdomain(host, sc.Name())
-		if err != nil {
-			return nil, err
-		}
-		r.Host = newHost
-		result.Spec.Rules = append(result.Spec.Rules, r)
-	}
-
 	return result, nil
 }
 

diff --git a/pkg/core/stack_resources_test.go b/pkg/core/stack_resources_test.go
@@ -278,10 +278,11 @@ func TestStackGenerateIngress(t *testing.T) {
 				Labels:      map[string]string{"ignored": "label"},
 				Annotations: map[string]string{"ingress": "annotation"},
 			},
-			Hosts: []string{"example.org", "example.com"},
+			Hosts: []string{"foo.example.org", "foo.example.com"},
 			Path:  "example",
 		},
-		backendPort: &backendPort,
+		backendPort:   &backendPort,
+		clusterDomain: "example.org",
 	}
 	ingress, err := c.GenerateIngress()
 	require.NoError(t, err)
@@ -295,23 +296,7 @@ func TestStackGenerateIngress(t *testing.T) {
 		Spec: extensions.IngressSpec{
 			Rules: []extensions.IngressRule{
 				{
-					Host: "foo-v1.org",
-					IngressRuleValue: extensions.IngressRuleValue{
-						HTTP: &extensions.HTTPIngressRuleValue{
-							Paths: []extensions.HTTPIngressPath{
-								{
-									Path: "example",
-									Backend: extensions.IngressBackend{
-										ServiceName: "foo-v1",
-										ServicePort: backendPort,
-									},
-								},
-							},
-						},
-					},
-				},
-				{
-					Host: "foo-v1.com",
+					Host: "foo-v1.example.org",
 					IngressRuleValue: extensions.IngressRuleValue{
 						HTTP: &extensions.HTTPIngressRuleValue{
 							Paths: []extensions.HTTPIngressPath{

diff --git a/pkg/core/stackset_test.go b/pkg/core/stackset_test.go
@@ -326,6 +326,7 @@ func dummyStacksetContainer() *StackSetContainer {
 				Stack: &zv1.Stack{},
 			},
 		},
+		stacksetManagesTraffic:      true,
 		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 }
@@ -353,7 +354,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(true, traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources()
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -397,7 +398,7 @@ func TestStackSetUpdateFromResourcesPopulatesBackendPort(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			c := dummyStacksetContainer()
 			c.StackSet.Spec = tc.spec
-			err := c.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources()
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -451,7 +452,7 @@ func TestStackSetUpdateFromResourcesScaleDown(t *testing.T) {
 				c.StackSet.Spec.Ingress = tc.ingress
 			}
 
-			err := c.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
+			err := c.UpdateFromResources()
 			require.NoError(t, err)
 
 			for _, sc := range c.StackContainers {
@@ -470,6 +471,18 @@ func TestStackSetUpdateFromResourcesScaleDown(t *testing.T) {
 	}
 }
 
+func TestStackSetUpdateFromResourcesClusterDomain(t *testing.T) {
+	c := dummyStacksetContainer()
+	c.clusterDomain = "foo.example.org"
+
+	err := c.UpdateFromResources()
+	require.NoError(t, err)
+
+	for _, sc := range c.StackContainers {
+		require.Equal(t, c.clusterDomain, sc.clusterDomain)
+	}
+}
+
 func TestStackUpdateFromResources(t *testing.T) {
 	runTest := func(name string, testFn func(t *testing.T, container *StackContainer)) {
 		t.Run(name, func(t *testing.T) {
@@ -781,7 +794,7 @@ func TestUpdateTrafficFromStackSet(t *testing.T) {
 				backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 			}
 
-			err := ssc.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
+			err := ssc.UpdateFromResources()
 			require.NoError(t, err)
 			require.True(t, ssc.stacksetManagesTraffic)
 
@@ -813,7 +826,7 @@ func TestStackSetExternalIngressForcesTrafficManagement(t *testing.T) {
 		backendWeightsAnnotationKey: traffic.DefaultBackendWeightsAnnotationKey,
 	}
 
-	err := ssc.UpdateFromResources(true, traffic.DefaultBackendWeightsAnnotationKey)
+	err := ssc.UpdateFromResources()
 	require.NoError(t, err)
 	require.True(t, ssc.stacksetManagesTraffic)
 	require.EqualValues(t, &backendPort, ssc.externalIngressBackendPort)
@@ -880,7 +893,7 @@ func TestUpdateTrafficFromIngress(t *testing.T) {
 				ssc.Ingress.Annotations[traffic.DefaultBackendWeightsAnnotationKey] = tc.actualWeights
 			}
 
-			err := ssc.UpdateFromResources(false, traffic.DefaultBackendWeightsAnnotationKey)
+			err := ssc.UpdateFromResources()
 			require.NoError(t, err)
 			require.False(t, ssc.stacksetManagesTraffic)
 

diff --git a/pkg/core/types.go b/pkg/core/types.go
@@ -59,6 +59,10 @@ type StackSetContainer struct {
 	// which annotation is used, defaults to
 	// traffic.DefaultBackendWeightsAnnotationKey
 	backendWeightsAnnotationKey string
+
+	// clusterDomain stores the main domain name of the cluster; per-stack ingress hostnames
+	// are not generated for names outside of it
+	clusterDomain string
 }
 
 // StackContainer is a container for storing the full state of a Stack
@@ -75,10 +79,11 @@ type StackContainer struct {
 	Resources StackResources
 
 	// Fields from the parent stackset
-	stacksetName string
-	ingressSpec  *zv1.StackSetIngressSpec
-	scaledownTTL time.Duration
-	backendPort  *intstr.IntOrString
+	stacksetName  string
+	ingressSpec   *zv1.StackSetIngressSpec
+	scaledownTTL  time.Duration
+	backendPort   *intstr.IntOrString
+	clusterDomain string
 
 	// Fields from the stack itself, with some defaults applied
 	stackReplicas int32
@@ -172,6 +177,17 @@ type StackResources struct {
 	Ingress    *extensions.Ingress
 }
 
+func NewContainer(stackset *zv1.StackSet, reconciler TrafficReconciler, stacksetManageTraffic bool, backendWeightsAnnotationKey, clusterDomain string) *StackSetContainer {
+	return &StackSetContainer{
+		StackSet:                    stackset,
+		StackContainers:             map[types.UID]*StackContainer{},
+		TrafficReconciler:           reconciler,
+		stacksetManagesTraffic:      stacksetManageTraffic,
+		backendWeightsAnnotationKey: backendWeightsAnnotationKey,
+		clusterDomain:               clusterDomain,
+	}
+}
+
 func (ssc *StackSetContainer) stackByName(name string) *StackContainer {
 	for _, container := range ssc.StackContainers {
 		if container.Name() == name {
@@ -321,10 +337,7 @@ func (ssc *StackSetContainer) updateActualTrafficFromStackSet() error {
 }
 
 // UpdateFromResources populates stack state information (e.g. replica counts or traffic) from related resources
-func (ssc *StackSetContainer) UpdateFromResources(stacksetManageTraffic bool, backendWeightsAnnotationKey string) error {
-	ssc.stacksetManagesTraffic = stacksetManageTraffic
-	ssc.backendWeightsAnnotationKey = backendWeightsAnnotationKey
-
+func (ssc *StackSetContainer) UpdateFromResources() error {
 	if len(ssc.StackContainers) == 0 {
 		return nil
 	}
@@ -354,6 +367,7 @@ func (ssc *StackSetContainer) UpdateFromResources(stacksetManageTraffic bool, ba
 		sc.ingressSpec = ingressSpec
 		sc.backendPort = backendPort
 		sc.scaledownTTL = scaledownTTL
+		sc.clusterDomain = ssc.clusterDomain
 		sc.updateFromResources()
 	}