controllers: Ensure console plugin creation after CSV verification

Delay the creation of the console plugin until after verifying that the
required CSVs are present. This ensures the necessary CRDs are present
in the cluster, preventing UI errors like "Model does not exist."

Signed-off-by: Nitin Goyal <nigoyal@redhat.com>

Author: iamniting

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

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 {