[release-4.16] OCPBUGS-32887: Delete and recreate canary route to clear spec.host

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)
 	}
 

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)

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 {