Skip to content

Commit e9798cf

Browse files
dprinceBrendan Shephard
andcommitted
Force reinstall of operator resources on release version upgrade
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. Also, adds bounds checking to skip copying clientConfig for new webhooks that don't exist in the current configuration, allowing the merge to complete successfully. Jira: OSPRH-23865 Co-authored-by: Brendan Shephard <[email protected]>
1 parent d6d2837 commit e9798cf

File tree

7 files changed

+133
-3
lines changed

7 files changed

+133
-3
lines changed

api/bases/operator.openstack.org_openstacks.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@ spec:
268268
for this object.
269269
format: int64
270270
type: integer
271+
releaseVersion:
272+
description: ReleaseVersion - the OpenStack release version that has
273+
been successfully deployed
274+
type: string
271275
totalOperatorCount:
272276
description: TotalOperatorCount - the number all operators available
273277
type: integer

api/operator/v1beta1/openstack_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,9 @@ type OpenStackStatus struct {
256256

257257
// ContainerImage - the container image that has been successfully deployed
258258
ContainerImage *string `json:"containerImage,omitempty"`
259+
260+
// ReleaseVersion - the OpenStack release version that has been successfully deployed
261+
ReleaseVersion *string `json:"releaseVersion,omitempty"`
259262
}
260263

261264
// +kubebuilder:object:root=true

api/operator/v1beta1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/operator.openstack.org_openstacks.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@ spec:
268268
for this object.
269269
format: int64
270270
type: integer
271+
releaseVersion:
272+
description: ReleaseVersion - the OpenStack release version that has
273+
been successfully deployed
274+
type: string
271275
totalOperatorCount:
272276
description: TotalOperatorCount - the number all operators available
273277
type: integer

internal/controller/operator/openstack_controller.go

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import (
4343
"github.com/openstack-k8s-operators/openstack-operator/internal/operator"
4444
"github.com/openstack-k8s-operators/openstack-operator/internal/operator/bindata"
4545
"github.com/pkg/errors"
46+
admissionv1 "k8s.io/api/admissionregistration/v1"
4647
appsv1 "k8s.io/api/apps/v1"
4748
corev1 "k8s.io/api/core/v1"
4849
discoveryv1 "k8s.io/api/discovery/v1"
@@ -250,6 +251,48 @@ func (r *OpenStackReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
250251
return ctrl.Result{}, err
251252
}
252253

254+
// Check if OPENSTACK_RELEASE_VERSION has changed - if so, delete all owned resources
255+
// This is a one-time fix to handle incompatible upgrades
256+
shouldReinstall := false
257+
if instance.Status.ReleaseVersion != nil && *instance.Status.ReleaseVersion != openstackReleaseVersion {
258+
// Explicit version change detected
259+
shouldReinstall = true
260+
} else if !isNewInstance && instance.Status.ReleaseVersion == nil {
261+
// Existing installation upgrading from version without ReleaseVersion field
262+
shouldReinstall = true
263+
}
264+
265+
if shouldReinstall {
266+
Log.Info("OpenStack release version changed, deleting all owned resources",
267+
"old", instance.Status.ReleaseVersion,
268+
"new", openstackReleaseVersion)
269+
270+
if err := r.deleteAllOwnedResources(ctx, instance); err != nil {
271+
instance.Status.Conditions.Set(condition.FalseCondition(
272+
operatorv1beta1.OpenStackOperatorReadyCondition,
273+
condition.ErrorReason,
274+
condition.SeverityWarning,
275+
operatorv1beta1.OpenStackOperatorErrorMessage,
276+
err))
277+
return ctrl.Result{}, err
278+
}
279+
280+
// Reset the container image status to force re-application of CRDs and RBAC
281+
instance.Status.ContainerImage = nil
282+
283+
// Update the release version in status
284+
instance.Status.ReleaseVersion = &openstackReleaseVersion
285+
286+
// Requeue to allow resources to be deleted before recreating
287+
Log.Info("Resources deleted, requeuing to recreate with new version")
288+
return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, nil
289+
}
290+
291+
// Set the release version if not set (for new installations only)
292+
if instance.Status.ReleaseVersion == nil {
293+
instance.Status.ReleaseVersion = &openstackReleaseVersion
294+
}
295+
253296
if err := r.applyManifests(ctx, instance); err != nil {
254297
instance.Status.Conditions.Set(condition.FalseCondition(
255298
operatorv1beta1.OpenStackOperatorReadyCondition,
@@ -316,6 +359,69 @@ func (r *OpenStackReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
316359

317360
}
318361

362+
func deleteOwnedResources[L client.ObjectList, T any](
363+
ctx context.Context,
364+
r *OpenStackReconciler,
365+
instance client.Object,
366+
list L,
367+
itemsGetter func(L) []T,
368+
) error {
369+
log := r.GetLogger(ctx)
370+
371+
err := r.List(ctx, any(list).(client.ObjectList), &client.ListOptions{Namespace: instance.GetNamespace()})
372+
if err != nil {
373+
return errors.Wrap(err, "failed to list resources")
374+
}
375+
376+
for _, item := range itemsGetter(list) {
377+
obj := any(&item).(client.Object)
378+
if metav1.IsControlledBy(obj, instance) {
379+
log.Info("Deleting owned resource", "kind", obj.GetObjectKind().GroupVersionKind().Kind, "name", obj.GetName())
380+
err := r.Delete(ctx, obj)
381+
if err != nil && !apierrors.IsNotFound(err) {
382+
return errors.Wrapf(err, "failed to delete %s", obj.GetName())
383+
}
384+
}
385+
}
386+
return nil
387+
}
388+
389+
func (r *OpenStackReconciler) deleteAllOwnedResources(ctx context.Context, instance *operatorv1beta1.OpenStack) error {
390+
Log := r.GetLogger(ctx)
391+
Log.Info("Deleting all owned resources for release version upgrade")
392+
393+
err := deleteOwnedResources(ctx, r, instance, &appsv1.DeploymentList{}, func(l *appsv1.DeploymentList) []appsv1.Deployment { return l.Items })
394+
if err != nil {
395+
return err
396+
}
397+
398+
err = deleteOwnedResources(ctx, r, instance, &corev1.ServiceAccountList{}, func(l *corev1.ServiceAccountList) []corev1.ServiceAccount { return l.Items })
399+
if err != nil {
400+
return err
401+
}
402+
403+
err = deleteOwnedResources(ctx, r, instance, &corev1.ServiceList{}, func(l *corev1.ServiceList) []corev1.Service { return l.Items })
404+
if err != nil {
405+
return err
406+
}
407+
408+
labelSelector, _ := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
409+
MatchLabels: map[string]string{"openstack.openstack.org/managed": "true"},
410+
})
411+
412+
deleteOpts := client.DeleteAllOfOptions{
413+
ListOptions: client.ListOptions{LabelSelector: labelSelector},
414+
}
415+
416+
err = r.DeleteAllOf(ctx, &admissionv1.ValidatingWebhookConfiguration{}, &deleteOpts)
417+
if err != nil && !apierrors.IsNotFound(err) {
418+
return errors.Wrap(err, "failed to delete validating webhooks")
419+
}
420+
421+
Log.Info("All owned resources deleted successfully")
422+
return nil
423+
}
424+
319425
func (r *OpenStackReconciler) reconcileDelete(ctx context.Context, instance *operatorv1beta1.OpenStack, helper *helper.Helper) (ctrl.Result, error) {
320426
Log := r.GetLogger(ctx)
321427
Log.Info("Reconciling OpenStack initialization resource delete")
@@ -987,7 +1093,7 @@ func (r *OpenStackReconciler) postCleanupObsoleteResources(ctx context.Context,
9871093
// The horizon-operator.openstack-operators has references to old roles/bindings
9881094
// the code below will delete those references before continuing
9891095
for _, ref := range refs {
990-
refData := ref.(map[string]interface{})
1096+
refData := ref.(map[string]any)
9911097
Log.Info("Deleting operator reference", "Reference", ref)
9921098
obj := uns.Unstructured{}
9931099
obj.SetName(refData["name"].(string))

internal/dataplane/util/image_registry_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package util
17+
package util //nolint:revive // util is an acceptable package name in this context
1818

1919
import (
2020
"context"

internal/operator/bindata/merge.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,18 @@ func MergeWebhookConfigurationForUpdate(current, updated *uns.Unstructured) erro
6060
gvk := updated.GroupVersionKind()
6161

6262
if gvk.Group == "admissionregistration.k8s.io" && (gvk.Kind == "MutatingWebhookConfiguration" || gvk.Kind == "ValidatingWebhookConfiguration") {
63+
currentWebhooks, currentExists := current.Object["webhooks"].([]interface{})
64+
if !currentExists {
65+
return nil
66+
}
6367

6468
for i, webhook := range updated.Object["webhooks"].([]interface{}) {
69+
// Check if the index exists in the current webhooks list
70+
if i >= len(currentWebhooks) {
71+
continue
72+
}
6573

66-
currentClientConfig := current.Object["webhooks"].([]interface{})[i].(map[string]interface{})["clientConfig"].(map[string]interface{})
74+
currentClientConfig := currentWebhooks[i].(map[string]interface{})["clientConfig"].(map[string]interface{})
6775
if currentClientConfig != nil {
6876
webhook.(map[string]interface{})["clientConfig"] = currentClientConfig
6977
}

0 commit comments

Comments
 (0)