-
Notifications
You must be signed in to change notification settings - Fork 107
CLOUDP-212108: Fix net peering cleanups #1265
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.
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
Conversation
Signed-off-by: Jose Vazquez <jose.vazquez@mongodb.com>
90405ba to
ad0d922
Compare
| if err != nil { | ||
| fmt.Println(text.FgRed.Sprintf("\tFailed to list networking peering for project %s: %s", projectID, err)) | ||
| var ( | ||
| SupportedProviders = []string{"AWS", "AZURE", "GCP"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In atlas.go there are already constants with the supported cloud providers, please reuse them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| SupportedProviders = []string{"AWS", "AZURE", "GCP"} | ||
| ) | ||
|
|
||
| func (c *Cleaner) listNetworkPeering(ctx context.Context, projectID string) []admin.BaseNetworkPeeringConnectionSettings { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See GetProjectDependencies() in projects.go and consider move the provider logic to there.
Reason is that Gov only support AWS and make a request for other provider results in an error
| } | ||
|
|
||
| func (gcp *GCP) DeleteVpc(ctx context.Context, vpcName string) error { | ||
| vpcGetRequest := &computepb.GetNetworkRequest{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does GCP require subnets to be remove VPCs with subnets? seems strange
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it failed on my test until I added this code.
Seems that
ListPeeringConnectionswould render only AWS related peering by default.Apart from that, GCP VPC removals could not be done before removing their subnets.
Fixing both here should probably fix the issues for GCP. For Azure we have not detected stuck entries so far.
All Submissions: