Merge pull request #14019 from wallyworld/inst-arch-filtering

#14019

When bootstrapping, if no arch constraint is specified, the arch of the host machine is used to find the agent binaries. However, instance selection is not filtered by arch, meaning we could get an amd64 instance with arm64 binaries.

As part of the fix, we also refactor various instance structs to use a single arch rather than slice of arches; the only reason we had more than one was because ec2 supported i386 and amd64, but that's no longer relevant as i386 was dropped a while back. And we can remove some now unnecessary checks when finalising instance config.

## QA steps

```
GOARCH=arm64 juju bootstrap aws --agent-version=2.9.29
juju add-machine
juju add-machine --constraints "arch=amd64"
juju add-machine --constraints "arch=arm64"
```

## Bug reference

https://bugs.launchpad.net/bugs/1972103

apiserver/common/instancetypes.go

@@ -22,14 +22,16 @@ func toParamsInstanceTypeResult(itypes []instances.InstanceType) []params.Instan
 		}
 		result[i] = params.InstanceType{
 			Name:         t.Name,
-			Arches:       t.Arches,
 			CPUCores:     int(t.CpuCores),
 			Memory:       int(t.Mem),
 			RootDiskSize: int(t.RootDisk),
 			VirtType:     virtType,
 			Deprecated:   t.Deprecated,
 			Cost:         int(t.Cost),
 		}
+		if t.Arch != "" {
+			result[i].Arches = []string{t.Arch}
+		}
 	}
 	return result
 }

container/broker/broker.go

@@ -159,9 +159,10 @@ func matchHostArchTools(allTools coretools.List) (coretools.List, error) {
 	arch := arch.HostArch()
 	archTools, err := allTools.Match(coretools.Filter{Arch: arch})
 	if err == coretools.ErrNoMatches {
+		agentArch, _ := allTools.OneArch()
 		return nil, errors.Errorf(
 			"need agent binaries for arch %s, only found %s",
-			arch, allTools.Arches(),
+			arch, agentArch,
 		)
 	} else if err != nil {
 		return nil, errors.Trace(err)

container/broker/lxd-broker_test.go

@@ -103,7 +103,9 @@ func (s *lxdBrokerSuite) TestStartInstanceWithoutHostNetworkChanges(c *gc.C) {
 	c.Assert(call.Args[0], gc.FitsTypeOf, &instancecfg.InstanceConfig{})
 	instanceConfig := call.Args[0].(*instancecfg.InstanceConfig)
 	c.Assert(instanceConfig.ToolsList(), gc.HasLen, 1)
-	c.Assert(instanceConfig.ToolsList().Arches(), jc.DeepEquals, []string{"amd64"})
+	arch, err := instanceConfig.ToolsList().OneArch()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(arch, gc.Equals, "amd64")
 }
 
 func (s *lxdBrokerSuite) TestStartInstancePopulatesFallbackNetworkInfo(c *gc.C) {
@@ -133,7 +135,7 @@ func (s *lxdBrokerSuite) TestStartInstanceNoHostArchTools(c *gc.C) {
 		}},
 		InstanceConfig: makeInstanceConfig(c, s, "1/lxd/0"),
 	})
-	c.Assert(err, gc.ErrorMatches, `need agent binaries for arch amd64, only found \[arm64\]`)
+	c.Assert(err, gc.ErrorMatches, `need agent binaries for arch amd64, only found arm64`)
 }
 
 func (s *lxdBrokerSuite) TestStartInstanceWithCloudInitUserData(c *gc.C) {

environs/bootstrap/bootstrap.go

@@ -447,7 +447,7 @@ func bootstrapIAAS(
 	} else {
 		// If no arch is specified as a constraint and we couldn't
 		// auto-discover the arch from the provider, we'll fall back
-		// to bootstrapping on on the same arch as the CLI client.
+		// to bootstrapping on the same arch as the CLI client.
 		bootstrapArch = localToolsArch()
 
 		// We no longer support controllers on i386. If we are

environs/bootstrap/bootstrap_test.go

@@ -1646,8 +1646,9 @@ func (e *bootstrapEnviron) Bootstrap(ctx environs.BootstrapContext, callCtx envc
 	if args.BootstrapSeries != "" {
 		series = args.BootstrapSeries
 	}
+	arch, _ := args.AvailableTools.OneArch()
 	return &environs.BootstrapResult{
-		Arch:                    args.AvailableTools.Arches()[0],
+		Arch:                    arch,
 		Series:                  series,
 		CloudBootstrapFinalizer: finalizer,
 	}, nil

environs/instances/image.go

@@ -10,6 +10,7 @@ import (
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"github.com/juju/utils/v3/arch"
+	"github.com/kr/pretty"
 
 	"github.com/juju/juju/core/constraints"
 	"github.com/juju/juju/environs/imagemetadata"
@@ -22,7 +23,7 @@ var logger = loggo.GetLogger("juju.environs.instances")
 type InstanceConstraint struct {
 	Region      string
 	Series      string
-	Arches      []string
+	Arch        string
 	Constraints constraints.Value
 
 	// Optional filtering criteria not supported by all providers. These
@@ -39,10 +40,10 @@ type InstanceConstraint struct {
 // String returns a human readable form of this InstanceConstraint.
 func (ic *InstanceConstraint) String() string {
 	return fmt.Sprintf(
-		"{region: %s, series: %s, arches: %s, constraints: %s, storage: %s}",
+		"{region: %s, series: %s, arch: %s, constraints: %s, storage: %s}",
 		ic.Region,
 		ic.Series,
-		ic.Arches,
+		ic.Arch,
 		ic.Constraints,
 		ic.Storage,
 	)
@@ -64,12 +65,18 @@ type InstanceSpec struct {
 func FindInstanceSpec(possibleImages []Image, ic *InstanceConstraint, allInstanceTypes []InstanceType) (*InstanceSpec, error) {
 	logger.Debugf("instance constraints %+v", ic)
 	if len(possibleImages) == 0 {
-		return nil, errors.Errorf("no metadata for %q images in %s with arches %s",
-			ic.Series, ic.Region, ic.Arches)
+		return nil, errors.Errorf("no metadata for %q images in %s with arch %s",
+			ic.Series, ic.Region, ic.Arch)
 	}
 
-	logger.Debugf("matching constraints %v against possible image metadata %+v", ic, possibleImages)
-	matchingTypes, err := MatchingInstanceTypes(allInstanceTypes, ic.Region, ic.Constraints)
+	logger.Debugf("matching constraints %v against possible image metadata %s", ic, pretty.Sprint(possibleImages))
+	// If no constraints arch is specified, we need to ensure instances are filtered
+	// on the arch of the agent binary.
+	cons := ic.Constraints
+	if !cons.HasArch() && ic.Arch != "" {
+		cons.Arch = &ic.Arch
+	}
+	matchingTypes, err := MatchingInstanceTypes(allInstanceTypes, ic.Region, cons)
 	if err != nil {
 		return nil, err
 	}
@@ -169,7 +176,7 @@ const (
 
 // match returns true if the image can run on the supplied instance type.
 func (image Image) match(itype InstanceType) imageMatch {
-	if !image.matchArch(itype.Arches) {
+	if itype.Arch != image.Arch {
 		return nonMatch
 	}
 	if itype.VirtType == nil || image.VirtType == *itype.VirtType {
@@ -183,15 +190,6 @@ func (image Image) match(itype InstanceType) imageMatch {
 	return nonMatch
 }
 
-func (image Image) matchArch(arches []string) bool {
-	for _, arch := range arches {
-		if arch == image.Arch {
-			return true
-		}
-	}
-	return false
-}
-
 // ImageMetadataToImages converts an array of ImageMetadata pointers (as
 // returned by imagemetadata.Fetch) to an array of Image objects (as required
 // by instances.FindInstanceSpec).