Skip to content

feat: add dry run option for cleanup subcommand #21

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
mrhillsman merged 2 commits into from
Feb 23, 2026
Merged

feat: add dry run option for cleanup subcommand #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40a805e
feat: add dry run option for cleanup subcommand
OchiengEd Feb 16, 2026
562d622
fix: mark the dry run output clearly
OchiengEd Feb 23, 2026
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
20 changes: 15 additions & 5 deletions cmd/virtwork/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func newCleanupCmd() *cobra.Command {

cmd.Flags().Bool("delete-namespace", false, "Also delete the namespace")
cmd.Flags().String("run-id", "", "Only delete resources from this specific run (UUID)")
cmd.Flags().Bool("dry-run", false, "Print intent without destroying resources")
return cmd
}

Expand Down Expand Up @@ -490,7 +491,16 @@ func cleanupE(cmd *cobra.Command, args []string) error {
ctx := context.Background()

// Start audit execution
execID, _, err := auditor.StartExecution(ctx, "cleanup", cfg)
var dryRunBanner string = ""
cmdName := "cleanup"
if cfg.DryRun {
// Log the command being executed
cmdName = "cleanup --dry-run"
// Add banner if '--dry-run' option is passed
dryRunBanner = "--- Dry Run ---\n"
}

execID, _, err := auditor.StartExecution(ctx, cmdName, cfg)
if err != nil {
return fmt.Errorf("starting audit execution: %w", err)
}
Expand All @@ -505,15 +515,15 @@ func cleanupE(cmd *cobra.Command, args []string) error {

_ = auditor.RecordEvent(ctx, execID, audit.EventRecord{
EventType: "cleanup_started",
Message: fmt.Sprintf("Cleanup started (namespace: %s, run-id filter: %q)", cfg.Namespace, targetRunID),
Message: fmt.Sprintf("Started %s: (namespace: %s, run-id filter: %q)", cmdName, cfg.Namespace, targetRunID),
})

c, err := cluster.Connect(cfg.KubeconfigPath)
if err != nil {
return fmt.Errorf("connecting to cluster: %w", err)
}

result, err := cleanup.CleanupAll(ctx, c, cfg.Namespace, deleteNS, targetRunID)
result, err := cleanup.CleanupAll(ctx, c, cfg, deleteNS, targetRunID)
if err != nil {
return fmt.Errorf("cleanup failed: %w", err)
}
Expand All @@ -537,8 +547,8 @@ func cleanupE(cmd *cobra.Command, args []string) error {
_ = auditor.CompleteExecution(ctx, execID, "success", "")
err = nil // clear for defer

fmt.Fprintf(cmd.OutOrStdout(), "Cleanup complete: %d VMs deleted, %d services deleted, %d secrets deleted",
result.VMsDeleted, result.ServicesDeleted, result.SecretsDeleted)
fmt.Fprintf(cmd.OutOrStdout(), "%sCleanup complete: %d VMs deleted, %d services deleted, %d secrets deleted",
dryRunBanner, result.VMsDeleted, result.ServicesDeleted, result.SecretsDeleted)
if result.NamespaceDeleted {
fmt.Fprintf(cmd.OutOrStdout(), ", namespace deleted")
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/virtwork/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ var _ = Describe("Cleanup command", func() {
Build()
ctx := context.Background()

result, err := cleanup.CleanupAll(ctx, c, constants.DefaultNamespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: constants.DefaultNamespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(1))
Expect(result.NamespaceDeleted).To(BeFalse())
Expand Down Expand Up @@ -677,7 +677,7 @@ var _ = Describe("CLI end-to-end scenarios", func() {
Build()
ctx := context.Background()

result, err := cleanup.CleanupAll(ctx, c, constants.DefaultNamespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: constants.DefaultNamespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(2))
})
Expand Down
28 changes: 18 additions & 10 deletions internal/cleanup/cleanup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
kubevirtv1 "kubevirt.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/opdev/virtwork/internal/config"
"github.com/opdev/virtwork/internal/constants"
)

Expand All @@ -30,7 +31,7 @@ type CleanupResult struct {
// If runID is non-empty, only resources with that specific virtwork/run-id label are deleted.
// Individual deletion failures are recorded but do not abort the operation.
// If deleteNamespace is true, the namespace itself is deleted as the final step.
func CleanupAll(ctx context.Context, c client.Client, namespace string, deleteNamespace bool, runID string) (*CleanupResult, error) {
func CleanupAll(ctx context.Context, c client.Client, cfg *config.Config, deleteNamespace bool, runID string) (*CleanupResult, error) {

@OchiengEd OchiengEd Feb 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In lieu of adding an argument for dry run, I chose to pass in the *config.Config from which we can determine the namespace and if dry run is set to true.

result := &CleanupResult{}
managedLabels := map[string]string{
constants.LabelManagedBy: constants.ManagedByValue,
Expand All @@ -41,18 +42,25 @@ func CleanupAll(ctx context.Context, c client.Client, namespace string, deleteNa

runIDSet := make(map[string]struct{})

// Setup delete options
opts := []client.DeleteOption{}
if cfg.DryRun {

@OchiengEd OchiengEd Feb 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When dry run is true, controller runtime delete will be called with the dry run all option.

opts = append(opts, client.DryRunAll)
}

// Delete VMs by label
vmList := &kubevirtv1.VirtualMachineList{}
listOpts := []client.ListOption{
client.InNamespace(namespace),
client.InNamespace(cfg.Namespace),
client.MatchingLabels(managedLabels),
}
if err := c.List(ctx, vmList, listOpts...); err != nil {
return result, fmt.Errorf("listing VMs in %s: %w", namespace, err)
return result, fmt.Errorf("listing VMs in %s: %w", cfg.Namespace, err)
}

for i := range vmList.Items {
collectRunID(vmList.Items[i].Labels, runIDSet)
if err := c.Delete(ctx, &vmList.Items[i]); err != nil {
if err := c.Delete(ctx, &vmList.Items[i], opts...); err != nil {
if !apierrors.IsNotFound(err) {
result.Errors = append(result.Errors, fmt.Errorf("deleting VM %s: %w", vmList.Items[i].Name, err))
}
Expand All @@ -64,11 +72,11 @@ func CleanupAll(ctx context.Context, c client.Client, namespace string, deleteNa
// Delete services by label
svcList := &corev1.ServiceList{}
if err := c.List(ctx, svcList, listOpts...); err != nil {
return result, fmt.Errorf("listing services in %s: %w", namespace, err)
return result, fmt.Errorf("listing services in %s: %w", cfg.Namespace, err)
}
for i := range svcList.Items {
collectRunID(svcList.Items[i].Labels, runIDSet)
if err := c.Delete(ctx, &svcList.Items[i]); err != nil {
if err := c.Delete(ctx, &svcList.Items[i], opts...); err != nil {
if !apierrors.IsNotFound(err) {
result.Errors = append(result.Errors, fmt.Errorf("deleting service %s: %w", svcList.Items[i].Name, err))
}
Expand All @@ -80,11 +88,11 @@ func CleanupAll(ctx context.Context, c client.Client, namespace string, deleteNa
// Delete secrets by label
secretList := &corev1.SecretList{}
if err := c.List(ctx, secretList, listOpts...); err != nil {
return result, fmt.Errorf("listing secrets in %s: %w", namespace, err)
return result, fmt.Errorf("listing secrets in %s: %w", cfg.Namespace, err)
}
for i := range secretList.Items {
collectRunID(secretList.Items[i].Labels, runIDSet)
if err := c.Delete(ctx, &secretList.Items[i]); err != nil {
if err := c.Delete(ctx, &secretList.Items[i], opts...); err != nil {
if !apierrors.IsNotFound(err) {
result.Errors = append(result.Errors, fmt.Errorf("deleting secret %s: %w", secretList.Items[i].Name, err))
}
Expand All @@ -102,12 +110,12 @@ func CleanupAll(ctx context.Context, c client.Client, namespace string, deleteNa
if deleteNamespace {
ns := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Name: cfg.Namespace,
},
}
if err := c.Delete(ctx, ns); err != nil {
if !apierrors.IsNotFound(err) {
result.Errors = append(result.Errors, fmt.Errorf("deleting namespace %s: %w", namespace, err))
result.Errors = append(result.Errors, fmt.Errorf("deleting namespace %s: %w", cfg.Namespace, err))
}
} else {
result.NamespaceDeleted = true
Expand Down
25 changes: 13 additions & 12 deletions internal/cleanup/cleanup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/opdev/virtwork/internal/cleanup"
"github.com/opdev/virtwork/internal/cluster"
"github.com/opdev/virtwork/internal/config"
"github.com/opdev/virtwork/internal/constants"
"github.com/opdev/virtwork/internal/vm"
)
Expand Down Expand Up @@ -82,7 +83,7 @@ var _ = Describe("CleanupAll", func() {
vm2 := newManagedVM("vm-2")
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm1, vm2).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(2))
Expect(result.Errors).To(BeEmpty())
Expand Down Expand Up @@ -113,7 +114,7 @@ var _ = Describe("CleanupAll", func() {
}).
Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(1))
Expect(result.Errors).To(HaveLen(1))
Expand All @@ -124,7 +125,7 @@ var _ = Describe("CleanupAll", func() {
svc2 := newManagedService("svc-2")
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(svc1, svc2).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.ServicesDeleted).To(Equal(2))
Expect(result.Errors).To(BeEmpty())
Expand Down Expand Up @@ -155,7 +156,7 @@ var _ = Describe("CleanupAll", func() {
}).
Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.ServicesDeleted).To(Equal(1))
Expect(result.Errors).To(HaveLen(1))
Expand All @@ -166,7 +167,7 @@ var _ = Describe("CleanupAll", func() {
sec2 := newManagedSecret("sec-2")
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sec1, sec2).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.SecretsDeleted).To(Equal(2))
Expect(result.Errors).To(BeEmpty())
Expand Down Expand Up @@ -196,7 +197,7 @@ var _ = Describe("CleanupAll", func() {
}).
Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.SecretsDeleted).To(Equal(1))
Expect(result.Errors).To(HaveLen(1))
Expand All @@ -211,7 +212,7 @@ var _ = Describe("CleanupAll", func() {
}
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ns).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.NamespaceDeleted).To(BeFalse())

Expand All @@ -229,7 +230,7 @@ var _ = Describe("CleanupAll", func() {
}
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ns).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, true, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, true, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.NamespaceDeleted).To(BeTrue())
})
Expand All @@ -247,7 +248,7 @@ var _ = Describe("CleanupAll", func() {
}).
Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, true, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, true, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.NamespaceDeleted).To(BeFalse())
Expect(result.Errors).To(HaveLen(1))
Expand All @@ -261,7 +262,7 @@ var _ = Describe("CleanupAll", func() {
sec1 := newManagedSecret("sec-1")
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vm1, vm2, vm3, svc1, sec1).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(3))
Expect(result.ServicesDeleted).To(Equal(1))
Expand All @@ -273,7 +274,7 @@ var _ = Describe("CleanupAll", func() {
It("should handle empty namespace gracefully", func() {
c := fake.NewClientBuilder().WithScheme(scheme).Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(0))
Expect(result.ServicesDeleted).To(Equal(0))
Expand Down Expand Up @@ -317,7 +318,7 @@ var _ = Describe("CleanupAll", func() {
WithObjects(managedVM, unmanagedVM, managedSvc, unmanagedSvc).
Build()

result, err := cleanup.CleanupAll(ctx, c, namespace, false, "")
result, err := cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, false, "")
Expect(err).NotTo(HaveOccurred())
Expect(result.VMsDeleted).To(Equal(1))
Expect(result.ServicesDeleted).To(Equal(1))
Expand Down
3 changes: 2 additions & 1 deletion internal/testutil/testutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/opdev/virtwork/internal/cleanup"
"github.com/opdev/virtwork/internal/cluster"
"github.com/opdev/virtwork/internal/config"
"github.com/opdev/virtwork/internal/constants"
"github.com/opdev/virtwork/internal/vm"
)
Expand Down Expand Up @@ -61,7 +62,7 @@ func ManagedLabels() map[string]string {
// then deletes the namespace itself. Errors are logged but do not cause panic.
// Suitable for use with Ginkgo's DeferCleanup.
func CleanupNamespace(ctx context.Context, c client.Client, namespace string) {
_, _ = cleanup.CleanupAll(ctx, c, namespace, true, "")
_, _ = cleanup.CleanupAll(ctx, c, &config.Config{Namespace: namespace}, true, "")
}

// DefaultVMOpts returns a minimal VMSpecOpts suitable for integration tests.
Expand Down