NE-1273: Add a watch to the ingress operator so it will recreate the gwapi crds

.gitignore

@@ -105,5 +105,8 @@ tags
 !.vscode/extensions.json
 .history
 
+# idea
+.idea
+
 
 # End of https://www.gitignore.io/api/go,vim,emacs,visualstudiocode

pkg/operator/controller/gatewayapi/controller.go

@@ -15,6 +15,7 @@ import (
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -51,6 +52,24 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	if err := c.Watch(source.Kind(operatorCache, &configv1.FeatureGate{}), &handler.EnqueueRequestForObject{}, clusterNamePredicate); err != nil {
 		return nil, err
 	}
+
+	toFeatureGate := func(ctx context.Context, _ client.Object) []reconcile.Request {
+		return []reconcile.Request{{
+			NamespacedName: operatorcontroller.FeatureGateClusterConfigName(),
+		}}
+	}
+
+	// watch for CRDs
+	for i := range managedCRDs {
+		if err = c.Watch(source.Kind(operatorCache, managedCRDs[i]), handler.EnqueueRequestsFromMapFunc(toFeatureGate), predicate.Funcs{
+			CreateFunc:  func(e event.CreateEvent) bool { return false },
+			DeleteFunc:  func(e event.DeleteEvent) bool { return true },
+			UpdateFunc:  func(e event.UpdateEvent) bool { return false },
+			GenericFunc: func(e event.GenericEvent) bool { return false },
+		}); err != nil {
+			return nil, err
+		}
+	}
 	return c, nil
 }
 

test/e2e/gateway_api_test.go

@@ -80,17 +80,24 @@ func TestGatewayAPI(t *testing.T) {
 // testGatewayAPIResources tests that Gateway API Custom Resource Definitions are available.
 // It specifically verifies that when the GatewayAPI feature gate is enabled, that the Gateway API
 // CRDs are created.
+// It also deletes and ensure the CRDs are recreated.
 func testGatewayAPIResources(t *testing.T) {
 	t.Helper()
 	// Make sure all the *.gateway.networking.k8s.io CRDs are available since the FeatureGate is enabled.
 	ensureCRDs(t)
+
+	// Deleting CRDs to ensure they gets recreated again
+	deleteCRDs(t)
+
+	// Make sure all the *.gateway.networking.k8s.io CRDs are available since they should be recreated after manual deletion.
+	ensureCRDs(t)
 }
 
 // testGatewayAPIIstioInstallation tests that once the Gateway API Custom Resource GatewayClass is created, that
 // the following installation operations complete automatically and successfully:
 // - the required Subscription and CatalogSource are created.
 // - the OSSM Istio operator is installed successfully and has status Running and Ready. e.g. istio-operator-9f5c88857-2xfrr  -n openshift-operators
-// - the Istiod proxy is installed successfully and has status Running and Ready.  e.g istiod-openshift-gateway-867bb8d5c7-4z6mp -n openshift-ingress
+// - Istiod is installed successfully and has status Running and Ready.  e.g istiod-openshift-gateway-867bb8d5c7-4z6mp -n openshift-ingress
 // - the SMCP is created successfully (OSSM 2.x).
 func testGatewayAPIIstioInstallation(t *testing.T) {
 	t.Helper()
@@ -99,13 +106,13 @@ func testGatewayAPIIstioInstallation(t *testing.T) {
 		t.Fatalf("failed to find expected Subscription %s: %v", expectedSubscriptionName, err)
 	}
 	if err := assertCatalogSource(t, expectedCatalogSourceNamespace, expectedCatalogSourceName); err != nil {
-		t.Fatalf("failed to find expected CatalogSource %s", expectedCatalogSourceName)
+		t.Fatalf("failed to find expected CatalogSource %s: %v", expectedCatalogSourceName, err)
 	}
 	if err := assertOSSMOperator(t); err != nil {
-		t.Fatal("failed to find expected Istio operator")
+		t.Fatalf("failed to find expected Istio operator: %v", err)
 	}
 	if err := assertIstiodControlPlane(t); err != nil {
-		t.Fatal("failed to find expected Istiod control plane")
+		t.Fatalf("failed to find expected Istiod control plane: %v", err)
 	}
 	// TODO - In OSSM 3.x the configuration object to check will be different.
 	if err := assertSMCP(t); err != nil {
@@ -146,6 +153,17 @@ func ensureCRDs(t *testing.T) {
 	}
 }
 
+// deleteCRDs deletes Gateway API custom resource definitions.
+func deleteCRDs(t *testing.T) {
+	t.Helper()
+	for _, crdName := range crdNames {
+		err := deleteExistingCRD(t, crdName)
+		if err != nil {
+			t.Errorf("failed to delete crd %s: %v", crdName, err)
+		}
+	}
+}
+
 // ensureGatewayObjectCreation tests that gateway class, gateway, and http route objects can be created.
 func ensureGatewayObjectCreation(ns *corev1.Namespace) error {
 	var domain string
@@ -163,7 +181,7 @@ func ensureGatewayObjectCreation(ns *corev1.Namespace) error {
 	if err != nil {
 		return fmt.Errorf("feature gate was enabled, but gateway object could not be created: %v", err)
 	}
-	// The gateway is cleaned up in testGatewayAPIObjects.
+	// The gateway is cleaned up in TestGatewayAPI.
 
 	hostname := names.SimpleNameGenerator.GenerateName("test-hostname-")
 	defaultRoutename = hostname + "." + domain

test/e2e/util_gatewayapi_test.go

@@ -39,7 +39,7 @@ const (
 	openshiftOperatorsNamespace = "openshift-operators"
 	// openshiftIstioOperatorDeploymentName holds the expected istio-operator deployment name.
 	openshiftIstioOperatorDeploymentName = "istio-operator"
-	// openshiftIstiodDeploymentName holds the expected istiod proxy deployment name
+	// openshiftIstiodDeploymentName holds the expected istiod deployment name
 	openshiftIstiodDeploymentName = "istiod-openshift-gateway"
 	// openshiftSMCPName holds the expected OSSM ServiceMeshControlPlane name
 	openshiftSMCPName = "openshift-gateway"
@@ -103,19 +103,63 @@ func assertCrdExists(t *testing.T, crdname string) (string, error) {
 	return crdVersion, err
 }
 
+// deleteExistingCRD deletes if the CRD of the given name exists and returns an error if not.
+func deleteExistingCRD(t *testing.T, crdName string) error {
+	t.Helper()
+	crd := &apiextensionsv1.CustomResourceDefinition{}
+	newCRD := &apiextensionsv1.CustomResourceDefinition{}
+	name := types.NamespacedName{Namespace: "", Name: crdName}
+
+	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 30*time.Second, false, func(context context.Context) (bool, error) {
+		if err := kclient.Get(context, name, crd); err != nil {
+			t.Logf("failed to get crd %s: %v", name, err)
+			return false, nil
+		}
+		return true, nil
+	})
+	if err != nil {
+		t.Errorf("failed to get crd %s: %v", name, err)
+		return err
+	}
+	// deleting CRD.
+	err = kclient.Delete(context.Background(), crd)
+	if err != nil {
+		t.Errorf("failed to delete crd %s: %v", name, err)
+		return err
+	}
+	err = wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(ctx context.Context) (bool, error) {
+		if err := kclient.Get(ctx, name, newCRD); err != nil {
+			if kerrors.IsNotFound(err) {
+				return true, nil
+			}
+			t.Logf("failed to delete gatewayAPI CRD %s: %v", crdName, err)
+			return false, nil
+		}
+		// if new CRD got recreated while the poll ensures the CRD is deleted.
+		if newCRD != nil && newCRD.UID != crd.UID {
+			return true, nil
+		}
+		t.Logf("crd %s still exists", crdName)
+		return false, nil
+	})
+	if err != nil {
+		return fmt.Errorf("timed out waiting for gatewayAPI CRD %s to be deleted: %v", crdName, err)
+	}
+	t.Logf("deleted crd %s", crdName)
+	return nil
+}
+
 // createHttpRoute checks if the HTTPRoute can be created.
 // If it can't an error is returned.
-func createHttpRoute(namespace, routename, parentnamespace, hostname, backendrefname string, gateway *gwapi.Gateway) (*gwapi.HTTPRoute, error) {
-	// Just in case gateway creation failed, supply a fake gateway name.
-	name := "NONE"
-	if gateway != nil {
-		name = gateway.Name
+func createHttpRoute(namespace, routeName, parentNamespace, hostname, backendRefname string, gateway *gwapi.Gateway) (*gwapi.HTTPRoute, error) {
+	if gateway == nil {
+		return nil, errors.New("unable to create httpRoute, no gateway available")
 	}
 
 	// Create the backend (service and pod) needed for the route to have resolvedRefs=true.
 	// The http route, service, and pod are cleaned up when the namespace is automatically deleted.
 	// buildEchoPod builds a pod that listens on port 8080.
-	echoPod := buildEchoPod(backendrefname, namespace)
+	echoPod := buildEchoPod(backendRefname, namespace)
 	if err := kclient.Create(context.TODO(), echoPod); err != nil {
 		return nil, fmt.Errorf("failed to create pod %s/%s: %v", namespace, echoPod.Name, err)
 	}
@@ -125,10 +169,10 @@ func createHttpRoute(namespace, routename, parentnamespace, hostname, backendref
 		return nil, fmt.Errorf("failed to create service %s/%s: %v", echoService.Namespace, echoService.Name, err)
 	}
 
-	httpRoute := buildHTTPRoute(routename, namespace, name, parentnamespace, hostname, backendrefname)
+	httpRoute := buildHTTPRoute(routeName, namespace, gateway.Name, parentNamespace, hostname, backendRefname)
 	if err := kclient.Create(context.TODO(), httpRoute); err != nil {
 		if kerrors.IsAlreadyExists(err) {
-			name := types.NamespacedName{Namespace: namespace, Name: routename}
+			name := types.NamespacedName{Namespace: namespace, Name: routeName}
 			if err = kclient.Get(context.TODO(), name, httpRoute); err == nil {
 				return httpRoute, nil
 			} else {
@@ -203,23 +247,23 @@ func buildGateway(name, namespace, gcname, fromNs, domain string) *gwapi.Gateway
 }
 
 // buildHTTPRoute initializes the HTTPRoute and returns its address.
-func buildHTTPRoute(routename, namespace, parentgateway, parentnamespace, hostname, backendrefname string) *gwapi.HTTPRoute {
-	parentns := gwapi.Namespace(parentnamespace)
+func buildHTTPRoute(routeName, namespace, parentgateway, parentNamespace, hostname, backendRefname string) *gwapi.HTTPRoute {
+	parentns := gwapi.Namespace(parentNamespace)
 	parent := gwapi.ParentReference{Name: gwapi.ObjectName(parentgateway), Namespace: &parentns}
 	port := gwapi.PortNumber(defaultPortNumber)
 	rule := gwapi.HTTPRouteRule{
 		BackendRefs: []gwapi.HTTPBackendRef{{
 			BackendRef: gwapi.BackendRef{
 				BackendObjectReference: gwapi.BackendObjectReference{
-					Name: gwapi.ObjectName(backendrefname),
+					Name: gwapi.ObjectName(backendRefname),
 					Port: &port,
 				},
 			},
 		}},
 	}
 
 	return &gwapi.HTTPRoute{
-		ObjectMeta: metav1.ObjectMeta{Name: routename, Namespace: namespace},
+		ObjectMeta: metav1.ObjectMeta{Name: routeName, Namespace: namespace},
 		Spec: gwapi.HTTPRouteSpec{
 			CommonRouteSpec: gwapi.CommonRouteSpec{ParentRefs: []gwapi.ParentReference{parent}},
 			Hostnames:       []gwapi.Hostname{gwapi.Hostname(hostname)},
@@ -310,7 +354,7 @@ func assertIstiodControlPlane(t *testing.T) error {
 	}
 	pod := podlist.Items[0]
 	if pod.Status.Phase != corev1.PodRunning {
-		return fmt.Errorf("Istiod proxy failure: pod %s is not running, it is %v", pod.Name, pod.Status.Phase)
+		return fmt.Errorf("Istiod failure: pod %s is not running, it is %v", pod.Name, pod.Status.Phase)
 	}
 
 	t.Logf("found istiod pod %s/%s to be %s", pod.Namespace, pod.Name, pod.Status.Phase)
@@ -333,7 +377,7 @@ func assertGatewayClassSuccessful(t *testing.T, name string) (*gwapi.GatewayClas
 			return false, nil
 		}
 		for _, condition := range gwc.Status.Conditions {
-			if condition.Type == string(gwapi.GatewayClassReasonAccepted) {
+			if condition.Type == string(gwapi.GatewayClassConditionStatusAccepted) {
 				recordedConditionMsg = condition.Message
 				if condition.Status == metav1.ConditionTrue {
 					return true, nil
@@ -344,7 +388,7 @@ func assertGatewayClassSuccessful(t *testing.T, name string) (*gwapi.GatewayClas
 		return false, nil
 	})
 	if err != nil {
-		return nil, fmt.Errorf("gateway class %s not %v, last recorded status message: %s", name, gwapi.GatewayClassReasonAccepted, recordedConditionMsg)
+		return nil, fmt.Errorf("gateway class %s not %v, last recorded status message: %s", name, gwapi.GatewayClassConditionStatusAccepted, recordedConditionMsg)
 	}
 
 	t.Logf("gateway class %s successful", name)
@@ -366,7 +410,7 @@ func assertGatewaySuccessful(t *testing.T, namespace, name string) (*gwapi.Gatew
 			return false, nil
 		}
 		for _, condition := range gw.Status.Conditions {
-			if condition.Type == string(gwapi.GatewayClassReasonAccepted) { // there is no GatewayReasonAccepted!
+			if condition.Type == string(gwapi.GatewayClassConditionStatusAccepted) { // TODO: Use GatewayConditionAccepted when updating to v1.
 				recordedConditionMsg = condition.Message
 				if condition.Status == metav1.ConditionTrue {
 					t.Logf("found gateway %s/%s as Accepted", namespace, name)
@@ -377,7 +421,7 @@ func assertGatewaySuccessful(t *testing.T, namespace, name string) (*gwapi.Gatew
 		return false, nil
 	})
 	if err != nil {
-		return nil, fmt.Errorf("gateway %s not %v, last recorded status message: %s", name, gwapi.GatewayClassReasonAccepted, recordedConditionMsg)
+		return nil, fmt.Errorf("gateway %s not %v, last recorded status message: %s", name, gwapi.GatewayClassConditionStatusAccepted, recordedConditionMsg)
 	}
 
 	return gw, nil
@@ -601,10 +645,8 @@ func assertDNSRecord(t *testing.T, recordName types.NamespacedName) error {
 					}
 				}
 			}
-		} else {
-			t.Logf("found DNSRecord %s/%s but could not determine its readiness. Retrying...", recordName.Namespace, recordName.Name)
-			return false, nil
 		}
+		t.Logf("found DNSRecord %s/%s but could not determine its readiness. Retrying...", recordName.Namespace, recordName.Name)
 		return false, nil
 	})
 	return err