provider/gce: Handle legacy networks #7150

Merged
merged 2 commits into from Mar 24, 2017

Conversation

Projects
None yet
3 participants
Member

babbageclunk commented Mar 24, 2017

Description of change

When an interface is on a legacy network, interface.Subnetwork is "" and we
get an error looking up the subnet for that URL. In that case use the network
to get the CIDR and ProviderNetworkId, and leave ProviderSubnetId blank.

QA steps

Bootstrap in a GCE legacy network. Add one LXD machine, and verify that it
starts and connects to the controller, and then deploy a charm to it.

Bug reference

Fixes https://bugs.launchpad.net/juju/+bug/1675546

provider/gce: Handle legacy networks
When an interface is on a legacy network, .Subnetwork is "". In that
case use the network to get the CIDR and ProviderNetworkId, and leave
ProviderSubnetId blank.

Fixes https://bugs.launchpad.net/juju/+bug/1675546
Member

babbageclunk commented Mar 24, 2017

!!build!!

Did you also QA that it still works with non-legacy networks?

if err != nil {
return nil, errors.Trace(err)
}
var results []network.SubnetInfo
for _, subnet := range allSubnets {
- networkId, ok := networks[subnet.Network]
+ netwk, ok := networks[subnet.Network]
@wallyworld

wallyworld Mar 24, 2017

Owner

we use "networks" elsewhere
can we use "network" here ?

@babbageclunk

babbageclunk Mar 24, 2017

Member

network clashes with github.com/juju/juju/network which I use just below - I could rename the import instead?

provider/gce/environ_network.go
+ network network.Id
+}
+
+func findNetworkDetails(iface compute.NetworkInterface, subnets subnetMap, networks networkMap) (networkDetails, error) {
@wallyworld

wallyworld Mar 24, 2017

Owner

worthy of a comment i think

urlSet := includeSet{items: set.NewStrings(urls...)}
allSubnets, err := e.gce.Subnetworks(e.cloud.Region)
if err != nil {
return nil, errors.Trace(err)
}
results := make(map[string]network.SubnetInfo)
for _, subnet := range allSubnets {
- networkId, ok := networks[subnet.Network]
+ netwk, ok := networks[subnet.Network]
@@ -231,14 +265,21 @@ func (e *environ) ReleaseContainerAddresses([]network.ProviderInterfaceInfo) err
return errors.NotSupportedf("container addresses")
}
+func copyStrings(items []string) []string {
@wallyworld

wallyworld Mar 24, 2017

Owner

We have copyStringSlice in cloudinit package.
It annoys me this stuff has to be copy and pasted all the time.
Just a random whine.

@babbageclunk

babbageclunk Mar 24, 2017

Member

Hmm, maybe I should amalgamate them somewhere more central?

Member

babbageclunk commented Mar 24, 2017

$$merge$$

Contributor

jujubot commented Mar 24, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit f111f64 into juju:develop Mar 24, 2017

@babbageclunk babbageclunk deleted the babbageclunk:gce-fix-bug branch Mar 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment