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

Zone parameter in subnet.create() #63

Closed
nuwang opened this issue Aug 31, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@nuwang
Copy link
Contributor

commented Aug 31, 2017

Do we need the zone parameter anymore? It would seem like we should either make it mandatory or remove it altogether?

@afgane

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2017

Amazon's subnets are zone-specific. So if a user requests placement for a launched instance, we need to be able to specify the zone where to create the subnet. An alternative is to create the subnet in every zone in the region but then the create method would return multiple subnets for AWS, which makes no sense. On the other hand, OpenStack ignores subnet zone altogether.

So it's easy to require it and simply ignore it for OS. If required, it may make the CloudLaunch a bit more complicated because it will be required to supply the subnet while ensuring it matches optionally selected placement.

@nuwang nuwang added this to the release 1.0.0 milestone Oct 1, 2017

@nuwang

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

conclusion: make zone mandatory for subnet.create()

@Dyex719

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

All the instances of subnet.create() https://github.com/gvlproject/cloudbridge/search?l=Python&q=subnets.create&type=&utf8=%E2%9C%93 are included in the function create_subnet() https://github.com/gvlproject/cloudbridge/blob/0fb03d21102466250be5a1d8056dd05091f690e5/cloudbridge/cloud/base/resources.py/#L940 which takes a 'zone=None' parameter.
Should changing this to 'zone=zone' be enough?

@Dyex719

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

@nuwang

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2017

@Dyex719 Yes, that sounds like a good plan. Starting with changing the test to include the zone parameter would probably be best.

 return self.provider.networking.subnets.create( 
     network=net, cidr_block="10.0.0.1/24", name=name,
     zone=helpers.get_provider_test_data(self.provider, 'placement')

Dyex719 added a commit to Dyex719/cloudbridge that referenced this issue Oct 27, 2017

Issue CloudVE#63
Adding the zone parameter to create_subnet
@Dyex719

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

Please refer to #101.
Also should what I mentioned in above be done? That is, adding a zone parameter to https://github.com/gvlproject/cloudbridge/blob/0fb03d21102466250be5a1d8056dd05091f690e5/cloudbridge/cloud/base/resources.py/#L940 to make it mandatory.

@afgane

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

Thanks for taking this on. Modifying the test is only a part of this though. It's also necessary to modify the interface for theSubnetService create method (https://github.com/gvlproject/cloudbridge/blob/master/cloudbridge/cloud/interfaces/services.py#L743) by removing the default None argument to the zone parameter. After that, the same should be done for each of the provider implementations. Finally, an update to the docs in the interface file to just remove the word optional for the zone parameter.

@afgane

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

Also, see the Travis tests output - there's a flake8 error: https://travis-ci.org/gvlproject/cloudbridge/jobs/293663371#L584

@afgane

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2018

Done in #101.

@afgane afgane closed this Aug 2, 2018

@afgane afgane removed the in progress label Aug 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.