Ensures via tests that API addresses for CAAS return public addresses

manadart · manadart · commit a838e86efba0 · 2020-04-03T14:39:18.000+02:00
for agents with no duplication.
diff --git a/state/address.go b/state/address.go
@@ -278,19 +278,13 @@ func (st *State) apiHostPortsForCAAS(public bool) (addresses []network.SpaceHost
 		logger.Debugf("getting api hostports for CAAS: public %t, addresses %v", public, addresses)
 	}()
 
-	ctrlSt, err := st.newStateNoWorkers(st.ControllerModelUUID())
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	defer func() { _ = ctrlSt.Close() }()
-
-	controllerConfig, err := ctrlSt.ControllerConfig()
+	controllerConfig, err := st.ControllerConfig()
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 
 	apiPort := controllerConfig.APIPort()
-	svc, err := ctrlSt.CloudService(controllerConfig.ControllerUUID())
+	svc, err := st.CloudService(controllerConfig.ControllerUUID())
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -300,15 +294,31 @@ func (st *State) apiHostPortsForCAAS(public bool) (addresses []network.SpaceHost
 		return []network.SpaceHostPorts{network.SpaceAddressesWithPort(addrs, apiPort)}
 	}
 
-	// select public address.
-	publicAddr, _ := addrs.OneMatchingScope(network.ScopeMatchPublic)
+	// Return all publicly available addresses for clients.
+	// Scope matching falls back through a hierarchy,
+	// so these may actually be local-cloud addresses.
+	publicAddrs := addrs.AllMatchingScope(network.ScopeMatchPublic)
 	if public {
-		return addrsToHostPorts(publicAddr), nil
+		return addrsToHostPorts(publicAddrs...), nil
+	}
+
+	// TODO(wallyworld) - for now, return all addresses for agents to try, public last.
+
+	// If we are after local-cloud addresses and those were all that public
+	// matching turned up, just return those.
+	if len(publicAddrs) > 0 && publicAddrs[0].Scope == network.ScopeCloudLocal {
+		return addrsToHostPorts(publicAddrs...), nil
+	}
+
+	localAddrs := addrs.AllMatchingScope(network.ScopeMatchCloudLocal)
+
+	// If there were no local-cloud addresses, return the public ones.
+	if len(localAddrs) == 0 || localAddrs[0].Scope == network.ScopePublic {
+		return addrsToHostPorts(publicAddrs...), nil
 	}
 
-	// TODO(wallyworld) - for now, return all addresses for agents to try, public last
-	result := addrsToHostPorts(addrs.AllMatchingScope(network.ScopeMatchCloudLocal)...)
-	return append(result, addrsToHostPorts(publicAddr)...), nil
+	// Otherwise return everything, local-cloud first.
+	return addrsToHostPorts(append(localAddrs, publicAddrs...)...), nil
 }
 
 // apiHostPortsForKey returns API addresses extracted from the document
diff --git a/state/address_test.go b/state/address_test.go
@@ -368,3 +368,139 @@ func (s *ControllerAddressesSuite) TestWatchAPIHostPortsForAgents(c *gc.C) {
 	statetesting.AssertStop(c, w)
 	wc.AssertClosed()
 }
+
+type CAASAddressesSuite struct {
+	statetesting.StateSuite
+}
+
+var _ = gc.Suite(&CAASAddressesSuite{})
+
+func (s *CAASAddressesSuite) SetUpTest(c *gc.C) {
+	s.StateSuite.SetUpTest(c)
+	state.SetModelTypeToCAAS(c, s.State, s.Model)
+}
+
+func (s *CAASAddressesSuite) TestAPIHostPortsCloudLocalOnly(c *gc.C) {
+	machineAddr := network.MachineAddress{
+		Value: "10.10.10.10",
+		Type:  network.IPv4Address,
+		Scope: network.ScopeCloudLocal,
+	}
+
+	_, err := s.State.SaveCloudService(state.SaveCloudServiceArgs{
+		Id:         s.Model.ControllerUUID(),
+		ProviderId: "whatever",
+		Addresses:  network.SpaceAddresses{{MachineAddress: machineAddr}},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	exp := []network.SpaceHostPorts{{{
+		SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr},
+		NetPort:      17777,
+	}}}
+
+	addrs, err := s.State.APIHostPortsForAgents()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(addrs, gc.DeepEquals, exp)
+
+	addrs, err = s.State.APIHostPortsForClients()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(addrs, gc.DeepEquals, exp)
+}
+
+func (s *CAASAddressesSuite) TestAPIHostPortsPublicOnly(c *gc.C) {
+	machineAddr := network.MachineAddress{
+		Value: "10.10.10.10",
+		Type:  network.IPv4Address,
+		Scope: network.ScopePublic,
+	}
+
+	_, err := s.State.SaveCloudService(state.SaveCloudServiceArgs{
+		Id:         s.Model.ControllerUUID(),
+		ProviderId: "whatever",
+		Addresses:  network.SpaceAddresses{{MachineAddress: machineAddr}},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	exp := []network.SpaceHostPorts{{{
+		SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr},
+		NetPort:      17777,
+	}}}
+
+	addrs, err := s.State.APIHostPortsForAgents()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(addrs, gc.DeepEquals, exp)
+
+	addrs, err = s.State.APIHostPortsForClients()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(addrs, gc.DeepEquals, exp)
+}
+
+func (s *CAASAddressesSuite) TestAPIHostPortsMultiple(c *gc.C) {
+	machineAddr1 := network.MachineAddress{
+		Value: "10.10.10.1",
+		Type:  network.IPv4Address,
+		Scope: network.ScopePublic,
+	}
+	machineAddr2 := network.MachineAddress{
+		Value: "10.10.10.2",
+		Type:  network.IPv4Address,
+		Scope: network.ScopePublic,
+	}
+	machineAddr3 := network.MachineAddress{
+		Value: "100.10.10.1",
+		Type:  network.IPv4Address,
+		Scope: network.ScopeCloudLocal,
+	}
+	machineAddr4 := network.MachineAddress{
+		Value: "100.10.10.2",
+		Type:  network.IPv4Address,
+		Scope: network.ScopeCloudLocal,
+	}
+
+	_, err := s.State.SaveCloudService(state.SaveCloudServiceArgs{
+		Id:         s.Model.ControllerUUID(),
+		ProviderId: "whatever",
+		Addresses: network.SpaceAddresses{
+			{MachineAddress: machineAddr1},
+			{MachineAddress: machineAddr2},
+			{MachineAddress: machineAddr3},
+			{MachineAddress: machineAddr4},
+		},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	addrs, err := s.State.APIHostPortsForAgents()
+	c.Assert(err, jc.ErrorIsNil)
+
+	// Local-cloud addresses must come first.
+	c.Assert(addrs[0][:2], jc.SameContents, network.SpaceHostPorts{
+		{
+			SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr3},
+			NetPort:      17777,
+		},
+		{
+			SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr4},
+			NetPort:      17777,
+		},
+	})
+
+	exp := network.SpaceHostPorts{
+		{
+			SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr1},
+			NetPort:      17777,
+		},
+		{
+			SpaceAddress: network.SpaceAddress{MachineAddress: machineAddr2},
+			NetPort:      17777,
+		},
+	}
+
+	// Public ones should also follow.
+	c.Assert(addrs[0][2:], jc.SameContents, exp)
+
+	// Only the public ones should be returned.
+	addrs, err = s.State.APIHostPortsForClients()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(addrs, gc.DeepEquals, []network.SpaceHostPorts{exp})
+}
