Skip to content

ROSAENG-61173 | test: complete delete cluster command coverage #3372

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

Closed
olucasfreitas wants to merge 1 commit into from
Closed

ROSAENG-61173 | test: complete delete cluster command coverage #3372

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
27 changes: 21 additions & 6 deletions cmd/dlt/cluster/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ var args struct {
mode string
}

var confirmDelete = confirm.Confirm

var runUninstallLogs = func(clusterKey string) {
uninstallLogs.Cmd.Run(uninstallLogs.Cmd, []string{clusterKey})
}

var Cmd = &cobra.Command{
Use: "cluster",
Short: "Delete cluster",
Expand Down Expand Up @@ -76,27 +82,34 @@ func init() {
)
}

func run(_ *cobra.Command, _ []string) {
func run(cmd *cobra.Command, argv []string) {
r := rosa.NewRuntime().WithAWS().WithOCM()
defer r.Cleanup()

err := runWithRuntime(r, cmd, argv)
if err != nil {
r.Reporter.Errorf("%s", err)
os.Exit(1)
}
}

func runWithRuntime(r *rosa.Runtime, _ *cobra.Command, _ []string) error {
clusterKey := r.GetClusterKey()

if args.bestEffort {
r.Reporter.Warnf("Deleting cluster '%s' with 'best effort' means that certain resources may be left behind"+
" in AWS account '%s'. These resources will need to be deleted manually.", clusterKey, r.Creator.AccountID)
}

if !confirm.Confirm("delete cluster %s", clusterKey) {
os.Exit(0)
if !confirmDelete("delete cluster %s", clusterKey) {
return nil
}

cluster := r.FetchCluster()

err := handleClusterDelete(r, cluster, clusterKey, args.bestEffort)
if err != nil {
r.Reporter.Errorf("%s", err)
os.Exit(1)
return err
Comment on lines 110 to +112

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Wrap deletion errors with operation context.

Returning the raw SDK error makes the command failure harder to diagnose. Preserve it with %w while identifying the failed cluster operation.

Proposed fix 
 	err := handleClusterDelete(r, cluster, clusterKey, args.bestEffort)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to delete cluster %q: %w", clusterKey, err)
 	}

As per coding guidelines, “Wrap returned errors with context using %w; do not drop the original error.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err := handleClusterDelete(r, cluster, clusterKey, args.bestEffort)
if err != nil {
r.Reporter.Errorf("%s", err)
os.Exit(1)
return err
err := handleClusterDelete(r, cluster, clusterKey, args.bestEffort)
if err != nil {
return fmt.Errorf("failed to delete cluster %q: %w", clusterKey, err)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/dlt/cluster/cmd.go` around lines 110 - 112, Update the error handling
around handleClusterDelete to wrap the returned error with descriptive
cluster-deletion operation context using %w, preserving the original error for
unwrapping.

Source: Coding guidelines

}

if cluster.AWS().STS().RoleARN() != "" {
Expand All @@ -122,13 +135,15 @@ func run(_ *cobra.Command, _ []string) {
}
if args.watch {
arguments.DisableRegionDeprecationWarning = true // disable region deprecation warning
uninstallLogs.Cmd.Run(uninstallLogs.Cmd, []string{clusterKey})
runUninstallLogs(clusterKey)
arguments.DisableRegionDeprecationWarning = false // enable region deprecation again
} else {
r.Reporter.Infof("To watch your cluster uninstallation logs, run 'rosa logs uninstall -c %s --watch'",
clusterKey,
)
}

return nil
}

func handleClusterDelete(r *rosa.Runtime, cluster *cmv1.Cluster, clusterKey string, bestEffort bool) error {
Expand Down
254 changes: 254 additions & 0 deletions cmd/dlt/cluster/cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
. "github.com/openshift-online/ocm-sdk-go/testing"

"github.com/openshift/rosa/pkg/arguments"
"github.com/openshift/rosa/pkg/interactive"
"github.com/openshift/rosa/pkg/test"
)

Expand All @@ -21,6 +23,258 @@ var _ = Describe("Delete cluster", func() {
BeforeEach(func() {
t = test.NewTestRuntime()
clusterId = test.MockClusterID
args.bestEffort = false
args.watch = false
interactive.SetEnabled(false)
arguments.DisableRegionDeprecationWarning = false
})

Context("runWithRuntime", func() {
var argv []string

BeforeEach(func() {
argv = []string{}
args.bestEffort = false
args.watch = false
confirmDelete = func(string, ...interface{}) bool { return true }
runUninstallLogs = func(string) {}
interactive.SetEnabled(false)
arguments.DisableRegionDeprecationWarning = false
})

It("runs the non-STS happy path and prints the uninstall log hint", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, ""))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(ContainSubstring("will start uninstalling"))
Expect(stdout).To(ContainSubstring("rosa logs uninstall -c"))
Expect(stdout).To(ContainSubstring("--watch"))
})

It("returns cleanly when deletion is not confirmed", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)
confirmDelete = func(string, ...interface{}) bool { return false }

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(BeEmpty())
})

It("prints the best-effort warning and passes the flag through", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)
args.bestEffort = true

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, ""))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(ContainSubstring("best effort"))
Expect(stderr).To(ContainSubstring("certain resources may be left behind"))
Expect(stdout).To(ContainSubstring("will start uninstalling"))
})
Comment on lines +84 to +107

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that bestEffort reaches the delete request.

This test only verifies warning output and a successful response; the mock handler accepts the request regardless of the flag. Inspect the request in the handler or use a spy so the test fails unless DeleteCluster receives bestEffort=true.

As per path instructions, “Flag weak tests that only restate implementation or changes that weaken existing assertions.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/dlt/cluster/cmd_test.go` around lines 84 - 107, Strengthen the test
around runWithRuntime so the mocked delete request inspects its payload or query
and asserts that bestEffort is true, rather than accepting any request. Keep the
existing warning, success, and uninstall-output assertions, and ensure the test
fails if DeleteCluster does not receive the flag.

Source: Path instructions


It("prints STS cleanup guidance for clusters with operator roles", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(
cmv1.NewSTS().
RoleARN("arn:aws:iam::123456789012:role/Installer").
OIDCEndpointURL("https://oidc.example.com").
OperatorRolePrefix("my-prefix").
OperatorIAMRoles(
cmv1.NewOperatorIAMRole().
Name("ebs-cloud-credentials").
Namespace("openshift-cluster-csi-drivers").
RoleARN("arn:aws:iam::123456789012:role/op-role"),
),
))
})
t.SetCluster(clusterId, clusterReady)

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, ""))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(ContainSubstring("Operator IAM Roles:"))
Expect(stdout).To(ContainSubstring("arn:aws:iam::123456789012:role/op-role"))
Expect(stdout).To(ContainSubstring("OIDC Provider : https://oidc.example.com"))
Expect(stdout).To(ContainSubstring("rosa delete operator-roles -c"))
Expect(stdout).To(ContainSubstring("rosa delete oidc-provider -c"))
})

It("prints STS cleanup guidance without operator role output when none remain", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(
cmv1.NewSTS().
RoleARN("arn:aws:iam::123456789012:role/Installer").
OIDCEndpointURL("https://oidc-no-roles.example.com"),
))
})
t.SetCluster(clusterId, clusterReady)

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, ""))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).NotTo(ContainSubstring("Operator IAM Roles:"))
Expect(stdout).To(ContainSubstring("OIDC Provider : https://oidc-no-roles.example.com"))
Expect(stdout).To(ContainSubstring("rosa delete operator-roles -c"))
Expect(stdout).To(ContainSubstring("rosa delete oidc-provider -c"))
})

It("runs uninstall logs when watch is enabled and restores the deprecation warning flag", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)
args.watch = true

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, ""))

var watchedClusterKey string
var sawDisabledWarning bool
runUninstallLogs = func(clusterKey string) {
watchedClusterKey = clusterKey
sawDisabledWarning = arguments.DisableRegionDeprecationWarning
}

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(ContainSubstring("will start uninstalling"))
Expect(watchedClusterKey).To(Equal(clusterId))
Expect(sawDisabledWarning).To(BeTrue())
Expect(arguments.DisableRegionDeprecationWarning).To(BeFalse())
Expect(stdout).NotTo(ContainSubstring("rosa logs uninstall -c"))
})

It("returns the already-uninstalling path through the command wrapper", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "uninstalling"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).NotTo(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(ContainSubstring("already uninstalling"))
Expect(stdout).To(ContainSubstring("rosa logs uninstall -c"))
})

It("returns an error from GetClusterState through the command wrapper", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)

t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusInternalServerError, ""))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).To(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(BeEmpty())
Expect(err.Error()).To(ContainSubstring("expected response content type"))
})

It("returns a delete error through the command wrapper", func() {
clusterReady := test.MockCluster(func(c *cmv1.ClusterBuilder) {
c.State(cmv1.ClusterStateReady)
c.AWS(cmv1.NewAWS().STS(cmv1.NewSTS()))
})
t.SetCluster(clusterId, clusterReady)

statusBody := fmt.Sprintf(`{
"kind": "ClusterStatus",
"id": "%s",
"state": "ready"
}`, clusterId)
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusOK, statusBody))
t.ApiServer.AppendHandlers(RespondWithJSON(
http.StatusOK, test.FormatClusterList([]*cmv1.Cluster{clusterReady})))
t.ApiServer.AppendHandlers(RespondWithJSON(http.StatusForbidden, `{
"kind": "Error",
"id": "403",
"href": "/api/clusters_mgmt/v1/errors/403",
"code": "CLUSTERS-MGMT-403",
"reason": "forbidden"
}`))

stdout, stderr, err := test.RunWithOutputCaptureAndArgv(runWithRuntime, t.RosaRuntime, Cmd, &argv)
Expect(err).To(HaveOccurred())
Expect(stderr).To(BeEmpty())
Expect(stdout).To(BeEmpty())
Expect(err.Error()).To(ContainSubstring("forbidden"))
})
})

Context("handleClusterDelete", func() {
Expand Down