-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
controllers: replicate odf tolerations to child subscriptions #368
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -42,39 +42,99 @@ import ( | |||
// because fake.Client does not support them. | ||||
func CheckExistingSubscriptions(cli client.Client, desiredSubscription *operatorv1alpha1.Subscription) (*operatorv1alpha1.Subscription, error) { | ||||
|
||||
odfSub, err := GetOdfSubscription(cli) | ||||
if err != nil { | ||||
return nil, err | ||||
} | ||||
|
||||
if odfSub.Spec.Config == nil { | ||||
odfSub.Spec.Config = &operatorv1alpha1.SubscriptionConfig{} | ||||
} | ||||
|
||||
subsList := &operatorv1alpha1.SubscriptionList{} | ||||
err := cli.List(context.TODO(), subsList, &client.ListOptions{Namespace: desiredSubscription.Namespace}) | ||||
err = cli.List(context.TODO(), subsList, &client.ListOptions{Namespace: desiredSubscription.Namespace}) | ||||
if err != nil { | ||||
return nil, err | ||||
} | ||||
|
||||
var subExsist bool | ||||
var actualSub *operatorv1alpha1.Subscription | ||||
pkgName := desiredSubscription.Spec.Package | ||||
for i, sub := range subsList.Items { | ||||
if sub.Spec.Package == pkgName { | ||||
subExsist = true | ||||
if actualSub != nil { | ||||
foundSubs := []string{actualSub.Name, sub.Name} | ||||
return nil, fmt.Errorf("multiple Subscriptions found for package '%s': %v", pkgName, foundSubs) | ||||
} | ||||
actualSub = &subsList.Items[i] | ||||
actualSub.Spec.Channel = desiredSubscription.Spec.Channel | ||||
|
||||
// If the config is not set, only then set it to the desired value => allow user to override | ||||
if actualSub.Spec.Config == nil { | ||||
if actualSub.Spec.Config == nil && desiredSubscription.Spec.Config == nil { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just update odf-operator/controllers/subscriptions.go Line 260 in 1969776
Add tolerations to desired subscription, and during reconcile merge actual and desired subs. Wouldn't that be simpler? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually that will require extensive changes in the code, we will have to change the signature of multiple funcs. |
||||
actualSub.Spec.Config = &operatorv1alpha1.SubscriptionConfig{} | ||||
actualSub.Spec.Config.Tolerations = odfSub.Spec.Config.Tolerations | ||||
} else if actualSub.Spec.Config == nil && desiredSubscription.Spec.Config != nil { | ||||
actualSub.Spec.Config = desiredSubscription.Spec.Config | ||||
actualSub.Spec.Config.Tolerations = getMergedTolerations(odfSub.Spec.Config.Tolerations, desiredSubscription.Spec.Config.Tolerations) | ||||
} else if actualSub.Spec.Config != nil && desiredSubscription.Spec.Config == nil { | ||||
actualSub.Spec.Config.Tolerations = odfSub.Spec.Config.Tolerations | ||||
} else if actualSub.Spec.Config != nil && desiredSubscription.Spec.Config != nil { | ||||
// Combines the environment variables from both subscriptions. | ||||
// If actualSub already contains an environment variable, its value will be updated with the value from desiredSubscription. | ||||
actualSub.Spec.Config.Env = getMergedEnvVars(actualSub.Spec.Config.Env, desiredSubscription.Spec.Config.Env) | ||||
// Combines the Tolerations from odf sub and desired sub. | ||||
actualSub.Spec.Config.Tolerations = getMergedTolerations(odfSub.Spec.Config.Tolerations, desiredSubscription.Spec.Config.Tolerations) | ||||
} | ||||
|
||||
desiredSubscription = actualSub | ||||
} | ||||
} | ||||
|
||||
if !subExsist { | ||||
if desiredSubscription.Spec.Config == nil { | ||||
desiredSubscription.Spec.Config = &operatorv1alpha1.SubscriptionConfig{ | ||||
Tolerations: odfSub.Spec.Config.Tolerations, | ||||
} | ||||
} else { | ||||
desiredSubscription.Spec.Config.Tolerations = getMergedTolerations(odfSub.Spec.Config.Tolerations, desiredSubscription.Spec.Config.Tolerations) | ||||
} | ||||
} | ||||
|
||||
return desiredSubscription, nil | ||||
} | ||||
|
||||
func getMergedTolerations(tol1, tol2 []corev1.Toleration) []corev1.Toleration { | ||||
|
||||
if len(tol1) == 0 { | ||||
return append([]corev1.Toleration{}, tol2...) | ||||
} else if len(tol2) == 0 { | ||||
return append([]corev1.Toleration{}, tol1...) | ||||
} | ||||
|
||||
mergedTolerations := append([]corev1.Toleration{}, tol1...) | ||||
|
||||
for _, t2 := range tol2 { | ||||
found := false | ||||
for i, t1 := range tol1 { | ||||
if t1.Key == t2.Key && t1.Operator == t2.Operator && t1.Value == t2.Value { | ||||
found = true | ||||
break | ||||
} | ||||
// If the toleration with the same key but different values is found, | ||||
// update the existing toleration in tol1 with the new toleration from tol2. | ||||
if t1.Key == t2.Key && t1.Operator == t2.Operator { | ||||
tol1[i] = t2 | ||||
found = true | ||||
break | ||||
} | ||||
} | ||||
if !found { | ||||
mergedTolerations = append(mergedTolerations, t2) | ||||
} | ||||
} | ||||
|
||||
return mergedTolerations | ||||
} | ||||
|
||||
// getMergedEnvVars updates the value of env variables in the envList1 with that of envList2 and | ||||
// returns the updated list of env variables. | ||||
func getMergedEnvVars(envList1, envList2 []corev1.EnvVar) []corev1.EnvVar { | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed. Do not update your own subscription.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not gonna update. It is just to save from the panic in the below code.