adapt traffic command to the possibility of missing non-segment routegroup/ingress (#704)

* adapt traffic command to the possibility of missing non-segment routegroup/ingress

* make sure test cases consider hostnames within cluster domains correctly

* clarify test cases and code with additional comments

* simplify the code a bit by using if/else statement

Author: linki

pkg/core/stack_resources.go

@@ -318,10 +318,10 @@ func (sc *StackContainer) GenerateService() (*v1.Service, error) {
 func (sc *StackContainer) stackHostnames(
 	spec ingressOrRouteGroupSpec,
 	segment bool,
-) ([]string, error) {
+) []string {
 	// The Ingress segment uses the original hostnames
 	if segment {
-		return spec.GetHosts(), nil
+		return spec.GetHosts()
 	}
 	result := sets.NewString()
 
@@ -339,7 +339,7 @@ func (sc *StackContainer) stackHostnames(
 			}
 		}
 	}
-	return result.List(), nil
+	return result.List()
 }
 
 func (sc *StackContainer) GenerateIngress() (*networking.Ingress, error) {
@@ -388,10 +388,7 @@ func (sc *StackContainer) generateIngress(segment bool) (
 		return nil, nil
 	}
 
-	hostnames, err := sc.stackHostnames(sc.ingressSpec, segment)
-	if err != nil {
-		return nil, err
-	}
+	hostnames := sc.stackHostnames(sc.ingressSpec, segment)
 	if len(hostnames) == 0 {
 		return nil, nil
 	}
@@ -481,10 +478,7 @@ func (sc *StackContainer) generateRouteGroup(segment bool) (
 		return nil, nil
 	}
 
-	hostnames, err := sc.stackHostnames(sc.routeGroupSpec, segment)
-	if err != nil {
-		return nil, err
-	}
+	hostnames := sc.stackHostnames(sc.routeGroupSpec, segment)
 	if len(hostnames) == 0 {
 		return nil, nil
 	}

pkg/core/stackset_test.go

@@ -860,14 +860,26 @@ func TestStackUpdateFromResources(t *testing.T) {
 
 	runTest("ingress isn't considered updated if the generation is different", func(t *testing.T, container *StackContainer) {
 		container.Stack.Generation = 11
-		container.ingressSpec = &zv1.StackSetIngressSpec{}
+		container.ingressSpec = &zv1.StackSetIngressSpec{
+			Hosts: []string{"foo.example.org"},
+		}
 		container.Resources.Deployment = deployment(11, 5, 5)
 		container.Resources.Service = service(11)
 		container.Resources.Ingress = ingress(10)
 		container.Resources.IngressSegment = ingress(11)
 		container.updateFromResources()
 		require.EqualValues(t, false, container.resourcesUpdated)
 	})
+	runTest("ingress is considered updated if no hostname matches cluster domain", func(t *testing.T, container *StackContainer) {
+		container.Stack.Generation = 11
+		container.ingressSpec = &zv1.StackSetIngressSpec{}
+		container.Resources.Deployment = deployment(11, 5, 5)
+		container.Resources.Service = service(11)
+		container.Resources.Ingress = nil
+		container.Resources.IngressSegment = ingress(11)
+		container.updateFromResources()
+		require.EqualValues(t, true, container.resourcesUpdated)
+	})
 	runTest("ingress isn't considered updated if it should be gone", func(t *testing.T, container *StackContainer) {
 		container.Stack.Generation = 11
 		container.Resources.Deployment = deployment(11, 5, 5)
@@ -896,14 +908,26 @@ func TestStackUpdateFromResources(t *testing.T) {
 	})
 	runTest("routegroup isn't considered updated if the generation is different", func(t *testing.T, container *StackContainer) {
 		container.Stack.Generation = 11
-		container.routeGroupSpec = &zv1.RouteGroupSpec{}
+		container.routeGroupSpec = &zv1.RouteGroupSpec{
+			Hosts: []string{"foo.example.org"},
+		}
 		container.Resources.Deployment = deployment(11, 5, 5)
 		container.Resources.Service = service(11)
 		container.Resources.RouteGroup = routegroup(10)
 		container.Resources.RouteGroupSegment = routegroup(11)
 		container.updateFromResources()
 		require.EqualValues(t, false, container.resourcesUpdated)
 	})
+	runTest("routegroup is considered updated if no hostname matches cluster domain", func(t *testing.T, container *StackContainer) {
+		container.Stack.Generation = 11
+		container.routeGroupSpec = &zv1.RouteGroupSpec{}
+		container.Resources.Deployment = deployment(11, 5, 5)
+		container.Resources.Service = service(11)
+		container.Resources.RouteGroup = nil
+		container.Resources.RouteGroupSegment = routegroup(11)
+		container.updateFromResources()
+		require.EqualValues(t, true, container.resourcesUpdated)
+	})
 	runTest("routegroup isn't considered updated if it should be gone", func(t *testing.T, container *StackContainer) {
 		container.Stack.Generation = 11
 		container.Resources.Deployment = deployment(11, 5, 5)

pkg/core/test_helpers.go

@@ -22,7 +22,8 @@ type testStackFactory struct {
 func testStack(name string) *testStackFactory {
 	return &testStackFactory{
 		container: &StackContainer{
-			backendPort: &intStrTestPort,
+			clusterDomains: []string{"example.org"},
+			backendPort:    &intStrTestPort,
 			Stack: &zv1.Stack{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:              name,

pkg/core/types.go

@@ -439,7 +439,14 @@ func (sc *StackContainer) updateFromResources() {
 
 	// ingress: ignore if ingress is not set or check if we are up to date
 	if sc.ingressSpec != nil {
-		ingressUpdated = sc.Resources.Ingress != nil && IsResourceUpToDate(sc.Stack, sc.Resources.Ingress.ObjectMeta)
+		// the per-stack ingress must either be present and up-to-date, or not present and not expected.
+		// the per-stack ingress is not expected if the stack has no hostnames matching the cluster domain.
+		if sc.Resources.Ingress != nil {
+			ingressUpdated = IsResourceUpToDate(sc.Stack, sc.Resources.Ingress.ObjectMeta)
+		} else {
+			hostnames := sc.stackHostnames(sc.ingressSpec, false)
+			ingressUpdated = len(hostnames) == 0
+		}
 		ingressSegmentUpdated = sc.Resources.IngressSegment != nil &&
 			IsResourceUpToDate(sc.Stack, sc.Resources.IngressSegment.ObjectMeta)
 	} else {
@@ -450,7 +457,14 @@ func (sc *StackContainer) updateFromResources() {
 
 	// routegroup: ignore if routegroup is not set or check if we are up to date
 	if sc.routeGroupSpec != nil {
-		routeGroupUpdated = sc.Resources.RouteGroup != nil && IsResourceUpToDate(sc.Stack, sc.Resources.RouteGroup.ObjectMeta)
+		// the per-stack route group must either be present and up-to-date, or not present and not expected.
+		// the per-stack route group is not expected if the stack has no hostnames matching the cluster domain.
+		if sc.Resources.RouteGroup != nil {
+			routeGroupUpdated = IsResourceUpToDate(sc.Stack, sc.Resources.RouteGroup.ObjectMeta)
+		} else {
+			hostnames := sc.stackHostnames(sc.routeGroupSpec, false)
+			routeGroupUpdated = len(hostnames) == 0
+		}
 		routeGroupSegmentUpdated = sc.Resources.RouteGroupSegment != nil &&
 			IsResourceUpToDate(
 				sc.Stack,