Skip to content

Commit

Permalink
Update ODLM CRs after ODLM is upgraded to desired version (IBM#2280)
Browse files Browse the repository at this point in the history 
* Update ODLM CRs after ODLM is upgraded to desired version

Signed-off-by: Daniel Fan <[email protected]>

* Optimize func waitOperatorCSV by getting the subscription via name first. If the subscription does not exist, list subscription via package manifest label

Signed-off-by: Daniel Fan <[email protected]>

---------

Signed-off-by: Daniel Fan <[email protected]>
  • Loading branch information
@Daniel-Fan
Daniel-Fan committed Nov 1, 2024
1 parent f28fe04 commit 7a18318
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 37 deletions.
58 changes: 32 additions & 26 deletions controllers/bootstrap/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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/<packageManifest>.<operatorNs>: ''"
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/<packageManifest>.<operatorNs>: ''"
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
Expand Down
20 changes: 10 additions & 10 deletions controllers/bootstrap/init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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))

Expand All @@ -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))

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion controllers/commonservice_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7a18318

Please sign in to comment.