controllers: Ensure console plugin creation after CSV verification

controllers/clusterversion_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"time"
 
 	configv1 "github.com/openshift/api/config/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -37,8 +38,9 @@ import (
 // ClusterVersionReconciler reconciles a ClusterVersion object
 type ClusterVersionReconciler struct {
 	client.Client
-	Scheme      *runtime.Scheme
-	ConsolePort int
+	Scheme            *runtime.Scheme
+	ConsolePort       int
+	OperatorNamespace string
 }
 
 //+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions,verbs=get;list;watch;create;update;patch;delete
@@ -58,7 +60,28 @@ func (r *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if err := r.Client.Get(context.TODO(), req.NamespacedName, &instance); err != nil {
 		return ctrl.Result{}, err
 	}
-	if err := r.ensureConsolePlugin(instance.Status.Desired.Version); err != nil {
+
+	if err := r.ensureOdfConsoleConfigMapAndService(); err != nil {
+		logger.Error(err, "Could not ensure configmap and service for odf-console deployment")
+		return ctrl.Result{}, err
+	}
+
+	csvList, err := util.GetNamespaceCSVs(ctx, r.Client, r.OperatorNamespace)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	// Check if it is upgrade from 4.17 to 4.18
+	// The new CSVs won't exists while upgrading
+	// They will exists only after new operator has created a new subscription
+	if !util.AreMultipleOdfOperatorCsvsPresent(csvList) {
+		if err := util.ValidateCSVsPresent(csvList, EssentialCSVs...); err != nil {
+			logger.Error(err, "Could not ensure CSVs presence")
+			return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 2}, nil
+		}
+	}
+
+	if err := r.ensureConsolePluginAndCLIDownload(instance.Status.Desired.Version); err != nil {
 		logger.Error(err, "Could not ensure compatibility for ODF consolePlugin")
 		return ctrl.Result{}, err
 	}
@@ -69,12 +92,7 @@ func (r *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 // SetupWithManager sets up the controller with the Manager.
 func (r *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	err := mgr.Add(manager.RunnableFunc(func(context.Context) error {
-		clusterVersion, err := util.DetermineOpenShiftVersion(r.Client)
-		if err != nil {
-			return err
-		}
-
-		return r.ensureConsolePlugin(clusterVersion)
+		return r.ensureOdfConsoleConfigMapAndService()
 	}))
 	if err != nil {
 		return err
@@ -85,15 +103,10 @@ func (r *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) error {
+func (r *ClusterVersionReconciler) ensureOdfConsoleConfigMapAndService() error {
 	logger := log.FromContext(context.TODO())
-	// The base path to where the request are sent
-	basePath := console.GetBasePath(clusterVersion)
 	nginxConf := console.NginxConf
 
-	// Customer portal link (CLI Tool download)
-	portalLink := console.CUSTOMER_PORTAL_LINK
-
 	// Get ODF console Deployment
 	odfConsoleDeployment := console.GetDeployment(OperatorNamespace)
 	err := r.Client.Get(context.TODO(), types.NamespacedName{
@@ -126,9 +139,19 @@ func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) er
 		return err
 	}
 
+	return nil
+}
+
+func (r *ClusterVersionReconciler) ensureConsolePluginAndCLIDownload(clusterVersion string) error {
+	logger := log.FromContext(context.TODO())
+	// The base path to where the request are sent
+	basePath := console.GetBasePath(clusterVersion)
+	// Customer portal link (CLI Tool download)
+	portalLink := console.CUSTOMER_PORTAL_LINK
+
 	// Create/Update ODF console ConsolePlugin
 	odfConsolePlugin := console.GetConsolePluginCR(r.ConsolePort, OperatorNamespace)
-	_, err = controllerutil.CreateOrUpdate(context.TODO(), r.Client, odfConsolePlugin, func() error {
+	_, err := controllerutil.CreateOrUpdate(context.TODO(), r.Client, odfConsolePlugin, func() error {
 		if odfConsolePlugin.Spec.Backend.Service != nil {
 			if currentBasePath := odfConsolePlugin.Spec.Backend.Service.BasePath; currentBasePath != basePath {
 				logger.Info(fmt.Sprintf("Set the BasePath for odf-console plugin as '%s'", basePath))

controllers/defaults.go

@@ -172,6 +172,14 @@ const (
 	OdfSubscriptionPackage = "odf-operator"
 )
 
+var (
+	EssentialCSVs = []string{
+		OcsSubscriptionStartingCSV,
+		RookSubscriptionStartingCSV,
+		NoobaaSubscriptionStartingCSV,
+	}
+)
+
 func GetEnvOrDefault(env string) string {
 	if val := os.Getenv(env); val != "" {
 		return val

controllers/subscription_controller.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -290,6 +291,14 @@ func (r *SubscriptionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		},
 	)
 
+	err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
+		odfDepsSub := GetStorageClusterSubscriptions()[0]
+		return EnsureDesiredSubscription(r.Client, odfDepsSub)
+	}))
+	if err != nil {
+		return err
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&operatorv1alpha1.Subscription{},
 			builder.WithPredicates(generationChangedPredicate, subscriptionPredicate)).

controllers/subscriptions.go

@@ -618,6 +618,8 @@ func GetStorageClusterSubscriptions() []*operatorv1alpha1.Subscription {
 		},
 	}
 
+	// Do not change the inxex of odfDepsSubscription. The 0 index is being used to create this subscription
+	// while starting in the subscription controller in SetupWithManager.
 	return []*operatorv1alpha1.Subscription{odfDepsSubscription, ocsSubscription, rookSubscription, noobaaSubscription,
 		csiAddonsSubscription, cephCsiSubscription, ocsClientSubscription, prometheusSubscription, recipeSubscription}
 }

controllers/subscriptions_test.go

@@ -30,6 +30,13 @@ import (
 	odfv1alpha1 "github.com/red-hat-storage/odf-operator/api/v1alpha1"
 )
 
+func TestSubscriptionIndex(t *testing.T) {
+	odfDepsSub := GetStorageClusterSubscriptions()[0]
+	msg := "odfDepsSub variable is expected to contain the 'odf-dependencies' subscription. " +
+		"Ensure the 'odf-dependencies' subscription indexed at 0."
+	assert.Equal(t, OdfDepsSubscriptionPackage, odfDepsSub.Spec.Package, msg)
+}
+
 func TestEnsureSubscription(t *testing.T) {
 
 	testCases := []struct {

main.go

@@ -158,9 +158,10 @@ func main() {
 	}
 
 	if err = (&controllers.ClusterVersionReconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		ConsolePort: odfConsolePort,
+		Client:            mgr.GetClient(),
+		Scheme:            mgr.GetScheme(),
+		ConsolePort:       odfConsolePort,
+		OperatorNamespace: operatorNamespace,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterVersion")
 		os.Exit(1)
@@ -176,17 +177,12 @@ func main() {
 	// can't check for only odf-dependencies CSV as OLM doesn't guarantee
 	// (based on observations) existence of all CRDs brought by OLM dependency
 	// mechanism.
-	csvsToBeSuceeded := []string{
-		controllers.OcsSubscriptionStartingCSV,
-		controllers.RookSubscriptionStartingCSV,
-		controllers.NoobaaSubscriptionStartingCSV,
-	}
 	if err := mgr.AddReadyzCheck(
 		"readyz",
 		util.CheckCSVPhase(
 			mgr.GetClient(),
 			operatorNamespace,
-			csvsToBeSuceeded...,
+			controllers.EssentialCSVs...,
 		),
 	); err != nil {
 		setupLog.Error(err, "unable to set up ready check")

pkg/util/readiness.go

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"strings"
@@ -26,15 +27,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 )
 
-func CheckCSVPhase(c client.Client, namespace string, csvNames ...string) healthz.Checker {
+func CheckCSVPhase(cli client.Client, namespace string, csvNames ...string) healthz.Checker {
 
-	csvMap := map[string]struct{}{}
-	for _, name := range csvNames {
-		csvMap[name] = struct{}{}
-	}
 	return func(r *http.Request) error {
-		csvList := &opv1a1.ClusterServiceVersionList{}
-		if err := c.List(r.Context(), csvList, client.InNamespace(namespace)); err != nil {
+
+		csvList, err := GetNamespaceCSVs(r.Context(), cli, namespace)
+		if err != nil {
 			return err
 		}
 
@@ -45,24 +43,51 @@ func CheckCSVPhase(c client.Client, namespace string, csvNames ...string) health
 			return nil
 		}
 
-		for idx := range csvList.Items {
-			csv := &csvList.Items[idx]
-			_, exists := csvMap[csv.Name]
-			if exists {
-				if csv.Status.Phase != opv1a1.CSVPhaseSucceeded {
-					return fmt.Errorf("CSV %s is not in Succeeded phase", csv.Name)
-				} else if csv.Status.Phase == opv1a1.CSVPhaseSucceeded {
-					delete(csvMap, csv.Name)
-				}
-			}
+		return validateCSVsSucceeded(csvList, csvNames...)
+	}
+}
+
+func validateCSVsSucceeded(csvList *opv1a1.ClusterServiceVersionList, csvsToBeSucceeded ...string) error {
+
+	for _, csvName := range csvsToBeSucceeded {
+		csv, found := getCSVByName(csvList, csvName)
+		if !found {
+			return fmt.Errorf("CSV %q not found in the list", csvName)
+		}
+
+		if csv.Status.Phase != opv1a1.CSVPhaseSucceeded {
+			return fmt.Errorf("CSV %q is not in the Succeeded phase; current phase: %s", csv.Name, csv.Status.Phase)
+		}
+	}
+	return nil
+}
+
+func ValidateCSVsPresent(csvList *opv1a1.ClusterServiceVersionList, csvsToBePresent ...string) error {
+
+	for _, csvName := range csvsToBePresent {
+		_, found := getCSVByName(csvList, csvName)
+		if !found {
+			return fmt.Errorf("CSV %q not found in the list", csvName)
 		}
-		if len(csvMap) != 0 {
-			for csvName := range csvMap {
-				return fmt.Errorf("CSV %s is not found", csvName)
-			}
+	}
+	return nil
+}
+
+func getCSVByName(csvList *opv1a1.ClusterServiceVersionList, name string) (*opv1a1.ClusterServiceVersion, bool) {
+
+	for i := range csvList.Items {
+		if csvList.Items[i].Name == name {
+			return &csvList.Items[i], true
 		}
-		return nil
 	}
+	return nil, false
+}
+
+func GetNamespaceCSVs(ctx context.Context, cli client.Client, namespace string) (*opv1a1.ClusterServiceVersionList, error) {
+
+	csvList := &opv1a1.ClusterServiceVersionList{}
+	err := cli.List(ctx, csvList, client.InNamespace(namespace))
+	return csvList, err
 }
 
 func AreMultipleOdfOperatorCsvsPresent(csvs *opv1a1.ClusterServiceVersionList) bool {