You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2014/01/08 00:19:43 UTC
[jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
- Adds networks field/methods to TemplateOptions.
- Adds them to children as well for legacy reasons.
- Deprecates CloudStackTemplateOptions#networkIds methods in favor of #networks.
- TODO: Modify compute abstraction layer for provisioning for nova,
EC2, et al to take advantage of this.
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 jclouds-416
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/246
-- Commit Summary --
* JCLOUDS-416. Add TemplateOptions#networks.
-- File Changes --
M apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/options/CloudSigmaTemplateOptions.java (20)
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/options/CloudStackTemplateOptions.java (59)
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/AdvancedNetworkOptionsConverter.java (21)
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/BasicNetworkOptionsConverter.java (8)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/CloudStackExperimentLiveTest.java (24)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/options/CloudStackTemplateOptionsTest.java (45)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/strategy/OptionsConverterTest.java (17)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/options/EC2TemplateOptions.java (47)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java (15)
M apis/vcloud/src/main/java/org/jclouds/vcloud/compute/options/VCloudTemplateOptions.java (16)
M compute/src/main/java/org/jclouds/compute/options/TemplateOptions.java (55)
M compute/src/test/java/org/jclouds/compute/options/TemplateOptionsTest.java (23)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateOptions.java (16)
M providers/glesys/src/main/java/org/jclouds/glesys/compute/options/GleSYSTemplateOptions.java (16)
M providers/gogrid/src/main/java/org/jclouds/gogrid/compute/options/GoGridTemplateOptions.java (16)
M providers/softlayer/src/main/java/org/jclouds/softlayer/compute/options/SoftLayerTemplateOptions.java (16)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/246.patch
https://github.com/jclouds/jclouds/pull/246.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Bayer <no...@github.com>.
> @@ -196,4 +190,11 @@ public void testNodeNames() {
> TemplateOptions options = nodeNames(nodeNames);
> assertTrue(options.getNodeNames().containsAll(nodeNames));
> }
> +
> + @Test
> + public void testNetworks() {
> + Set<String> networks = ImmutableSet.of("first-network", "second-network");
> + TemplateOptions options = networks(networks);
> + assertTrue(options.getNetworks().containsAll(networks));
I think this is actually me hitting some weirdness in ordering somewhere and deciding to shortcut it. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8712001
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -43,8 +40,8 @@ public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptio
> // at least one network must be given to CloudStack,
> // but jclouds will try to autodetect an appropriate network if none given.
> checkArgument(templateOptions.getSecurityGroupIds().isEmpty(), "security groups cannot be specified for locations (zones) that use advanced networking");
> - if (templateOptions.getNetworkIds().size() > 0) {
> - options.networkIds(templateOptions.getNetworkIds());
> + if (templateOptions.getNetworks().size() > 0) {
`if (!templateOptions.getNetworks().isEmpty())`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711455
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Bayer <no...@github.com>.
> @@ -468,4 +492,21 @@ public CloudStackTemplateOptions userMetadata(String key, String value) {
> public CloudStackTemplateOptions nodeNames(Iterable<String> nodeNames) {
> return CloudStackTemplateOptions.class.cast(super.nodeNames(nodeNames));
> }
> +
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public CloudStackTemplateOptions networks(Iterable<String> networks) {
> + return CloudStackTemplateOptions.class.cast(super.networks(networks));
> + }
> +
> +
Found that and cleaned it up. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711894
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #962](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/962/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31794051
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Bayer <no...@github.com>.
> @@ -43,8 +40,8 @@ public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptio
> // at least one network must be given to CloudStack,
> // but jclouds will try to autodetect an appropriate network if none given.
> checkArgument(templateOptions.getSecurityGroupIds().isEmpty(), "security groups cannot be specified for locations (zones) that use advanced networking");
> - if (templateOptions.getNetworkIds().size() > 0) {
> - options.networkIds(templateOptions.getNetworkIds());
> + if (templateOptions.getNetworks().size() > 0) {
Dang, I missed one!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711900
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -35,8 +35,8 @@ public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptio
> if (templateOptions.getSecurityGroupIds().size() > 0) {
> options.securityGroupIds(templateOptions.getSecurityGroupIds());
> }
> - if (templateOptions.getNetworkIds().size() > 0) {
> - options.networkIds(templateOptions.getNetworkIds());
> + if (templateOptions.getNetworks().size() > 0) {
`if (!templateOptions.getNetworks().isEmpty())`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711459
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -196,4 +190,11 @@ public void testNodeNames() {
> TemplateOptions options = nodeNames(nodeNames);
> assertTrue(options.getNodeNames().containsAll(nodeNames));
> }
> +
> + @Test
> + public void testNetworks() {
> + Set<String> networks = ImmutableSet.of("first-network", "second-network");
> + TemplateOptions options = networks(networks);
> + assertTrue(options.getNetworks().containsAll(networks));
Stupid question, but is this what we're trying to test, or that the networks are _equal_? I.e. would it be OK if `options.getNetworks()` had **more** entries?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711540
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #498](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/498/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31793951
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -114,6 +114,14 @@ public static CloudSigmaTemplateOptions nodeNames(Iterable<String> nodeNames) {
> return CloudSigmaTemplateOptions.class.cast(options.nodeNames(nodeNames));
> }
>
> + /**
> + * @see TemplateOptions#nodeNames(Iterable)
> + */
> + public static CloudSigmaTemplateOptions networks(Iterable<String> networks) {
> + CloudSigmaTemplateOptions options = new CloudSigmaTemplateOptions();
> + return CloudSigmaTemplateOptions.class.cast(options.nodeNames(networks));
Eek...good catch! Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8742728
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -114,6 +114,14 @@ public static CloudSigmaTemplateOptions nodeNames(Iterable<String> nodeNames) {
> return CloudSigmaTemplateOptions.class.cast(options.nodeNames(nodeNames));
> }
>
> + /**
> + * @see TemplateOptions#nodeNames(Iterable)
> + */
> + public static CloudSigmaTemplateOptions networks(Iterable<String> networks) {
> + CloudSigmaTemplateOptions options = new CloudSigmaTemplateOptions();
> + return CloudSigmaTemplateOptions.class.cast(options.nodeNames(networks));
I think this should be `options.networks(networks)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8742283
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -332,7 +329,15 @@ public static EC2TemplateOptions nodeNames(Iterable<String> nodeNames) {
> EC2TemplateOptions options = new EC2TemplateOptions();
> return EC2TemplateOptions.class.cast(options.nodeNames(nodeNames));
> }
> -
> +
> + /**
> + * @see TemplateOptions#networks(Iterable)
> + */
> + public static EC2TemplateOptions networks(Iterable<String> networks) {
> + EC2TemplateOptions options = new EC2TemplateOptions();
> + return EC2TemplateOptions.class.cast(options.nodeNames(networks));
Also `options.networks` here? See Ignasi's comment above.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8742790
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #723](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/723/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31793662
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #963](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/963/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31797196
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
> @@ -468,4 +492,21 @@ public CloudStackTemplateOptions userMetadata(String key, String value) {
> public CloudStackTemplateOptions nodeNames(Iterable<String> nodeNames) {
> return CloudStackTemplateOptions.class.cast(super.nodeNames(nodeNames));
> }
> +
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public CloudStackTemplateOptions networks(Iterable<String> networks) {
> + return CloudStackTemplateOptions.class.cast(super.networks(networks));
> + }
> +
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711443
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #499](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/499/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31797143
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Phillips <no...@github.com>.
One question and a couple of minor points...thanks, @abayer!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246#issuecomment-31792632
Re: [jclouds] JCLOUDS-416. Add TemplateOptions#networks. (#246)
Posted by Andrew Bayer <no...@github.com>.
> @@ -35,8 +35,8 @@ public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptio
> if (templateOptions.getSecurityGroupIds().size() > 0) {
> options.securityGroupIds(templateOptions.getSecurityGroupIds());
> }
> - if (templateOptions.getNetworkIds().size() > 0) {
> - options.networkIds(templateOptions.getNetworkIds());
> + if (templateOptions.getNetworks().size() > 0) {
...dang I missed two!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/246/files#r8711907