diff --git a/controllers/bootstrap/init.go b/controllers/bootstrap/init.go index 23517aef4..4640a4e88 100644 --- a/controllers/bootstrap/init.go +++ b/controllers/bootstrap/init.go @@ -253,7 +253,7 @@ func (b *Bootstrap) InitResources(instance *apiv3.CommonService, forceUpdateODLM } klog.Info("Waiting for ODLM Operator to be ready") - if isWaiting, err := b.waitOperatorCSV("ibm-odlm", b.CSData.CPFSNs); err != nil { + if isWaiting, err := b.waitOperatorCSV(constant.IBMODLMPackage, "ibm-odlm", b.CSData.CPFSNs); err != nil { return err } else if isWaiting { forceUpdateODLMCRs = true @@ -267,7 +267,7 @@ func (b *Bootstrap) InitResources(instance *apiv3.CommonService, forceUpdateODLM return err } // Reinstall/update OperandRegistry and OperandConfig if not installed/updated in the previous step - if !existOpreg || !existOpcon { + if !existOpreg || !existOpcon || forceUpdateODLMCRs { // Set "Pending" condition when creating OperandRegistry and OperandConfig instance.SetPendingCondition(constant.MasterCR, apiv3.ConditionTypePending, corev1.ConditionTrue, apiv3.ConditionReasonInit, apiv3.ConditionMessageInit) @@ -583,7 +583,7 @@ func (b *Bootstrap) ListOperandConfig(ctx context.Context, opts ...client.ListOp func (b *Bootstrap) ListOperatorConfig(ctx context.Context, opts ...client.ListOption) *odlm.OperatorConfigList { operatorConfigList := &odlm.OperatorConfigList{} if err := b.Client.List(ctx, operatorConfigList, opts...); err != nil { - klog.Errorf("failed to List OperandConfig: %v", err) + klog.Errorf("failed to List OperatorConfig: %v", err) return nil } @@ -2084,12 +2084,12 @@ func setEDBUserManaged(instance *apiv3.CommonService) { } } -func (b *Bootstrap) waitOperatorCSV(packageManifest, operatorNs string) (bool, error) { +func (b *Bootstrap) waitOperatorCSV(subName, packageManifest, operatorNs string) (bool, error) { var isWaiting bool // Wait for the operator CSV to be installed klog.Infof("Waiting for the operator CSV with packageManifest %s in namespace %s to be installed", packageManifest, operatorNs) if err := utilwait.PollImmediate(time.Second*5, time.Minute*5, func() (done bool, err error) { - installed, err := b.checkOperatorCSV(packageManifest, operatorNs) + installed, err := b.checkOperatorCSV(subName, packageManifest, operatorNs) if err != nil { return false, err } else if !installed { @@ -2103,37 +2103,43 @@ func (b *Bootstrap) waitOperatorCSV(packageManifest, operatorNs string) (bool, e return isWaiting, nil } -func (b *Bootstrap) checkOperatorCSV(packageManifest, operatorNs string) (bool, error) { - // List the subscription by packageManifest and operatorNs - // The subscription contain label "operators.coreos.com/.: ''" - subList := &olmv1alpha1.SubscriptionList{} - labelKey := util.GetFirstNCharacter(packageManifest+"."+operatorNs, 63) - if err := b.Client.List(context.TODO(), subList, &client.ListOptions{ - LabelSelector: labels.SelectorFromSet(labels.Set{ - "operators.coreos.com/" + labelKey: "", - }), - Namespace: operatorNs, - }); err != nil { - klog.Errorf("Failed to list Subscription by packageManifest %s and operatorNs %s: %v", packageManifest, operatorNs, err) - return false, err - } +func (b *Bootstrap) checkOperatorCSV(subName, packageManifest, operatorNs string) (bool, error) { + // Get the subscription by name and namespace + sub := &olmv1alpha1.Subscription{} + if err := b.Reader.Get(context.TODO(), types.NamespacedName{Name: subName, Namespace: operatorNs}, sub); err != nil { + klog.Warningf("Failed to get Subscription %s in namespace %s: %v, list subscription by packageManifest and operatorNs", subName, operatorNs, err) + // List the subscription by packageManifest and operatorNs + // The subscription contain label "operators.coreos.com/.: ''" + subList := &olmv1alpha1.SubscriptionList{} + labelKey := util.GetFirstNCharacter(packageManifest+"."+operatorNs, 63) + if err := b.Reader.List(context.TODO(), subList, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(labels.Set{ + "operators.coreos.com/" + labelKey: "", + }), + Namespace: operatorNs, + }); err != nil { + klog.Errorf("Failed to list Subscription by packageManifest %s and operatorNs %s: %v", packageManifest, operatorNs, err) + return false, err + } - // Check if multiple subscriptions exist - if len(subList.Items) > 1 { - return false, fmt.Errorf("multiple subscriptions found by packageManifest %s and operatorNs %s", packageManifest, operatorNs) - } else if len(subList.Items) == 0 { - return false, fmt.Errorf("no subscription found by packageManifest %s and operatorNs %s", packageManifest, operatorNs) + // Check if multiple subscriptions exist + if len(subList.Items) > 1 { + return false, fmt.Errorf("multiple subscriptions found by packageManifest %s and operatorNs %s", packageManifest, operatorNs) + } else if len(subList.Items) == 0 { + return false, fmt.Errorf("no subscription found by packageManifest %s and operatorNs %s", packageManifest, operatorNs) + } + sub = &subList.Items[0] } // Get the channel in the subscription .spec.channel, and check if it is semver - channel := subList.Items[0].Spec.Channel + channel := sub.Spec.Channel if !semver.IsValid(channel) { klog.Warningf("channel %s is not a semver for operator with packageManifest %s and operatorNs %s", channel, packageManifest, operatorNs) return false, nil } // Get the CSV from subscription .status.installedCSV - installedCSV := subList.Items[0].Status.InstalledCSV + installedCSV := sub.Status.InstalledCSV var installedVersion string if installedCSV != "" { // installedVersion is the version after the first dot in installedCSV diff --git a/controllers/bootstrap/init_test.go b/controllers/bootstrap/init_test.go index 6a36eaf26..118d53677 100644 --- a/controllers/bootstrap/init_test.go +++ b/controllers/bootstrap/init_test.go @@ -78,11 +78,11 @@ func TestCheckOperatorCSV(t *testing.T) { } // Test case 1: Single subscription found with valid semver - result, err := bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err := bootstrap.checkOperatorCSV("subscription-1", packageManifest, operatorNs) assert.True(t, result) assert.NoError(t, err) - // Test case 2: Multiple subscriptions found + // Test case 2: Multiple subscriptions found and not subscription name matched err = fakeClient.Create(context.TODO(), &olmv1alpha1.Subscription{ ObjectMeta: metav1.ObjectMeta{ Name: "subscription-2", @@ -100,7 +100,7 @@ func TestCheckOperatorCSV(t *testing.T) { }) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-non-match", packageManifest, operatorNs) assert.False(t, result) assert.EqualError(t, err, fmt.Sprintf("multiple subscriptions found by packageManifest %s and operatorNs %s", packageManifest, operatorNs)) @@ -117,7 +117,7 @@ func TestCheckOperatorCSV(t *testing.T) { }) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-1", packageManifest, operatorNs) assert.False(t, result) assert.EqualError(t, err, fmt.Sprintf("no subscription found by packageManifest %s and operatorNs %s", packageManifest, operatorNs)) @@ -139,7 +139,7 @@ func TestCheckOperatorCSV(t *testing.T) { }) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-3", packageManifest, operatorNs) assert.False(t, result) assert.NoError(t, err) @@ -153,7 +153,7 @@ func TestCheckOperatorCSV(t *testing.T) { err = fakeClient.Update(context.TODO(), subscription) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-3", packageManifest, operatorNs) assert.True(t, result) assert.NoError(t, err) @@ -167,7 +167,7 @@ func TestCheckOperatorCSV(t *testing.T) { err = fakeClient.Update(context.TODO(), subscription) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-3", packageManifest, operatorNs) assert.False(t, result) assert.NoError(t, err) @@ -181,7 +181,7 @@ func TestCheckOperatorCSV(t *testing.T) { err = fakeClient.Update(context.TODO(), subscription) assert.NoError(t, err) - result, err = bootstrap.checkOperatorCSV(packageManifest, operatorNs) + result, err = bootstrap.checkOperatorCSV("subscription-3", packageManifest, operatorNs) assert.True(t, result) assert.NoError(t, err) @@ -232,12 +232,12 @@ func TestWaitOperatorCSV(t *testing.T) { assert.NoError(t, err) }() - isWaiting, err := bootstrap.waitOperatorCSV(packageManifest, operatorNs) + isWaiting, err := bootstrap.waitOperatorCSV("subscription-1", packageManifest, operatorNs) assert.True(t, isWaiting) assert.NoError(t, err) // Test case 2: Operator CSV is already installed - isWaiting, err = bootstrap.waitOperatorCSV(packageManifest, operatorNs) + isWaiting, err = bootstrap.waitOperatorCSV("subscription-1", packageManifest, operatorNs) assert.False(t, isWaiting) assert.NoError(t, err) } diff --git a/controllers/commonservice_controller.go b/controllers/commonservice_controller.go index b1199e170..87774a9fd 100644 --- a/controllers/commonservice_controller.go +++ b/controllers/commonservice_controller.go @@ -77,7 +77,7 @@ func (r *CommonServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.ReconcileNonConfigurableCR(ctx, instance) } - if err := r.Bootstrap.Client.Get(ctx, req.NamespacedName, instance); err != nil { + if err := r.Reader.Get(ctx, req.NamespacedName, instance); err != nil { if errors.IsNotFound(err) { if err := r.handleDelete(ctx); err != nil { return ctrl.Result{}, err