Merge pull request #8762 from jameinel/2.3-into-develop

#8762

## Description of change

This includes my change to the Openstack provider to avoid duplicate subnet ranges by only including subnets that are part of the networks that you provide in config.
This also includes a no-op merge of Anastasia's change to status, since she had already independently merged a change to develop.

 prdesc Merge pull request #8751 from jameinel/2.3-openstack-subnets-1733266
 prdesc Merge pull request #8756 from anastasiamac/status-tabular-header-lines

## QA steps

See individual patches.

## Documentation changes

None.

## Bug reference

 prdesc https://bugs.launchpad.net/bugs/1761706
 prdesc https://bugs.launchpad.net/bugs/1733266

provider/openstack/local_test.go

@@ -1138,15 +1138,16 @@ func (s *localServerSuite) TestSupportsNetworking(c *gc.C) {
 	c.Assert(ok, jc.IsTrue)
 }
 
-func (s *localServerSuite) prepareNetworkingEnviron(c *gc.C) environs.NetworkingEnviron {
-	env := s.Open(c, s.env.Config())
+func (s *localServerSuite) prepareNetworkingEnviron(c *gc.C, cfg *config.Config) environs.NetworkingEnviron {
+	env := s.Open(c, cfg)
 	netenv, supported := environs.SupportsNetworking(env)
 	c.Assert(supported, jc.IsTrue)
 	return netenv
 }
 
 func (s *localServerSuite) TestSubnetsFindAll(c *gc.C) {
-	env := s.prepareNetworkingEnviron(c)
+	env := s.prepareNetworkingEnviron(c, s.env.Config())
+	// the environ is opened with network:"private_999" which maps to network id "999"
 	obtainedSubnets, err := env.Subnets(s.callCtx, instance.Id(""), []network.Id{})
 	c.Assert(err, jc.ErrorIsNil)
 	neutronClient := openstack.GetNeutronClient(s.env)
@@ -1160,6 +1161,46 @@ func (s *localServerSuite) TestSubnetsFindAll(c *gc.C) {
 
 	expectedSubnetMap := make(map[network.Id]network.SubnetInfo)
 	for _, os := range openstackSubnets {
+		if os.NetworkId != "999" {
+			continue
+		}
+		net, err := neutronClient.GetNetworkV2(os.NetworkId)
+		c.Assert(err, jc.ErrorIsNil)
+		expectedSubnetMap[network.Id(os.Id)] = network.SubnetInfo{
+			CIDR:              os.Cidr,
+			ProviderId:        network.Id(os.Id),
+			VLANTag:           0,
+			AvailabilityZones: net.AvailabilityZones,
+			SpaceProviderId:   "",
+		}
+	}
+
+	c.Check(obtainedSubnetMap, jc.DeepEquals, expectedSubnetMap)
+}
+
+func (s *localServerSuite) TestSubnetsFindAllWithExternal(c *gc.C) {
+	cfg := s.env.Config()
+	cfg, err := cfg.Apply(map[string]interface{}{"external-network": "ext-net"})
+	c.Assert(err, jc.ErrorIsNil)
+	env := s.prepareNetworkingEnviron(c, cfg)
+	// private_999 is the internal network, 998 is the external network
+	// the environ is opened with network:"private_999" which maps to network id "999"
+	obtainedSubnets, err := env.Subnets(s.callCtx, instance.Id(""), []network.Id{})
+	c.Assert(err, jc.ErrorIsNil)
+	neutronClient := openstack.GetNeutronClient(s.env)
+	openstackSubnets, err := neutronClient.ListSubnetsV2()
+	c.Assert(err, jc.ErrorIsNil)
+
+	obtainedSubnetMap := make(map[network.Id]network.SubnetInfo)
+	for _, sub := range obtainedSubnets {
+		obtainedSubnetMap[sub.ProviderId] = sub
+	}
+
+	expectedSubnetMap := make(map[network.Id]network.SubnetInfo)
+	for _, os := range openstackSubnets {
+		if os.NetworkId != "999" && os.NetworkId != "998" {
+			continue
+		}
 		net, err := neutronClient.GetNetworkV2(os.NetworkId)
 		c.Assert(err, jc.ErrorIsNil)
 		expectedSubnetMap[network.Id(os.Id)] = network.SubnetInfo{
@@ -1175,23 +1216,26 @@ func (s *localServerSuite) TestSubnetsFindAll(c *gc.C) {
 }
 
 func (s *localServerSuite) TestSubnetsWithMissingSubnet(c *gc.C) {
-	env := s.prepareNetworkingEnviron(c)
+	env := s.prepareNetworkingEnviron(c, s.env.Config())
 	subnets, err := env.Subnets(s.callCtx, instance.Id(""), []network.Id{"missing"})
 	c.Assert(err, gc.ErrorMatches, `failed to find the following subnet ids: \[missing\]`)
 	c.Assert(subnets, gc.HasLen, 0)
 }
 
 func (s *localServerSuite) TestSuperSubnets(c *gc.C) {
-	env := s.prepareNetworkingEnviron(c)
+	env := s.prepareNetworkingEnviron(c, s.env.Config())
 	obtainedSubnets, err := env.SuperSubnets(s.callCtx)
 	c.Assert(err, jc.ErrorIsNil)
 	neutronClient := openstack.GetNeutronClient(s.env)
 	openstackSubnets, err := neutronClient.ListSubnetsV2()
 	c.Assert(err, jc.ErrorIsNil)
 
-	expectedSubnets := make([]string, len(openstackSubnets))
-	for i, os := range openstackSubnets {
-		expectedSubnets[i] = os.Cidr
+	expectedSubnets := make([]string, 0, len(openstackSubnets))
+	for _, os := range openstackSubnets {
+		if os.NetworkId != "999" {
+			continue
+		}
+		expectedSubnets = append(expectedSubnets, os.Cidr)
 	}
 	sort.Strings(obtainedSubnets)
 	sort.Strings(expectedSubnets)

provider/openstack/networking.go

@@ -331,19 +331,52 @@ func (n *NeutronNetworking) Subnets(instId instance.Id, subnetIds []network.Id)
 	for _, subId := range subnetIds {
 		subIdSet.Add(string(subId))
 	}
+	netIds := set.NewStrings()
+	displayIds := ""
+	neutron := n.env.neutron()
+	network := n.env.ecfg().network()
+	netId, err := resolveNeutronNetwork(neutron, network, false)
+	if err != nil {
+		logger.Warningf("could not resolve internal network id for %q: %v", network, err)
+		// Note: (jam 2018-05-23) We don't treat this as fatal because we used to never pay attention to it anyway
+	} else {
+		netIds.Add(netId)
+		displayIds = fmt.Sprintf("[%q", netId)
+		// Note, there are cases where we will detect an external
+		// network without it being explicitly configured by the user.
+		// When we get to a point where we start detecting spaces for
+		// users on Openstack, we'll probably need to include better logic here.
+		externalNetwork := n.env.ecfg().externalNetwork()
+		if externalNetwork != "" {
+			netId, err := resolveNeutronNetwork(neutron, externalNetwork, true)
+			if err != nil {
+				logger.Warningf("could not resolve external network id for %q: %v", externalNetwork, err)
+			} else {
+				netIds.Add(netId)
+				displayIds = fmt.Sprintf("%s, %q", displayIds, netId)
+			}
+		}
+		displayIds = displayIds + "]"
+	}
+	logger.Debugf("finding subnets in networks: %s", displayIds)
 
 	if instId != instance.UnknownId {
 		// TODO(hml): 2017-03-20
 		// Implement Subnets() for case where instId is specified
 		return nil, errors.NotSupportedf("neutron subnets with instance Id")
 	} else {
-		neutron := n.env.neutron()
+		// TODO(jam): 2018-05-23 It is likely that ListSubnetsV2 could
+		// take a Filter rather that doing the filtering client side.
 		subnets, err := neutron.ListSubnetsV2()
 		if err != nil {
 			return nil, errors.Annotatef(err, "failed to retrieve subnets")
 		}
 		if len(subnetIds) == 0 {
 			for _, subnet := range subnets {
+				if !netIds.IsEmpty() && !netIds.Contains(subnet.NetworkId) {
+					logger.Tracef("ignoring subnet %q, part of network %q not %v", subnet.Id, subnet.NetworkId, displayIds)
+					continue
+				}
 				subIdSet.Add(subnet.Id)
 			}
 		}

state/containernetworking.go

@@ -70,6 +70,10 @@ func (m *Model) discoverFan(environ environs.Environ, modelConfig *config.Config
 		if err != nil {
 			return ""
 		}
+		// We don't create FAN networks for IPv6 networks
+		if ipNet.IP.To4() == nil {
+			return ""
+		}
 		if ones, _ := ipNet.Mask.Size(); ones <= 8 {
 			return ""
 		}

state/containernetworking_test.go

@@ -126,3 +126,18 @@ func (s *ContainerNetworkingSuite) TestAutoConfigureContainerNetworkingDefault(c
 	c.Check(attrs["container-networking-method"], gc.Equals, "fan")
 	c.Check(attrs["fan-config"], gc.Equals, "172.31.0.0/16=252.0.0.0/8 192.168.1.0/24=253.0.0.0/8")
 }
+
+func (s *ContainerNetworkingSuite) TestAutoConfigureContainerNetworkingIgnoresIPv6(c *gc.C) {
+	environ := containerTestNetworkedEnviron{
+		stub: &testing.Stub{},
+		supportsContainerAddresses: true,
+		superSubnets:               []string{"172.31.0.0/16", "2000::dead:beef:1/64"},
+	}
+	err := s.Model.AutoConfigureContainerNetworking(&environ)
+	c.Check(err, jc.ErrorIsNil)
+	config, err := s.Model.ModelConfig()
+	c.Assert(err, jc.ErrorIsNil)
+	attrs := config.AllAttrs()
+	c.Check(attrs["container-networking-method"], gc.Equals, "provider")
+	c.Check(attrs["fan-config"], gc.Equals, "172.31.0.0/16=252.0.0.0/8")
+}