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.

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

Oracle allow explicitly specifying vcn for subnet selection #315

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

a-dubs
Copy link
Collaborator

@a-dubs a-dubs commented Aug 9, 2023

feat(oracle): ability to specify vcn by name for subnet selection

Oracle's pycloudlib implementation currently uses the most recently created VCN (Virtual Cloud Network) in the given compartment for new VM instances being created. Problems can therefore arise if a newly created VCN uses a subnet with different rules than the previously used VCN's subnet.

Now, instead of relying on just blindly choosing the most recently created VCN, pycloudlib supports specifying a vcn_name value which it will use to find a VCN that has a name that exactly matches the given vcn_name.

@a-dubs a-dubs changed the title Oracle allow explicitly specifying vcn for subnet selection Oracle allow explicitly specifying vcn for subnet selection (CPC-2908) Aug 15, 2023
@a-dubs a-dubs changed the title Oracle allow explicitly specifying vcn for subnet selection (CPC-2908) Oracle allow explicitly specifying vcn for subnet selection Aug 15, 2023
@a-dubs a-dubs force-pushed the oracle-allow-specifying-vcn branch 3 times, most recently from fb1e3ee to c603b58 Compare August 16, 2023 04:27
Copy link
Contributor

@CalvoM CalvoM left a comment

Choose a reason for hiding this comment

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

LGTM, but I need to do some testing for the functionality.

@@ -47,13 +48,27 @@ def get_subnet_id(
network_client: Instance of VirtualNetworkClient.
compartment_id: Compartment where the subnet has to belong
availability_domain: Domain to look for subnet id in.
target_subnet_name: Name of the subnet to look for.
Copy link
Member

Choose a reason for hiding this comment

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

Is this line needed?

@a-dubs a-dubs force-pushed the oracle-allow-specifying-vcn branch from c603b58 to 692bbbe Compare August 17, 2023 15:15
Copy link
Contributor

@CalvoM CalvoM left a comment

Choose a reason for hiding this comment

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

Tested it with CTF and it works. LGTM.

@a-dubs a-dubs requested a review from holmanb August 22, 2023 13:25
Copy link
Collaborator

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

Thanks! Apart from the typing fix, LGTM.

pycloudlib/oci/utils.py Outdated Show resolved Hide resolved
Oracle's pycloudlib implementation currently uses the most recently created VCN (Virtual Cloud Network) in the given compartment for new VM instances being created. Problems can therefore arise if a newly created VCN uses a subnet with different rules than the previously used VCN's subnet.
Now, instead of relying on just blindly choosing the most recently created VCN, pycloudlib supports specifying a vcn_name value which it will use to find a VCN that has a name that exactly matches the given vcn_name.
@a-dubs a-dubs force-pushed the oracle-allow-specifying-vcn branch from 692bbbe to 461cbee Compare August 24, 2023 15:47
Copy link
Collaborator

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@aciba90 aciba90 merged commit 31940ab into canonical:main Aug 24, 2023
4 checks passed
@a-dubs
Copy link
Collaborator Author

a-dubs commented Aug 24, 2023

Jira ticket for context: https://warthogs.atlassian.net/browse/CPC-2908

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants