Skip to content
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

Jump to bottom

Do not perform explicit cluster deletion in prod e2e #3513

Merged
merged 3 commits into from
Apr 25, 2024
Merged

Do not perform explicit cluster deletion in prod e2e #3513

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
508ce32
Do not perform explicit cluster deletion in prod e2e
tsatam Apr 10, 2024
85a8435
Add more robust logging to deletion process
tsatam Apr 10, 2024
d69240c
Refactor pkg/util/cluster Delete function to declaratively define del…
tsatam Apr 17, 2024
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
39 changes: 31 additions & 8 deletions pkg/util/cluster/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,22 +388,33 @@ func (c *Cluster) Delete(ctx context.Context, vnetResourceGroup, clusterName str
c.log.Print("deleting role assignments")
err = c.deleteRoleAssignments(ctx, vnetResourceGroup, *oc.OpenShiftClusterProperties.ServicePrincipalProfile.ClientID)
if err != nil {
errs = append(errs, err)
}

c.log.Print("deleting cluster")
err = c.openshiftclustersv20200430.DeleteAndWait(ctx, vnetResourceGroup, clusterName)
if err != nil {
c.log.Errorf("error when deleting role assignments: %v", err)
errs = append(errs, err)
}
}

if c.ci {
// Only perform explicit cluster deletion when in local development mode, otherwise
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why doesn't deleting the resource group only work in local development mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Discussed during sync call: The implementation here, with the two boolean flags (CI and localDevelopmentMode) leads to confusion on which flags are set and what behavior we expect for each permutation. We should follow-up this change with a refactor to make it more explicit and clear what steps are being performed in what contexts, even if it does lead to code duplication.

// we let the resource group deletion performed below clean up the cluster.
//
// When the cluster resource is known by ARM (e.g. prod e2e), deleting the cluster
// here can result in race conditions when deleting the resource group below,
// as ARM will also attempt to delete the cluster.
if env.IsLocalDevelopmentMode() {
c.log.Print("deleting cluster")
err = c.openshiftclustersv20200430.DeleteAndWait(ctx, vnetResourceGroup, clusterName)
if err != nil {
c.log.Errorf("error when deleting cluster: %v", err)
errs = append(errs, err)
}
}

_, err = c.groups.Get(ctx, vnetResourceGroup)
if err == nil {
c.log.Print("deleting resource group")
err = c.groups.DeleteAndWait(ctx, vnetResourceGroup)
if err != nil {
c.log.Errorf("error when deleting resource group: %v", err)
errs = append(errs, err)
}
}
Expand All @@ -414,41 +425,53 @@ func (c *Cluster) Delete(ctx context.Context, vnetResourceGroup, clusterName str
err = c.ciParentVnetPeerings.DeleteAndWait(ctx, r.ResourceGroup, r.ResourceName, vnetResourceGroup+"-peer")
}
if err != nil {
c.log.Errorf("error when deleting vnet peerings: %v", err)
errs = append(errs, err)
}
}
} else {
c.log.Print("deleting cluster")
err = c.openshiftclustersv20200430.DeleteAndWait(ctx, vnetResourceGroup, clusterName)
if err != nil {
c.log.Errorf("error when deleting cluster: %v", err)
errs = append(errs, err)
}

// Deleting the deployment does not clean up the associated resources
c.log.Info("deleting deployment")
err = c.deployments.DeleteAndWait(ctx, vnetResourceGroup, clusterName)
if err != nil {
c.log.Errorf("error when deleting deployment: %v", err)
errs = append(errs, err)
}

c.log.Info("deleting master/worker subnets")
err = c.subnets.DeleteAndWait(ctx, vnetResourceGroup, "dev-vnet", clusterName+"-master")
if err != nil {
c.log.Errorf("error when deleting master subnet: %v", err)
errs = append(errs, err)
}

err = c.subnets.DeleteAndWait(ctx, vnetResourceGroup, "dev-vnet", clusterName+"-worker")
if err != nil {
c.log.Errorf("error when deleting worker subnet: %v", err)
errs = append(errs, err)
}

c.log.Info("deleting route table")
err = c.routetables.DeleteAndWait(ctx, vnetResourceGroup, clusterName+"-rt")
if err != nil {
c.log.Errorf("error when deleting route table: %v", err)
errs = append(errs, err)
}
}

c.log.Info("done")

if errs != nil {
c.log.Error("done, errors encountered")
return errs // https://golang.org/doc/faq#nil_error
}

c.log.Info("done")
return nil
}

Expand Down
Loading