controllers: Ensure console plugin creation after CSV verification

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

Signed-off-by: Nitin Goyal <[email protected]>

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,18 @@ 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
+	}
+
+	if err := util.EnsureCSVsSucceeded(ctx, r.Client, r.OperatorNamespace, CsvsToBeSuceeded...); err != nil {
+		logger.Error(err, "Could not ensure CSVs in Succeeded state")
+		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 +82,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 +93,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 +129,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 (
+	CsvsToBeSuceeded = []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.CsvsToBeSuceeded...,
 		),
 	); 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,50 @@ 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 EnsureCSVsSucceeded(ctx context.Context, cli client.Client, namespace string, csvsToBeSuceeded ...string) error {
+
+	csvList, err := GetNamespaceCSVs(ctx, cli, namespace)
+	if err != nil {
+		return err
+	}
+
+	return ValidateCSVsSucceeded(csvList, csvsToBeSuceeded...)
+}
+
+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)
 		}
-		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 _, csv := range csvList.Items {
+		if csv.Name == name {
+			return &csv, 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 {