From 5faffd18fc8e946324d462c297ab8c3a76c92ab2 Mon Sep 17 00:00:00 2001 From: Andrew Karpow Date: Tue, 3 Nov 2026 13:58:20 -0500 Subject: [PATCH] Clean up orphaned test volumes during onboarding Aborted smoke test builds (e.g. volume quota exceeded) leave their boot volumes behind in the test project. Once the project exhausts its volume quota, every subsequent onboarding fails with VolumeLimitExceeded and the host is stuck. The onboarding controller now sweeps the test project for orphaned volumes after a test server ends up in error state and before creating a new test server. A volume is only deleted when it is unattached, named after an instance UUID, and no instance with that ID exists in the project anymore. Signed-off-by: Andrew Karpow --- internal/controller/onboarding_controller.go | 70 +++++++++++ .../controller/onboarding_controller_test.go | 118 +++++++++++++++++- 2 files changed, 187 insertions(+), 1 deletion(-) diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index f94a738f..aa923e90 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -24,6 +24,7 @@ import ( "net/http" "strings" "time" + "uuid" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -37,6 +38,7 @@ import ( logger "sigs.k8s.io/controller-runtime/pkg/log" "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/hypervisors" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/services" @@ -72,6 +74,7 @@ type OnboardingController struct { testComputeClient *gophercloud.ServiceClient testImageClient *gophercloud.ServiceClient testNetworkClient *gophercloud.ServiceClient + testVolumeClient *gophercloud.ServiceClient requeueInterval time.Duration } @@ -284,6 +287,12 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis log.Error(err, "failed to delete test instance", "id", id) } + // Aborted builds can leak the boot volume; delete it so the test + // project does not exhaust its volume quota. + if err := r.deleteOrphanedTestVolumes(ctx); err != nil { + log.Error(err, "failed to delete orphaned test volumes") + } + return ctrl.Result{RequeueAfter: defaultWaitTime}, nil case "ACTIVE": @@ -401,6 +410,57 @@ func (r *OnboardingController) deleteTestServers(ctx context.Context, host strin return errors.Join(errs...) } +// deleteOrphanedTestVolumes deletes unattached boot volumes left behind by +// aborted test server builds. Nova names boot volumes after the instance +// UUID, so a volume is only deleted when it is unattached and no instance +// with that ID exists in the project anymore. +func (r *OnboardingController) deleteOrphanedTestVolumes(ctx context.Context) error { + log := logger.FromContext(ctx) + + serverPages, err := servers.ListSimple(r.testComputeClient, servers.ListOpts{}).AllPages(ctx) + if err != nil { + return err + } + serverList, err := servers.ExtractServers(serverPages) + if err != nil { + return err + } + existingInstances := make(map[string]struct{}, len(serverList)) + for _, server := range serverList { + existingInstances[server.ID] = struct{}{} + } + + volumePages, err := volumes.List(r.testVolumeClient, volumes.ListOpts{}).AllPages(ctx) + if err != nil { + return err + } + volumeList, err := volumes.ExtractVolumes(volumePages) + if err != nil { + return err + } + + var errs []error + for _, volume := range volumeList { + if volume.Status != "available" { + continue + } + if _, err := uuid.Parse(volume.Name); err != nil { + continue + } + if _, exists := existingInstances[volume.Name]; exists { + continue + } + log.Info("deleting orphaned test volume", "id", volume.ID, "name", volume.Name) + if err := volumes.Delete(ctx, r.testVolumeClient, volume.ID, nil).ExtractErr(); err != nil { + if !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { + errs = append(errs, err) + } + } + } + + return errors.Join(errs...) +} + func (r *OnboardingController) lookupNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) (hypervisorID, serviceID string, err error) { hypervisorAddress := hv.Labels[corev1.LabelHostname] if hypervisorAddress == "" { @@ -478,6 +538,12 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, return foundServer, nil } + // Aborted builds can leak boot volumes, which eventually exhausts the + // test project's volume quota. + if err := r.deleteOrphanedTestVolumes(ctx); err != nil { + log.Error(err, "failed to delete orphaned test volumes") + } + flavorRef := r.TestFlavorID imageRef, err := r.findTestImage(ctx) @@ -605,6 +671,10 @@ func (r *OnboardingController) SetupWithManager(mgr ctrl.Manager) error { } r.testNetworkClient.ResourceBase = fmt.Sprintf("%vv2.0/", r.testNetworkClient.Endpoint) + if r.testVolumeClient, err = openstack.GetServiceClient(ctx, "volumev3", testAuth); err != nil { + return err + } + if r.Clock == nil { r.Clock = clock.RealClock{} } diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index b1d8b347..13349dae 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -227,6 +227,7 @@ var _ = Describe("Onboarding Controller", func() { testComputeClient: client.ServiceClient(fakeServer), testImageClient: client.ServiceClient(fakeServer), testNetworkClient: client.ServiceClient(fakeServer), + testVolumeClient: client.ServiceClient(fakeServer), } DeferCleanup(func() { @@ -412,6 +413,8 @@ var _ = Describe("Onboarding Controller", func() { serverActionHandler func(http.ResponseWriter, *http.Request) serverDeleteHandler func(http.ResponseWriter, *http.Request) serverDetailHandler func(http.ResponseWriter, *http.Request) + serverListHandler func(http.ResponseWriter, *http.Request) + volumeListHandler func(http.ResponseWriter, *http.Request) ) BeforeEach(func(ctx SpecContext) { @@ -433,11 +436,14 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) }) - fakeServer.Mux.HandleFunc("GET /servers", func(w http.ResponseWriter, r *http.Request) { + serverListHandler = func(w http.ResponseWriter, _ *http.Request) { w.Header().Add("Content-Type", "application/json") w.WriteHeader(http.StatusOK) _, err := fmt.Fprint(w, emptyServersBody) Expect(err).NotTo(HaveOccurred()) + } + fakeServer.Mux.HandleFunc("GET /servers", func(w http.ResponseWriter, r *http.Request) { + serverListHandler(w, r) }) serverDetailHandler = func(w http.ResponseWriter, _ *http.Request) { @@ -471,6 +477,16 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) }) + volumeListHandler = func(w http.ResponseWriter, _ *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := fmt.Fprint(w, `{"volumes": []}`) + Expect(err).NotTo(HaveOccurred()) + } + fakeServer.Mux.HandleFunc("GET /volumes/detail", func(w http.ResponseWriter, r *http.Request) { + volumeListHandler(w, r) + }) + fakeServer.Mux.HandleFunc("POST /servers", func(w http.ResponseWriter, r *http.Request) { w.Header().Add("Content-Type", "application/json") w.WriteHeader(http.StatusOK) @@ -716,5 +732,105 @@ var _ = Describe("Onboarding Controller", func() { }) }) + When("the test server ended up in an error state", func() { + var ( + serverDeleteCalls int + orphanedVolumeDeleteCalls int + liveVolumeDeleteCalls int + inUseVolumeDeleteCalls int + ) + + BeforeEach(func(ctx SpecContext) { + By("Overriding HV status to Testing state") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + // Construct the server name the controller will look for. + serverName := fmt.Sprintf("%s-%s-%s", testPrefixName, hypervisorName, string(hv.UID)) + + // GET /servers/detail returns the test server in ERROR state. + serverDetailHandler = func(w http.ResponseWriter, _ *http.Request) { + _, err := fmt.Fprintf(w, + `{"servers": [{"id": "server-id", "name": %q, "status": "ERROR"}], "servers_links": []}`, + serverName) + Expect(err).NotTo(HaveOccurred()) + } + + // The unfiltered server list holds an unrelated live instance + // whose boot volume must not be deleted. + serverListHandler = func(w http.ResponseWriter, _ *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := fmt.Fprint(w, `{"servers": [{"id": "c48f6247-abe4-4a24-824e-e820f-afd1c2bc7521", "name": "live-instance", "status": "ACTIVE"}]}`) + Expect(err).NotTo(HaveOccurred()) + } + + fakeServer.Mux.HandleFunc("GET /servers/server-id", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := fmt.Fprint(w, `{"server": {"id": "server-id", "name": "server-id", "status": "ERROR", "fault": {"message": "Build of instance 77b7129b-aecb-4d6e-820f-afd1c2bc7521 aborted: VolumeLimitExceeded: Maximum number of volumes allowed (100) exceeded for quota 'volumes_premium'."}}}`) + Expect(err).NotTo(HaveOccurred()) + }) + + serverDeleteHandler = func(w http.ResponseWriter, _ *http.Request) { + serverDeleteCalls++ + w.WriteHeader(http.StatusAccepted) + } + + // Three volumes: an orphaned boot volume (instance gone), the + // boot volume of the live instance (instance exists), and an + // in-use volume (attached). + volumeListHandler = func(w http.ResponseWriter, _ *http.Request) { + w.Header().Add("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := fmt.Fprint(w, `{"volumes": [ + {"id": "orphaned-volume-id", "name": "77b7129b-aecb-4d6e-820f-afd1c2bc7521", "status": "available"}, + {"id": "live-instance-volume-id", "name": "c48f6247-abe4-4a24-824e-e820f-afd1c2bc7521", "status": "available"}, + {"id": "in-use-volume-id", "name": "in-use-volume-id", "status": "in-use"} + ]}`) + Expect(err).NotTo(HaveOccurred()) + } + fakeServer.Mux.HandleFunc("DELETE /volumes/orphaned-volume-id", func(w http.ResponseWriter, _ *http.Request) { + orphanedVolumeDeleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + fakeServer.Mux.HandleFunc("DELETE /volumes/live-instance-volume-id", func(w http.ResponseWriter, _ *http.Request) { + liveVolumeDeleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + fakeServer.Mux.HandleFunc("DELETE /volumes/in-use-volume-id", func(w http.ResponseWriter, _ *http.Request) { + inUseVolumeDeleteCalls++ + w.WriteHeader(http.StatusAccepted) + }) + }) + + It("should delete the server and the orphaned volume and record the fault", func(ctx SpecContext) { + _, err := onboardingReconciler.Reconcile(ctx, reconcileReq) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying the failed server was deleted") + Expect(serverDeleteCalls).To(Equal(1)) + + By("Verifying only the orphaned volume was deleted") + Expect(orphanedVolumeDeleteCalls).To(Equal(1)) + Expect(liveVolumeDeleteCalls).To(BeZero()) + Expect(inUseVolumeDeleteCalls).To(BeZero()) + + By("Verifying the onboarding condition records the fault") + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + onboardingCond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + Expect(onboardingCond).NotTo(BeNil()) + Expect(onboardingCond.Reason).To(Equal(kvmv1.ConditionReasonTesting)) + Expect(onboardingCond.Message).To(ContainSubstring("VolumeLimitExceeded")) + }) + }) + }) })