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

Add VPCZones #12

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Add VPCZones #12

merged 1 commit into from
Jun 10, 2024

Conversation

dharaneeshvrd
Copy link
Contributor

Add toronto region
Add few utility functions
Removed VPCRegion for Montreal since it does not have respective VPC region

@ppc64le-cloud-bot ppc64le-cloud-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 4, 2024
@dharaneeshvrd
Copy link
Contributor Author

/cc @mkumatag @Karthik-K-N

mkumatag
mkumatag requested changes Jun 5, 2024
View reviewed changes
Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

Need needs a revisit based on the data what is been published in the powervs doc here - https://cloud.ibm.com/docs/power-iaas?topic=power-iaas-creating-power-virtual-server

and also please confirm the powervs zones to the colocated vpc zones mapping

@dharaneeshvrd dharaneeshvrd force-pushed the update-region branch 2 times, most recently from 67b2d48 to 62d3d24 Compare June 5, 2024 06:05
Karthik-K-N
Karthik-K-N reviewed Jun 5, 2024
View reviewed changes
region.go
SysTypes: []string{"s922", "e980"},
VPCZones: []string{"ca-tor-1", "ca-tor-2", "ca-tor-3"},
},
"us-south": {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving this next to dal region block and adding a comment stating why we added it as separate entry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it in same order to maintain the alpha ascending order. Just added a comment.

@dharaneeshvrd dharaneeshvrd force-pushed the update-region branch 2 times, most recently from e832f78 to 23e6bd4 Compare June 5, 2024 06:48
@dharaneeshvrd
Add VPCZones
2d1707a 
Add toronto & us-south region
Update us-east region
Add an utility function
Remove VPCRegion for Montreal since it does not have respective VPC region
@dharaneeshvrd
Copy link
Contributor Author

@mkumatag Refactored now, region is not going to be used anywhere, just used as a plain string to map the respective values.
Segregated us-south as separate since key should be prefix to easily retrieve mapping from powervs zone, us-east also treated like that.
Regarding VPC zones, I have added all 3 zones for all the regions since it's possible to create subnets in them.

mkumatag
mkumatag approved these changes Jun 7, 2024
View reviewed changes
Copy link
Member

@mkumatag mkumatag left a comment

Choose a reason for hiding this comment

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

I will let @Karthik-K-N to LGTM

@ppc64le-cloud-bot ppc64le-cloud-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2024
Karthik-K-N
Karthik-K-N approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@Karthik-K-N Karthik-K-N left a comment

Choose a reason for hiding this comment

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

/lgtm

@ppc64le-cloud-bot ppc64le-cloud-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2024
@ppc64le-cloud-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharaneeshvrd, Karthik-K-N, mkumatag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ppc64le-cloud-bot ppc64le-cloud-bot merged commit 6bdc07d into ppc64le-cloud:main Jun 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkumatag mkumatag mkumatag approved these changes

@Karthik-K-N Karthik-K-N Karthik-K-N approved these changes

@bkhadars bkhadars Awaiting requested review from bkhadars

Assignees

@Karthik-K-N Karthik-K-N

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dharaneeshvrd @ppc64le-cloud-bot @mkumatag @Karthik-K-N