From cb4ada0b01f5ee820bdadc9ebb97fbfa8639bb9d Mon Sep 17 00:00:00 2001 From: Nitin Goyal Date: Thu, 21 Nov 2024 16:57:46 +0530 Subject: [PATCH] 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 --- controllers/clusterversion_controller.go | 45 ++++++++++------ controllers/defaults.go | 8 +++ main.go | 14 ++--- pkg/util/readiness.go | 68 ++++++++++++++++-------- vendor/modules.txt | 2 +- 5 files changed, 89 insertions(+), 48 deletions(-) diff --git a/controllers/clusterversion_controller.go b/controllers/clusterversion_controller.go index b5f25a5e7..ca65163c5 100644 --- a/controllers/clusterversion_controller.go +++ b/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)) diff --git a/controllers/defaults.go b/controllers/defaults.go index b8d837b21..a7613498b 100644 --- a/controllers/defaults.go +++ b/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 diff --git a/main.go b/main.go index 7079ffe60..55cd28cd1 100644 --- a/main.go +++ b/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") diff --git a/pkg/util/readiness.go b/pkg/util/readiness.go index fe5e5a8f8..4f282931f 100644 --- a/pkg/util/readiness.go +++ b/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 { diff --git a/vendor/modules.txt b/vendor/modules.txt index 866eb7fc1..f59da9a17 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,5 +1,5 @@ # github.com/IBM/ibm-storage-odf-operator v1.6.0 -## explicit; go 1.20 +## explicit; go 1.22.0 github.com/IBM/ibm-storage-odf-operator/api/v1alpha1 # github.com/beorn7/perks v1.0.1 ## explicit; go 1.11