From ea6af9f825cc005a5ef6b127d24a53974a342702 Mon Sep 17 00:00:00 2001 From: anirudhAgniRedhat Date: Fri, 12 Jul 2024 17:39:11 +0530 Subject: [PATCH 1/2] E2E Fixes from NE-1208: Gateway API E2E Testing https://github.com/openshift/cluster-ingress-operator/pull/1023 --- test/e2e/gateway_api_test.go | 10 ++++----- test/e2e/util_gatewayapi_test.go | 38 ++++++++++++++------------------ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/test/e2e/gateway_api_test.go b/test/e2e/gateway_api_test.go index 37e12266d7..dfa38c47b2 100644 --- a/test/e2e/gateway_api_test.go +++ b/test/e2e/gateway_api_test.go @@ -90,7 +90,7 @@ func testGatewayAPIResources(t *testing.T) { // 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 +99,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 { @@ -163,7 +163,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 diff --git a/test/e2e/util_gatewayapi_test.go b/test/e2e/util_gatewayapi_test.go index d0922e98d0..a6541b724d 100644 --- a/test/e2e/util_gatewayapi_test.go +++ b/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" @@ -105,17 +105,15 @@ func assertCrdExists(t *testing.T, crdname string) (string, error) { // 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 +123,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,15 +201,15 @@ 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, }, }, @@ -219,7 +217,7 @@ func buildHTTPRoute(routename, namespace, parentgateway, parentnamespace, hostna } 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 +308,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 +331,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 +342,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 +364,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 +375,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 +599,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 From e9c4200f1f283e471dbc1fe8c62da8c06f418c96 Mon Sep 17 00:00:00 2001 From: anirudhAgniRedhat Date: Fri, 26 Jul 2024 14:55:25 +0530 Subject: [PATCH 2/2] NE-1273: Add a watch to the ingress operator so it will recreate the gwapi crds and E2E tests --- .gitignore | 3 ++ .../controller/gatewayapi/controller.go | 19 ++++++++ test/e2e/gateway_api_test.go | 18 ++++++++ test/e2e/util_gatewayapi_test.go | 46 +++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/.gitignore b/.gitignore index 6ac9759040..43465c0238 100644 --- a/.gitignore +++ b/.gitignore @@ -105,5 +105,8 @@ tags !.vscode/extensions.json .history +# idea +.idea + # End of https://www.gitignore.io/api/go,vim,emacs,visualstudiocode diff --git a/pkg/operator/controller/gatewayapi/controller.go b/pkg/operator/controller/gatewayapi/controller.go index a4ad6239cb..9bd3d2d84c 100644 --- a/pkg/operator/controller/gatewayapi/controller.go +++ b/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 } diff --git a/test/e2e/gateway_api_test.go b/test/e2e/gateway_api_test.go index dfa38c47b2..d2c8c8146b 100644 --- a/test/e2e/gateway_api_test.go +++ b/test/e2e/gateway_api_test.go @@ -80,10 +80,17 @@ 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 @@ -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 diff --git a/test/e2e/util_gatewayapi_test.go b/test/e2e/util_gatewayapi_test.go index a6541b724d..ac1a3168d1 100644 --- a/test/e2e/util_gatewayapi_test.go +++ b/test/e2e/util_gatewayapi_test.go @@ -103,6 +103,52 @@ 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) {