diff --git a/pkg/operator/controller/canary/route.go b/pkg/operator/controller/canary/route.go index da587e3037..e1bf40e99d 100644 --- a/pkg/operator/controller/canary/route.go +++ b/pkg/operator/controller/canary/route.go @@ -14,6 +14,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + crclient "sigs.k8s.io/controller-runtime/pkg/client" ) // ensureCanaryRoute ensures the canary route exists @@ -75,6 +77,21 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e return false, nil } + if len(updated.Spec.Host) == 0 && len(current.Spec.Host) != 0 { + // Attempts to clear spec.host may be ignored. Thus, to clear + // spec.host, it is necessary to delete and recreate the route. + log.Info("deleting and recreating the canary route to clear spec.host", "namespace", current.Namespace, "name", current.Name, "old spec.host", current.Spec.Host) + foreground := metav1.DeletePropagationForeground + deleteOptions := crclient.DeleteOptions{PropagationPolicy: &foreground} + if _, err := r.deleteCanaryRoute(current, &deleteOptions); err != nil { + return false, err + } + if err := r.createCanaryRoute(desired); err != nil { + return false, err + } + return true, nil + } + // Diff before updating because the client may mutate the object. diff := cmp.Diff(current, updated, cmpopts.EquateEmpty()) if err := r.client.Update(context.TODO(), updated); err != nil { @@ -85,9 +102,9 @@ func (r *reconciler) updateCanaryRoute(current, desired *routev1.Route) (bool, e } // deleteCanaryRoute deletes a given route -func (r *reconciler) deleteCanaryRoute(route *routev1.Route) (bool, error) { +func (r *reconciler) deleteCanaryRoute(route *routev1.Route, options *crclient.DeleteOptions) (bool, error) { - if err := r.client.Delete(context.TODO(), route); err != nil { + if err := r.client.Delete(context.TODO(), route, options); err != nil { return false, fmt.Errorf("failed to delete canary route %s/%s: %v", route.Namespace, route.Name, err) } diff --git a/test/e2e/all_test.go b/test/e2e/all_test.go index 2d81b5b7d0..3e670051cb 100644 --- a/test/e2e/all_test.go +++ b/test/e2e/all_test.go @@ -111,6 +111,7 @@ func TestAll(t *testing.T) { t.Run("TestRouterCompressionOperation", TestRouterCompressionOperation) t.Run("TestUpdateDefaultIngressControllerSecret", TestUpdateDefaultIngressControllerSecret) t.Run("TestCanaryRoute", TestCanaryRoute) + t.Run("TestCanaryRouteClearsSpecHost", TestCanaryRouteClearsSpecHost) t.Run("TestRouteHTTP2EnableAndDisableIngressConfig", TestRouteHTTP2EnableAndDisableIngressConfig) t.Run("TestRouteHardStopAfterEnableOnIngressConfig", TestRouteHardStopAfterEnableOnIngressConfig) t.Run("TestRouteHardStopAfterEnableOnIngressControllerHasPriorityOverIngressConfig", TestRouteHardStopAfterEnableOnIngressControllerHasPriorityOverIngressConfig) diff --git a/test/e2e/canary_test.go b/test/e2e/canary_test.go index ae9160577b..00a46ca863 100644 --- a/test/e2e/canary_test.go +++ b/test/e2e/canary_test.go @@ -6,6 +6,7 @@ package e2e import ( "bufio" "context" + "fmt" "strings" "testing" "time" @@ -124,6 +125,64 @@ func TestCanaryRoute(t *testing.T) { } } +// TestCanaryRouteClearsSpecHost verifies that the operator clears the canary +// route's spec.host field (which requires deleting and recreating the route) if +// spec.host gets set. +// +// This is a serial test because it modifies the canary route. +func TestCanaryRouteClearsSpecHost(t *testing.T) { + t.Log("Waiting for the default IngressController to be available...") + if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil { + t.Fatal(err) + } + + var canaryRoute routev1.Route + canaryRouteName := controller.CanaryRouteName() + t.Log("Getting the canary route...") + if err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) { + if err := kclient.Get(context.TODO(), canaryRouteName, &canaryRoute); err != nil { + t.Log(err) + return false, nil + } + + return true, nil + }); err != nil { + t.Fatal(err) + } + + if v := canaryRoute.Spec.Host; len(v) != 0 { + t.Fatalf("Expected canary route to have empty spec.host, found %q.", v) + } + + const bogusHost = "foo.bar" + canaryRoute.Spec.Host = bogusHost + t.Log("Setting a bogus spec.host...") + if err := kclient.Update(context.TODO(), &canaryRoute); err != nil { + t.Fatal(err) + } + + t.Log("Waiting for the operator to clear spec.host...") + if err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) { + if err := kclient.Get(context.TODO(), canaryRouteName, &canaryRoute); err != nil { + t.Log(err) + return false, nil + } + + switch v := canaryRoute.Spec.Host; v { + case bogusHost: + t.Log("The operator has not yet cleared spec.host.") + return false, nil + case "": + t.Log("The operator has cleared spec.host.") + return true, nil + default: + return true, fmt.Errorf("found unexpected spec.host: %q", v) + } + }); err != nil { + t.Fatal(err) + } +} + // buildCanaryCurlPod returns a pod definition for a pod with the given name and image // and in the given namespace that curls the specified route via the route's hostname. func buildCanaryCurlPod(name, namespace, image, host string) *corev1.Pod {