Skip to content

Clean up orphaned test volumes during onboarding #385

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.

Sign up for GitHub

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

Merged
notandy merged 1 commit into from
Nov 4, 2026
Merged

Clean up orphaned test volumes during onboarding #385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions internal/controller/onboarding_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net/http"
"strings"
"time"
"uuid"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -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"
Expand Down Expand Up @@ -72,6 +74,7 @@ type OnboardingController struct {
testComputeClient *gophercloud.ServiceClient
testImageClient *gophercloud.ServiceClient
testNetworkClient *gophercloud.ServiceClient
testVolumeClient *gophercloud.ServiceClient
requeueInterval time.Duration
}

Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{}
}
Expand Down
118 changes: 117 additions & 1 deletion internal/controller/onboarding_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"))
})
})

})
})
Loading