fix: add extra check on servicemesh CRD and svc before proceeding cre…

…ate SMCP CR (#1359) * feat: add extra check on servicemesh CRD and service before proceeding create SMCP CR - when we install operator we might have a race condition: - ossm is not ready to server - dsci already created smcp CR - since we do not know the version of smcp to use - ossm webhook can run into conversion problem since no version specify in smcp CR - test: add more negative test - when CRD does not exist - when service does not exist Signed-off-by: Wen Zhou <[email protected]> --------- Signed-off-by: Wen Zhou <[email protected]>
opendatahub-io · Nov 12, 2024 · e25487a · e25487a
1 parent 61871bb
commit e25487a
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 3 deletions.
diff --git a/pkg/cluster/operator.go b/pkg/cluster/operator.go
@@ -4,9 +4,14 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	ofapiv2 "github.com/operator-framework/api/pkg/operators/v2"
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -67,3 +72,31 @@ func OperatorExists(ctx context.Context, cli client.Client, operatorPrefix strin
 
 	return false, nil
 }
+
+// CustomResourceDefinitionExists checks if a CustomResourceDefinition with the given GVK exists.
+func CustomResourceDefinitionExists(ctx context.Context, cli client.Client, crdGK schema.GroupKind) error {
+	crd := &apiextv1.CustomResourceDefinition{}
+	resourceInterval, resourceTimeout := 2*time.Second, 5*time.Second
+	name := strings.ToLower(fmt.Sprintf("%ss.%s", crdGK.Kind, crdGK.Group)) // we need plural form of the kind
+
+	err := wait.PollUntilContextTimeout(ctx, resourceInterval, resourceTimeout, false, func(ctx context.Context) (bool, error) {
+		err := cli.Get(ctx, client.ObjectKey{Name: name}, crd)
+		if err != nil {
+			if errors.IsNotFound(err) {
+				return false, nil
+			}
+			return false, err
+		}
+
+		for _, condition := range crd.Status.Conditions {
+			if condition.Type == apiextv1.Established {
+				if condition.Status == apiextv1.ConditionTrue {
+					return true, nil
+				}
+			}
+		}
+		return false, nil
+	})
+
+	return err
+}
diff --git a/pkg/feature/servicemesh/conditions.go b/pkg/feature/servicemesh/conditions.go
@@ -7,6 +7,8 @@ import (
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -37,6 +39,21 @@ func EnsureServiceMeshOperatorInstalled(ctx context.Context, cli client.Client,
 	if err := feature.EnsureOperatorIsInstalled("servicemeshoperator")(ctx, cli, f); err != nil {
 		return fmt.Errorf("failed to find the pre-requisite Service Mesh Operator subscription, please ensure Service Mesh Operator is installed. %w", err)
 	}
+	// Extra check SMCP CRD is installed and is active.
+	if err := cluster.CustomResourceDefinitionExists(ctx, cli, gvk.ServiceMeshControlPlane.GroupKind()); err != nil {
+		return fmt.Errorf("failed to find the Service Mesh Control Plane CRD, please ensure Service Mesh Operator is installed. %w", err)
+	}
+	// Extra check smcp validation service is running.
+	validationService := &corev1.Service{}
+	if err := cli.Get(ctx, client.ObjectKey{
+		Name:      "istio-operator-service",
+		Namespace: "openshift-operators",
+	}, validationService); err != nil {
+		if k8serr.IsNotFound(err) {
+			return fmt.Errorf("failed to find the Service Mesh VWC service, please ensure Service Mesh Operator is running. %w", err)
+		}
+		return fmt.Errorf("failed to find the Service Mesh VWC service. %w", err)
+	}
 
 	return nil
 }

diff --git a/tests/integration/features/fixtures/cluster_test_fixtures.go b/tests/integration/features/fixtures/cluster_test_fixtures.go
@@ -79,6 +79,28 @@ func GetService(ctx context.Context, client client.Client, namespace, name strin
 	return svc, err
 }
 
+func CreateService(ctx context.Context, client client.Client, namespace, svcName string) (*corev1.Service, error) {
+	if err := client.Create(ctx, &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      svcName,
+			Namespace: namespace,
+		},
+		Spec: corev1.ServiceSpec{
+			Selector: map[string]string{
+				"name": "istio-operator",
+			},
+			Ports: []corev1.ServicePort{
+				{
+					Port: 443,
+				},
+			},
+		},
+	}); err != nil {
+		return nil, err
+	}
+	return GetService(ctx, client, namespace, svcName)
+}
+
 func CreateSecret(name, namespace string) feature.Action {
 	return func(ctx context.Context, cli client.Client, f *feature.Feature) error {
 		secret := &corev1.Secret{

diff --git a/tests/integration/features/servicemesh_feature_test.go b/tests/integration/features/servicemesh_feature_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"path"
 
+	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -49,9 +50,11 @@ var _ = Describe("Service Mesh setup", func() {
 
 		Context("operator setup", func() {
 
-			When("operator is not installed", func() {
+			Context("operator is not installed", Ordered, func() {
 
-				It("should fail using precondition check", func(ctx context.Context) {
+				var smcpCrdObj *apiextensionsv1.CustomResourceDefinition
+
+				It("should fail using precondition subscription check", func(ctx context.Context) {
 					// given
 					featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error {
 						errFeatureAdd := registry.Add(feature.Define("no-service-mesh-operator-check").
@@ -69,19 +72,72 @@ var _ = Describe("Service Mesh setup", func() {
 					// then
 					Expect(applyErr).To(MatchError(ContainSubstring("failed to find the pre-requisite operator subscription \"servicemeshoperator\"")))
 				})
+
+				It("should fail using precondition CRD check", func(ctx context.Context) {
+					// given
+					err := fixtures.CreateSubscription(ctx, envTestClient, "openshift-operators", fixtures.OssmSubscription)
+					Expect(err).ToNot(HaveOccurred())
+
+					featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error {
+						errFeatureAdd := registry.Add(feature.Define("no-service-mesh-crd-check").
+							PreConditions(servicemesh.EnsureServiceMeshOperatorInstalled),
+						)
+
+						Expect(errFeatureAdd).ToNot(HaveOccurred())
+
+						return nil
+					})
+
+					// when
+					applyErr := featuresHandler.Apply(ctx, envTestClient)
+
+					// then
+					Expect(applyErr).To(MatchError(ContainSubstring("failed to find the Service Mesh Control Plane CRD")))
+				})
+
+				It("should fail using precondition service check", func(ctx context.Context) {
+					// given
+					smcpCrdObj = installServiceMeshCRD(ctx)
+
+					featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error {
+						errFeatureAdd := registry.Add(feature.Define("no-service-mesh-service-check").
+							PreConditions(servicemesh.EnsureServiceMeshOperatorInstalled),
+						)
+
+						Expect(errFeatureAdd).ToNot(HaveOccurred())
+
+						return nil
+					})
+
+					// when
+					applyErr := featuresHandler.Apply(ctx, envTestClient)
+
+					// then
+					Expect(applyErr).To(MatchError(ContainSubstring("failed to find the Service Mesh VWC service")))
+				})
+
+				AfterAll(func(ctx context.Context) {
+					objectCleaner.DeleteAll(ctx, smcpCrdObj)
+				})
 			})
 
 			When("operator is installed", func() {
 				var smcpCrdObj *apiextensionsv1.CustomResourceDefinition
+				var svc *corev1.Service
 
 				BeforeEach(func(ctx context.Context) {
 					err := fixtures.CreateSubscription(ctx, envTestClient, "openshift-operators", fixtures.OssmSubscription)
 					Expect(err).ToNot(HaveOccurred())
+
 					smcpCrdObj = installServiceMeshCRD(ctx)
+
+					svc, err = fixtures.CreateService(ctx, envTestClient, "openshift-operators", "istio-operator-service")
+					Expect(err).ToNot(HaveOccurred())
+
 				})
 
 				AfterEach(func(ctx context.Context) {
-					objectCleaner.DeleteAll(ctx, smcpCrdObj)
+					objectCleaner.DeleteAll(ctx, smcpCrdObj, svc)
 				})
 
 				It("should succeed using precondition check", func(ctx context.Context) {