Skip to content

Commit d6c695c

Browse files
wallyworldwallyworld
committed
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
1 parent 36865de commit d6c695c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+342
-511
lines changed

‎apiserver/common/instancetypes.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,16 @@ func toParamsInstanceTypeResult(itypes []instances.InstanceType) []params.Instan
2222
}
2323
result[i] = params.InstanceType{
2424
Name: t.Name,
25-
Arches: t.Arches,
2625
CPUCores: int(t.CpuCores),
2726
Memory: int(t.Mem),
2827
RootDiskSize: int(t.RootDisk),
2928
VirtType: virtType,
3029
Deprecated: t.Deprecated,
3130
Cost: int(t.Cost),
3231
}
32+
if t.Arch != "" {
33+
result[i].Arches = []string{t.Arch}
34+
}
3335
}
3436
return result
3537
}

‎container/broker/broker.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,10 @@ func matchHostArchTools(allTools coretools.List) (coretools.List, error) {
159159
arch := arch.HostArch()
160160
archTools, err := allTools.Match(coretools.Filter{Arch: arch})
161161
if err == coretools.ErrNoMatches {
162+
agentArch, _ := allTools.OneArch()
162163
return nil, errors.Errorf(
163164
"need agent binaries for arch %s, only found %s",
164-
arch, allTools.Arches(),
165+
arch, agentArch,
165166
)
166167
} else if err != nil {
167168
return nil, errors.Trace(err)

‎container/broker/lxd-broker_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ func (s *lxdBrokerSuite) TestStartInstanceWithoutHostNetworkChanges(c *gc.C) {
103103
c.Assert(call.Args[0], gc.FitsTypeOf, &instancecfg.InstanceConfig{})
104104
instanceConfig := call.Args[0].(*instancecfg.InstanceConfig)
105105
c.Assert(instanceConfig.ToolsList(), gc.HasLen, 1)
106-
c.Assert(instanceConfig.ToolsList().Arches(), jc.DeepEquals, []string{"amd64"})
106+
arch, err := instanceConfig.ToolsList().OneArch()
107+
c.Assert(err, jc.ErrorIsNil)
108+
c.Assert(arch, gc.Equals, "amd64")
107109
}
108110

109111
func (s *lxdBrokerSuite) TestStartInstancePopulatesFallbackNetworkInfo(c *gc.C) {
@@ -133,7 +135,7 @@ func (s *lxdBrokerSuite) TestStartInstanceNoHostArchTools(c *gc.C) {
133135
}},
134136
InstanceConfig: makeInstanceConfig(c, s, "1/lxd/0"),
135137
})
136-
c.Assert(err, gc.ErrorMatches, `need agent binaries for arch amd64, only found \[arm64\]`)
138+
c.Assert(err, gc.ErrorMatches, `need agent binaries for arch amd64, only found arm64`)
137139
}
138140

139141
func (s *lxdBrokerSuite) TestStartInstanceWithCloudInitUserData(c *gc.C) {

‎environs/bootstrap/bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ func bootstrapIAAS(
447447
} else {
448448
// If no arch is specified as a constraint and we couldn't
449449
// auto-discover the arch from the provider, we'll fall back
450-
// to bootstrapping on on the same arch as the CLI client.
450+
// to bootstrapping on the same arch as the CLI client.
451451
bootstrapArch = localToolsArch()
452452

453453
// We no longer support controllers on i386. If we are

‎environs/bootstrap/bootstrap_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1646,8 +1646,9 @@ func (e *bootstrapEnviron) Bootstrap(ctx environs.BootstrapContext, callCtx envc
16461646
if args.BootstrapSeries != "" {
16471647
series = args.BootstrapSeries
16481648
}
1649+
arch, _ := args.AvailableTools.OneArch()
16491650
return &environs.BootstrapResult{
1650-
Arch: args.AvailableTools.Arches()[0],
1651+
Arch: arch,
16511652
Series: series,
16521653
CloudBootstrapFinalizer: finalizer,
16531654
}, nil

‎environs/instances/image.go

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/juju/errors"
1111
"github.com/juju/loggo"
1212
"github.com/juju/utils/v3/arch"
13+
"github.com/kr/pretty"
1314

1415
"github.com/juju/juju/core/constraints"
1516
"github.com/juju/juju/environs/imagemetadata"
@@ -22,7 +23,7 @@ var logger = loggo.GetLogger("juju.environs.instances")
2223
type InstanceConstraint struct {
2324
Region string
2425
Series string
25-
Arches []string
26+
Arch string
2627
Constraints constraints.Value
2728

2829
// Optional filtering criteria not supported by all providers. These
@@ -39,10 +40,10 @@ type InstanceConstraint struct {
3940
// String returns a human readable form of this InstanceConstraint.
4041
func (ic *InstanceConstraint) String() string {
4142
return fmt.Sprintf(
42-
"{region: %s, series: %s, arches: %s, constraints: %s, storage: %s}",
43+
"{region: %s, series: %s, arch: %s, constraints: %s, storage: %s}",
4344
ic.Region,
4445
ic.Series,
45-
ic.Arches,
46+
ic.Arch,
4647
ic.Constraints,
4748
ic.Storage,
4849
)
@@ -64,12 +65,18 @@ type InstanceSpec struct {
6465
func FindInstanceSpec(possibleImages []Image, ic *InstanceConstraint, allInstanceTypes []InstanceType) (*InstanceSpec, error) {
6566
logger.Debugf("instance constraints %+v", ic)
6667
if len(possibleImages) == 0 {
67-
return nil, errors.Errorf("no metadata for %q images in %s with arches %s",
68-
ic.Series, ic.Region, ic.Arches)
68+
return nil, errors.Errorf("no metadata for %q images in %s with arch %s",
69+
ic.Series, ic.Region, ic.Arch)
6970
}
7071

71-
logger.Debugf("matching constraints %v against possible image metadata %+v", ic, possibleImages)
72-
matchingTypes, err := MatchingInstanceTypes(allInstanceTypes, ic.Region, ic.Constraints)
72+
logger.Debugf("matching constraints %v against possible image metadata %s", ic, pretty.Sprint(possibleImages))
73+
// If no constraints arch is specified, we need to ensure instances are filtered
74+
// on the arch of the agent binary.
75+
cons := ic.Constraints
76+
if !cons.HasArch() && ic.Arch != "" {
77+
cons.Arch = &ic.Arch
78+
}
79+
matchingTypes, err := MatchingInstanceTypes(allInstanceTypes, ic.Region, cons)
7380
if err != nil {
7481
return nil, err
7582
}
@@ -169,7 +176,7 @@ const (
169176

170177
// match returns true if the image can run on the supplied instance type.
171178
func (image Image) match(itype InstanceType) imageMatch {
172-
if !image.matchArch(itype.Arches) {
179+
if itype.Arch != image.Arch {
173180
return nonMatch
174181
}
175182
if itype.VirtType == nil || image.VirtType == *itype.VirtType {
@@ -183,15 +190,6 @@ func (image Image) match(itype InstanceType) imageMatch {
183190
return nonMatch
184191
}
185192

186-
func (image Image) matchArch(arches []string) bool {
187-
for _, arch := range arches {
188-
if arch == image.Arch {
189-
return true
190-
}
191-
}
192-
return false
193-
}
194-
195193
// ImageMetadataToImages converts an array of ImageMetadata pointers (as
196194
// returned by imagemetadata.Fetch) to an array of Image objects (as required
197195
// by instances.FindInstanceSpec).

0 commit comments

Comments
 (0)