Force reinstall of operator resources on release version upgrade #1746
+114
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bshephar
reviewed
Dec 27, 2025
Comment on lines
352
to
435
| func (r *OpenStackReconciler) deleteAllOwnedResources(ctx context.Context, instance *operatorv1beta1.OpenStack) error { | ||
| Log := r.GetLogger(ctx) | ||
| Log.Info("Deleting all owned resources for release version upgrade") | ||
|
|
||
| // Delete all owned deployments | ||
| deployments := &appsv1.DeploymentList{} | ||
| err := r.List(ctx, deployments, &client.ListOptions{Namespace: instance.Namespace}) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to list deployments") | ||
| } | ||
| for _, deployment := range deployments.Items { | ||
| if metav1.IsControlledBy(&deployment, instance) { | ||
| Log.Info("Deleting deployment", "name", deployment.Name) | ||
| err := r.Delete(ctx, &deployment) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return errors.Wrapf(err, "failed to delete deployment %s", deployment.Name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Delete all owned service accounts | ||
| serviceAccounts := &corev1.ServiceAccountList{} | ||
| err = r.List(ctx, serviceAccounts, &client.ListOptions{Namespace: instance.Namespace}) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to list service accounts") | ||
| } | ||
| for _, sa := range serviceAccounts.Items { | ||
| if metav1.IsControlledBy(&sa, instance) { | ||
| Log.Info("Deleting service account", "name", sa.Name) | ||
| err := r.Delete(ctx, &sa) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return errors.Wrapf(err, "failed to delete service account %s", sa.Name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Delete all owned services | ||
| services := &corev1.ServiceList{} | ||
| err = r.List(ctx, services, &client.ListOptions{Namespace: instance.Namespace}) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed to list services") | ||
| } | ||
| for _, svc := range services.Items { | ||
| if metav1.IsControlledBy(&svc, instance) { | ||
| Log.Info("Deleting service", "name", svc.Name) | ||
| err := r.Delete(ctx, &svc) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return errors.Wrapf(err, "failed to delete service %s", svc.Name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Delete webhooks (these are cluster-scoped and not owned, but managed by label) | ||
| valWebhooks, err := r.Kclient.AdmissionregistrationV1().ValidatingWebhookConfigurations().List(ctx, metav1.ListOptions{ | ||
| LabelSelector: "openstack.openstack.org/managed=true", | ||
| }) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed listing validating webhook configurations") | ||
| } | ||
| for _, webhook := range valWebhooks.Items { | ||
| Log.Info("Deleting validating webhook", "name", webhook.Name) | ||
| err := r.Kclient.AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(ctx, webhook.Name, metav1.DeleteOptions{}) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return errors.Wrapf(err, "failed to delete validating webhook %s", webhook.Name) | ||
| } | ||
| } | ||
|
|
||
| mutWebhooks, err := r.Kclient.AdmissionregistrationV1().MutatingWebhookConfigurations().List(ctx, metav1.ListOptions{ | ||
| LabelSelector: "openstack.openstack.org/managed=true", | ||
| }) | ||
| if err != nil { | ||
| return errors.Wrap(err, "failed listing mutating webhook configurations") | ||
| } | ||
| for _, webhook := range mutWebhooks.Items { | ||
| Log.Info("Deleting mutating webhook", "name", webhook.Name) | ||
| err := r.Kclient.AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(ctx, webhook.Name, metav1.DeleteOptions{}) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return errors.Wrapf(err, "failed to delete mutating webhook %s", webhook.Name) | ||
| } | ||
| } | ||
|
|
||
| Log.Info("All owned resources deleted successfully") | ||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
Be really nice if this could be done with some Generics to avoid the repetition. Does something like this work?
bshephar@80a5be2
(I don't have a OCP / OKD env to validate. So just throwing out a suggestion for you.)
dprince
force-pushed
the
reinstall_operators
branch
from
7b22ba9 to
5080ea0
Compare
Fixes upgrade failures from 0.4 to main caused by incompatible webhook configuration changes that trigger index out of range panics during manifest merging. When OPENSTACK_RELEASE_VERSION is bumped, the controller now: - Detects the version change by comparing against status.ReleaseVersion - Deletes all owned resources (deployments, services, serviceaccounts, configmaps) - Removes managed webhooks (validating and mutating configurations) - Requeues to recreate resources with new manifests This one-time cleanup ensures a clean slate for incompatible upgrades where the structure of resources (especially webhooks) has changed between versions. Adds ReleaseVersion field to OpenStackStatus to track the deployed version. Jira: OSPRH-23865 Co-authored-by: Brendan Shephard <bshephar@fedora-g16.bne-home.net>
dprince
force-pushed
the
reinstall_operators
branch
from
5080ea0 to
73c5855
Compare
bshephar
approved these changes
Jan 7, 2026
Contributor
bshephar
left a comment
bshephar
left a comment
There was a problem hiding this comment.
Nice, looks good.
It could probably be tightened up to avoid the runtime assertions by using client.ObjectList instead of any for the list arg. I guess all of those types should match client.ObjectList. But that could just be a nice follow up patch imo. Something like:
-func deleteOwnedResources[L any, T any](
+func deleteOwnedResources[L client.ObjectList, T any](
ctx context.Context,
r *OpenStackReconciler,
instance client.Object,
@@ -359,7 +359,7 @@ func deleteOwnedResources[L any, T any](
) error {
log := r.GetLogger(ctx)
- err := r.List(ctx, any(list).(client.ObjectList), &client.ListOptions{Namespace: instance.GetNamespace()})
+ err := r.List(ctx, list, &client.ListOptions{Namespace: instance.GetNamespace()})
if err != nil {
return errors.Wrap(err, "failed to list resources")
}
I think that should still work.
Happy New Year btw.
Contributor
|
@bshephar: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bshephar, dprince The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes upgrade failures from 0.4 to main caused by incompatible webhook configuration changes that trigger index out of range panics during manifest merging.
When OPENSTACK_RELEASE_VERSION is bumped, the controller now:
This one-time cleanup ensures a clean slate for incompatible upgrades where the structure of resources (especially webhooks) has changed between versions.
Adds ReleaseVersion field to OpenStackStatus to track the deployed version.
Jira: OSPRH-23865