Merge pull request juju#13422 from manadart/2.9-openstack-binding-provisioning

jujubot · web-flow · commit 7b53d417eb53 · 2021-10-18T02:23:19.000+10:00
juju#13422 When determining OpenStack networking for instances, we only process the `SubnetsToZones` topology if we have space constraints. This is not correct, because the presence of a non-empty `SubnetsToZones` map already implies that provisioning info created it based on constraints and/or endpoint bindings. The result is that specifying multiple bindings without constraints results in an incorrectly provisioned machine. Here we simple remove the gate on the presence of space constraints, meaning that endpoint bindings (implied by a populated AZ/space topology) will be honoured. ## QA steps On an OpenStack with multiple subnets: - Bootstrap. - Carve out 2 spaces, each with a different subnet. - juju deploy --bind 'space-1 public=space-2' cs:nagios`. - Check that the provisioned machine has 2 network devices. ## Documentation changes None. ## Bug reference https://bugs.launchpad.net/juju/+bug/1943503
diff --git a/apiserver/facades/agent/provisioner/provisioninginfo.go b/apiserver/facades/agent/provisioner/provisioninginfo.go
@@ -409,8 +409,9 @@ func (api *ProvisionerAPI) machineSpaces(m *state.Machine,
 func (api *ProvisionerAPI) machineSpaceTopology(machineID string, spaceNames []string) (params.ProvisioningNetworkTopology, error) {
 	var topology params.ProvisioningNetworkTopology
 
-	// If there are no space names, or if there is only one space name and
-	// that's the alpha space.
+	// If there are no space names, or if there is only one space
+	// name and that's the alpha space, we don't bother setting a
+	// topology that constrains provisioning.
 	if len(spaceNames) < 1 || (len(spaceNames) == 1 && spaceNames[0] == network.AlphaSpaceName) {
 		return topology, nil
 	}
diff --git a/provider/openstack/provider.go b/provider/openstack/provider.go
@@ -1418,11 +1418,12 @@ func (e *Environ) networksForInstance(
 		return nil, errors.Trace(err)
 	}
 
-	if !args.Constraints.HasSpaces() {
+	if len(args.SubnetsToZones) == 0 {
 		return networks, nil
 	}
 	if len(networks) == 0 {
-		return nil, errors.New("space constraints were supplied, but no Openstack network is configured")
+		return nil, errors.New(
+			"space constraints and/or bindings were supplied, but no Openstack network is configured")
 	}
 
 	// We know that we are operating in the single configured network.
@@ -1452,7 +1453,7 @@ func (e *Environ) networksForInstance(
 		subnetIDsForZone[i] = network.FilterInFanNetwork(subnetIDs)
 	}
 
-	/// For each list of subnet IDs that satisfy space and zone constraints,
+	// For each list of subnet IDs that satisfy space and zone constraints,
 	// choose a single one at random.
 	subnetIDForZone := make([]network.Id, len(subnetIDsForZone))
 	for i, subnetIDs := range subnetIDsForZone {
diff --git a/provider/openstack/provider_test.go b/provider/openstack/provider_test.go
@@ -530,7 +530,7 @@ var handlerFunc = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request)
 	// This line is critical, the openstack provider will reject the message
 	// if you return 200 like a mere mortal.
 	w.WriteHeader(http.StatusMultipleChoices)
-	fmt.Fprint(w, `
+	_, _ = fmt.Fprint(w, `
 {
   "versions": {
     "values": [
@@ -920,11 +920,6 @@ func (s *providerUnitTests) TestNetworksForInstanceWithAZ(c *gc.C) {
 	siParams := environs.StartInstanceParams{
 		AvailabilityZone: "eu-west-az",
 		SubnetsToZones:   []map[network.Id][]string{{"subnet-foo": {"eu-west-az", "eu-east-az"}}},
-		Constraints: constraints.Value{
-			Spaces: &[]string{
-				"eu-west-az",
-			},
-		},
 	}
 
 	result, err := envWithNetworking(mockNetworking).networksForInstance(siParams, netCfg)

Original file line number	Diff line number	Diff line change
`@@ -1418,11 +1418,12 @@ func (e *Environ) networksForInstance(`
`1418`	`1418`	`return nil, errors.Trace(err)`
`1419`	`1419`	`}`
`1420`	`1420`
`1421`		`- if !args.Constraints.HasSpaces() {`
	`1421`	`+ if len(args.SubnetsToZones) == 0 {`
`1422`	`1422`	`return networks, nil`
`1423`	`1423`	`}`
`1424`	`1424`	`if len(networks) == 0 {`
`1425`		`- return nil, errors.New("space constraints were supplied, but no Openstack network is configured")`
	`1425`	`+ return nil, errors.New(`
	`1426`	`+ "space constraints and/or bindings were supplied, but no Openstack network is configured")`
`1426`	`1427`	`}`
`1427`	`1428`
`1428`	`1429`	`// We know that we are operating in the single configured network.`
`@@ -1452,7 +1453,7 @@ func (e *Environ) networksForInstance(`
`1452`	`1453`	`subnetIDsForZone[i] = network.FilterInFanNetwork(subnetIDs)`
`1453`	`1454`	`}`
`1454`	`1455`
`1455`		`- /// For each list of subnet IDs that satisfy space and zone constraints,`
	`1456`	`+ // For each list of subnet IDs that satisfy space and zone constraints,`
`1456`	`1457`	`// choose a single one at random.`
`1457`	`1458`	`subnetIDForZone := make([]network.Id, len(subnetIDsForZone))`
`1458`	`1459`	`for i, subnetIDs := range subnetIDsForZone {`