Skip to content

Commit 112e445

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 112e445

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

+339
-510
lines changed

‎apiserver/common/instancetypes.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func toParamsInstanceTypeResult(itypes []instances.InstanceType) []params.Instan
2222
}
2323
result[i] = params.InstanceType{
2424
Name: t.Name,
25-
Arches: t.Arches,
25+
Arches: []string{t.Arch},
2626
CPUCores: int(t.CpuCores),
2727
Memory: int(t.Mem),
2828
RootDiskSize: int(t.RootDisk),

‎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: 3 additions & 1 deletion
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, jc.DeepEquals, []string{"amd64"})
107109
}
108110

109111
func (s *lxdBrokerSuite) TestStartInstancePopulatesFallbackNetworkInfo(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).

‎environs/instances/image_test.go

Lines changed: 41 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,10 @@ var jsonImagesContent = `
9090
}
9191
}
9292
},
93-
"com.ubuntu.cloud:server:12.04:armhf": {
93+
"com.ubuntu.cloud:server:12.04:arm64": {
9494
"release": "precise",
9595
"version": "12.04",
96-
"arch": "armhf",
96+
"arch": "arm64",
9797
"versions": {
9898
"20121218": {
9999
"items": {
@@ -116,7 +116,7 @@ var jsonImagesContent = `
116116
"id": "ami-00000036"
117117
}
118118
},
119-
"pubname": "ubuntu-precise-12.04-armhf-server-20121218",
119+
"pubname": "ubuntu-precise-12.04-arm64-server-20121218",
120120
"label": "release"
121121
}
122122
}
@@ -204,7 +204,7 @@ var jsonImagesContent = `
204204
type instanceSpecTestParams struct {
205205
desc string
206206
region string
207-
arches []string
207+
arch string
208208
stream string
209209
constraints string
210210
instanceTypes []InstanceType
@@ -215,11 +215,11 @@ type instanceSpecTestParams struct {
215215
}
216216

217217
func (p *instanceSpecTestParams) init() {
218-
if p.arches == nil {
219-
p.arches = []string{"amd64", "armhf"}
218+
if p.arch == "" {
219+
p.arch = "amd64"
220220
}
221221
if p.instanceTypes == nil {
222-
p.instanceTypes = []InstanceType{{Id: "1", Name: "it-1", Arches: []string{"amd64", "armhf"}}}
222+
p.instanceTypes = []InstanceType{{Id: "1", Name: "it-1", Arch: "amd64"}}
223223
p.instanceTypeId = "1"
224224
p.instanceTypeName = "it-1"
225225
}
@@ -232,43 +232,7 @@ var findInstanceSpecTests = []instanceSpecTestParams{
232232
region: "test",
233233
imageId: "ami-00000033",
234234
instanceTypes: []InstanceType{
235-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &pv, Mem: 512},
236-
},
237-
},
238-
{
239-
desc: "prefer amd64 over i386",
240-
region: "test",
241-
imageId: "ami-00000033",
242-
arches: []string{"amd64", "i386"},
243-
instanceTypes: []InstanceType{
244-
{Id: "1", Name: "it-1", Arches: []string{"i386", "amd64"}, VirtType: &pv, Mem: 512},
245-
},
246-
},
247-
{
248-
desc: "prefer armhf over i386 (first alphabetical wins)",
249-
region: "test",
250-
imageId: "ami-00000034",
251-
arches: []string{"armhf", "i386"},
252-
instanceTypes: []InstanceType{
253-
{Id: "1", Name: "it-1", Arches: []string{"armhf", "i386"}, VirtType: &pv, Mem: 512},
254-
},
255-
},
256-
{
257-
desc: "prefer ppc64el over i386 (64-bit trumps 32-bit, regardless of alphabetical order)",
258-
region: "test",
259-
imageId: "ami-b79b09b9",
260-
arches: []string{"ppc64el", "i386"},
261-
instanceTypes: []InstanceType{
262-
{Id: "1", Name: "it-1", Arches: []string{"i386", "ppc64el"}, VirtType: &pv, Mem: 512},
263-
},
264-
},
265-
{
266-
desc: "prefer amd64 over arm64 (first 64-bit alphabetical wins)",
267-
region: "test",
268-
imageId: "ami-00000033",
269-
arches: []string{"arm64", "amd64"},
270-
instanceTypes: []InstanceType{
271-
{Id: "1", Name: "it-1", Arches: []string{"arm64", "amd64"}, VirtType: &pv, Mem: 512},
235+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &pv, Mem: 512},
272236
},
273237
},
274238
{
@@ -277,7 +241,7 @@ var findInstanceSpecTests = []instanceSpecTestParams{
277241
stream: "released",
278242
imageId: "ami-00000035",
279243
instanceTypes: []InstanceType{
280-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 512, CpuCores: 2},
244+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &hvm, Mem: 512, CpuCores: 2},
281245
},
282246
},
283247
{
@@ -286,15 +250,15 @@ var findInstanceSpecTests = []instanceSpecTestParams{
286250
stream: "daily",
287251
imageId: "ami-10000035",
288252
instanceTypes: []InstanceType{
289-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 512, CpuCores: 2},
253+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &hvm, Mem: 512, CpuCores: 2},
290254
},
291255
},
292256
{
293257
desc: "multiple images exists in metadata, use most recent",
294258
region: "test",
295259
imageId: "ami-00000035",
296260
instanceTypes: []InstanceType{
297-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 512, CpuCores: 2},
261+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &hvm, Mem: 512, CpuCores: 2},
298262
},
299263
},
300264
{
@@ -303,7 +267,7 @@ var findInstanceSpecTests = []instanceSpecTestParams{
303267
constraints: "instance-type=",
304268
imageId: "ami-00000033",
305269
instanceTypes: []InstanceType{
306-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &pv, Mem: 512},
270+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &pv, Mem: 512},
307271
},
308272
},
309273
{
@@ -312,34 +276,44 @@ var findInstanceSpecTests = []instanceSpecTestParams{
312276
constraints: "instance-type=it-1",
313277
imageId: "ami-00000035",
314278
instanceTypes: []InstanceType{
315-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 512, CpuCores: 2},
316-
{Id: "2", Name: "it-2", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 1024, CpuCores: 2},
279+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &hvm, Mem: 512, CpuCores: 2},
280+
{Id: "2", Name: "it-2", Arch: "amd64", VirtType: &hvm, Mem: 1024, CpuCores: 2},
281+
},
282+
},
283+
{
284+
desc: "use instance type non amd64 constraint",
285+
region: "test",
286+
arch: "ppc64el",
287+
imageId: "ami-b79b09b9",
288+
instanceTypes: []InstanceType{
289+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &pv, Mem: 4096, CpuCores: 2, Cost: 1},
290+
{Id: "2", Name: "it-2", Arch: "ppc64el", VirtType: &pv, Mem: 4096, CpuCores: 2, Cost: 2},
317291
},
318292
},
319293
{
320294
desc: "instance type constraint, no matching instance types",
321295
region: "test",
322296
constraints: "instance-type=it-10",
323297
instanceTypes: []InstanceType{
324-
{Id: "1", Name: "it-1", Arches: []string{"amd64"}, VirtType: &hvm, Mem: 512, CpuCores: 2},
298+
{Id: "1", Name: "it-1", Arch: "amd64", VirtType: &hvm, Mem: 512, CpuCores: 2},
325299
},
326-
err: `no instance types in test matching constraints "instance-type=it-10"`,
300+
err: `no instance types in test matching constraints "arch=amd64 instance-type=it-10"`,
327301
},
328302
{
329303
desc: "no image exists in metadata",
330304
region: "invalid-region",
331-
err: `no metadata for "precise" images in invalid-region with arches \[amd64 armhf\]`,
305+
err: `no metadata for "precise" images in invalid-region with arch amd64`,
332306
},
333307
{
334308
desc: "no valid instance types",
335309
region: "test",
336310
instanceTypes: []InstanceType{},
337-
err: `no instance types in test matching constraints ""`,
311+
err: `no instance types in test matching constraints "arch=amd64"`,
338312
},
339313
{
340314
desc: "no compatible instance types",
341315
region: "arm-only",
342-
instanceTypes: []InstanceType{{Id: "1", Name: "it-1", Arches: []string{"amd64"}, Mem: 2048}},
316+
instanceTypes: []InstanceType{{Id: "1", Name: "it-1", Arch: "amd64", Mem: 2048}},
343317
err: `no "precise" images in arm-only matching instance types \[it-1\]`,
344318
},
345319
}
@@ -351,7 +325,6 @@ func (s *imageSuite) TestFindInstanceSpec(c *gc.C) {
351325
cons, err := imagemetadata.NewImageConstraint(simplestreams.LookupParams{
352326
CloudSpec: simplestreams.CloudSpec{t.region, "ep"},
353327
Releases: []string{"precise"},
354-
Arches: t.arches,
355328
Stream: t.stream,
356329
})
357330
c.Assert(err, jc.ErrorIsNil)
@@ -376,7 +349,7 @@ func (s *imageSuite) TestFindInstanceSpec(c *gc.C) {
376349
spec, err := FindInstanceSpec(images, &InstanceConstraint{
377350
Series: "precise",
378351
Region: t.region,
379-
Arches: t.arches,
352+
Arch: t.arch,
380353
Constraints: imageCons,
381354
}, t.instanceTypes)
382355
if t.err != "" {
@@ -404,30 +377,30 @@ var imageMatchtests = []struct {
404377
}{
405378
{
406379
image: Image{Arch: "amd64"},
407-
itype: InstanceType{Arches: []string{"amd64"}},
380+
itype: InstanceType{Arch: "amd64"},
408381
match: exactMatch,
409382
}, {
410383
image: Image{Arch: "amd64"},
411-
itype: InstanceType{Arches: []string{"amd64", "armhf"}},
384+
itype: InstanceType{Arch: "amd64"},
412385
match: exactMatch,
413386
}, {
414387
image: Image{Arch: "amd64", VirtType: hvm},
415-
itype: InstanceType{Arches: []string{"amd64"}, VirtType: &hvm},
388+
itype: InstanceType{Arch: "amd64", VirtType: &hvm},
416389
match: exactMatch,
417390
}, {
418-
image: Image{Arch: "armhf"},
419-
itype: InstanceType{Arches: []string{"amd64"}},
391+
image: Image{Arch: "arm64"},
392+
itype: InstanceType{Arch: "amd64"},
420393
}, {
421394
image: Image{Arch: "amd64", VirtType: hvm},
422-
itype: InstanceType{Arches: []string{"amd64"}},
395+
itype: InstanceType{Arch: "amd64"},
423396
match: exactMatch,
424397
}, {
425398
image: Image{Arch: "amd64"}, // no known virt type
426-
itype: InstanceType{Arches: []string{"amd64"}, VirtType: &hvm},
399+
itype: InstanceType{Arch: "amd64", VirtType: &hvm},
427400
match: partialMatch,
428401
}, {
429402
image: Image{Arch: "amd64", VirtType: "pv"},
430-
itype: InstanceType{Arches: []string{"amd64"}, VirtType: &hvm},
403+
itype: InstanceType{Arch: "amd64", VirtType: &hvm},
431404
match: nonMatch,
432405
},
433406
}
@@ -484,15 +457,15 @@ func (*imageSuite) TestInstanceConstraintString(c *gc.C) {
484457
ic := &InstanceConstraint{
485458
Series: "precise",
486459
Region: "region",
487-
Arches: []string{"amd64", "arm64"},
460+
Arch: "amd64",
488461
Constraints: imageCons,
489462
}
490463
c.Assert(
491464
ic.String(), gc.Equals,
492-
"{region: region, series: precise, arches: [amd64 arm64], constraints: mem=4096M, storage: []}")
465+
"{region: region, series: precise, arch: amd64, constraints: mem=4096M, storage: []}")
493466

494467
ic.Storage = []string{"ebs", "ssd"}
495468
c.Assert(
496469
ic.String(), gc.Equals,
497-
"{region: region, series: precise, arches: [amd64 arm64], constraints: mem=4096M, storage: [ebs ssd]}")
470+
"{region: region, series: precise, arch: amd64, constraints: mem=4096M, storage: [ebs ssd]}")
498471
}

0 commit comments

Comments
 (0)