From cb0f58f03b8feaca0026ea1bd8e6633108ec6430 Mon Sep 17 00:00:00 2001 From: Andrew Bays Date: Tue, 29 Oct 2024 13:30:22 +0000 Subject: [PATCH] Improve backup and restore functionality for OpenStackBaremetalSets --- api/shared/labels.go | 4 + api/v1beta1/openstackbaremetalset_webhook.go | 24 +++++- .../openstackbaremetalset_controller.go | 2 +- pkg/openstackbackup/funcs.go | 85 +++++++++++++++++++ 4 files changed, 110 insertions(+), 5 deletions(-) diff --git a/api/shared/labels.go b/api/shared/labels.go index 98e4178f..5c0b548c 100644 --- a/api/shared/labels.go +++ b/api/shared/labels.go @@ -38,4 +38,8 @@ const ( OwnerControllerNameLabelSelector string = "osp-director.openstack.org/controller" // OwnerUIDLabelSelector - OwnerUIDLabelSelector string = "osp-director.openstack.org/uid" + // OwnerNameLabelSelector - + OwnerNameLabelSelector = "osp-director.openstack.org/name" + // RequireBMHDeprovision - placed on a BMH to indicate that it must fully deprovision before we are done with it + RequireBMHDeprovision = "osp-director.openstack.org/require-deprovision" ) diff --git a/api/v1beta1/openstackbaremetalset_webhook.go b/api/v1beta1/openstackbaremetalset_webhook.go index 194c21a3..f112dd40 100644 --- a/api/v1beta1/openstackbaremetalset_webhook.go +++ b/api/v1beta1/openstackbaremetalset_webhook.go @@ -25,7 +25,6 @@ import ( "context" "fmt" - metal3v1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" "github.com/openstack-k8s-operators/osp-director-operator/api/shared" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" @@ -91,7 +90,24 @@ func (r *OpenStackBaremetalSet) ValidateCreate() error { return err } - if _, err := VerifyBaremetalSetScaleUp(baremetalsetlog, r, baremetalHostsList, &metal3v1.BareMetalHostList{}); err != nil { + // + // Even though this is a new OpenStackBaremetalSet resource, we need to get a list of + // existing BMHs in case this is being executed in the process of an OpenStackBackup restore + // + existingBaremetalHosts, err := GetBmhHosts( + context.TODO(), + webhookClient, + "openshift-machine-api", + map[string]string{ + shared.OwnerControllerNameLabelSelector: shared.OpenStackBaremetalSetAppLabel, + shared.OwnerNameLabelSelector: r.Name, + }, + ) + if err != nil { + return err + } + + if _, err := VerifyBaremetalSetScaleUp(baremetalsetlog, r, baremetalHostsList, existingBaremetalHosts); err != nil { return err } @@ -157,7 +173,7 @@ func (r *OpenStackBaremetalSet) ValidateUpdate(old runtime.Object) error { "openshift-machine-api", map[string]string{ shared.OwnerControllerNameLabelSelector: shared.OpenStackBaremetalSetAppLabel, - shared.OwnerUIDLabelSelector: string(r.GetUID()), + shared.OwnerNameLabelSelector: r.Name, }, ) if err != nil { @@ -174,7 +190,7 @@ func (r *OpenStackBaremetalSet) ValidateUpdate(old runtime.Object) error { "openshift-machine-api", map[string]string{ shared.OwnerControllerNameLabelSelector: shared.OpenStackBaremetalSetAppLabel, - shared.OwnerUIDLabelSelector: string(r.GetUID()), + shared.OwnerNameLabelSelector: r.Name, }, ) if err != nil { diff --git a/controllers/openstackbaremetalset_controller.go b/controllers/openstackbaremetalset_controller.go index b5404858..932c9a45 100644 --- a/controllers/openstackbaremetalset_controller.go +++ b/controllers/openstackbaremetalset_controller.go @@ -1683,7 +1683,7 @@ func (r *OpenStackBaremetalSetReconciler) getExistingBaremetalHosts( "openshift-machine-api", map[string]string{ common.OwnerControllerNameLabelSelector: shared.OpenStackBaremetalSetAppLabel, - common.OwnerUIDLabelSelector: string(instance.GetUID()), + common.OwnerNameLabelSelector: instance.Name, }, ) if err != nil { diff --git a/pkg/openstackbackup/funcs.go b/pkg/openstackbackup/funcs.go index 0f2ec7fd..ba50957f 100644 --- a/pkg/openstackbackup/funcs.go +++ b/pkg/openstackbackup/funcs.go @@ -8,6 +8,8 @@ import ( k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + metal3v1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" + "github.com/openstack-k8s-operators/osp-director-operator/api/shared" ospdirectorv1beta1 "github.com/openstack-k8s-operators/osp-director-operator/api/v1beta1" ospdirectorv1beta2 "github.com/openstack-k8s-operators/osp-director-operator/api/v1beta2" "github.com/openstack-k8s-operators/osp-director-operator/pkg/common" @@ -605,11 +607,94 @@ func CleanNamespace( // OSP-D CRs can be mass-deleted if len(crLists.OpenStackBaremetalSets.Items) > 0 { foundRemaining = true + waitingOnBmhDeprovisionLabelAdded := false + + // Special case: since OpenStackBaremetalSets might have associated BMHs currently provisioned, + // we need to check for those BMHs and wait until they are finished deprovisioning before continuing + // with deleting other resources. If we don't, we can remove resources (such as networking CRs) + // that BMH deprovisioning currently depends upon and sabotage the deprovisioning by doing so (which + // will leave the BMHs stuck in the deprovisioning state). + + for _, osBms := range crLists.OpenStackBaremetalSets.Items { + baremetalHostsList, err := ospdirectorv1beta1.GetBmhHosts( + ctx, + r.GetClient(), + "openshift-machine-api", + map[string]string{ + common.OwnerControllerNameLabelSelector: shared.OpenStackBaremetalSetAppLabel, + common.OwnerNameLabelSelector: osBms.Name, + }, + ) + if err != nil { + return false, err + } + + // Annotate all the OpenStackBaremetalSet's BMHs with a special annotation to indicate to us + // that they must fully deprovision before we are done with them here + for _, bmh := range baremetalHostsList.Items { + labels := bmh.GetObjectMeta().GetLabels() + if _, ok := labels[shared.RequireBMHDeprovision]; !ok { + labels[shared.RequireBMHDeprovision] = "" + bmh.GetObjectMeta().SetLabels(labels) + + waitingOnBmhDeprovisionLabelAdded = true + + if err = r.GetClient().Update(ctx, &bmh); err != nil { + return false, err + } + } + } + } + + // If we added our "RequireBMHDeprovision" label to any BMH, we need to return from the reconcile + // and then check back again later + if waitingOnBmhDeprovisionLabelAdded { + return false, nil + } + + // Delete the OpenStackBaremetalSets, which will trigger deprovisioning of any associated BMHs if err := r.GetClient().DeleteAllOf(ctx, &ospdirectorv1beta1.OpenStackBaremetalSet{}, client.InNamespace(namespace)); err != nil { return false, err } } + // At this point, all OpenStackBaremetalSets have been deleted, so any associated BMHs will be deprovisioning. + // So we need to get all BMHs that we might be waiting on (waiting on to deprovision and reach the available state) + // that are flagged as such by our special label + baremetalHostsList, err := ospdirectorv1beta1.GetBmhHosts( + ctx, + r.GetClient(), + "openshift-machine-api", + map[string]string{ + shared.RequireBMHDeprovision: "", + }, + ) + if err != nil { + return false, err + } + + for _, bmh := range baremetalHostsList.Items { + // If the BMH is not available (i.e. not done deprovisioning), we need to return from + // this reconcile and check back again later + if bmh.Status.Provisioning.State != metal3v1.StateAvailable { + return false, nil + } + } + + // If we get here, all BMHs that were associated with our OpenStackBaremetalSets have been fully deprovisioned, + // so we can proceed to remove our special label + for _, bmh := range baremetalHostsList.Items { + labels := bmh.GetObjectMeta().GetLabels() + if _, ok := labels[shared.RequireBMHDeprovision]; ok { + delete(labels, shared.RequireBMHDeprovision) + bmh.GetObjectMeta().SetLabels(labels) + + if err = r.GetClient().Update(ctx, &bmh); err != nil { + return false, err + } + } + } + if len(crLists.OpenStackProvisionServers.Items) > 0 { foundRemaining = true if err := r.GetClient().DeleteAllOf(ctx, &ospdirectorv1beta1.OpenStackProvisionServer{}, client.InNamespace(namespace)); err != nil {