Fix inst filtering to avoid arch mismatches

As part of the fix, 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
juju · May 12, 2022 · d6c695c · d6c695c
1 parent 36865de
commit d6c695c
Show file tree

Hide file tree

Showing 41 changed files with 342 additions and 511 deletions.
diff --git a/apiserver/common/instancetypes.go b/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
 }

diff --git a/container/broker/broker.go b/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)

diff --git a/container/broker/lxd-broker_test.go b/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) {

diff --git a/environs/bootstrap/bootstrap.go b/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

diff --git a/environs/bootstrap/bootstrap_test.go b/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

diff --git a/environs/instances/image.go b/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).