Skip to content

Commit cb1d134

Browse files
dlomdlom
committed
Revendor (functional fixes)
1 parent 715f338 commit cb1d134

6 files changed

Lines changed: 272 additions & 13 deletions

File tree

‎pkg/clusterresource/ovirt.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ func (p *OvirtCloudBuilder) addInstallConfigPlatform(o *Builder, ic *installerty
117117
ClusterID: p.ClusterID,
118118
StorageDomainID: p.StorageDomainID,
119119
NetworkName: p.NetworkName,
120-
APIVIP: p.APIVIP,
121-
IngressVIP: p.IngressVIP,
120+
APIVIPs: []string{p.APIVIP},
121+
IngressVIPs: []string{p.IngressVIP},
122122
},
123123
}
124124
}

‎pkg/clusterresource/vsphere.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ func (p *VSphereCloudBuilder) addInstallConfigPlatform(o *Builder, ic *installer
142142
DefaultDatastore: p.DefaultDatastore,
143143
Folder: p.Folder,
144144
Cluster: p.Cluster,
145-
APIVIP: p.APIVIP,
146-
IngressVIP: p.IngressVIP,
145+
APIVIPs: []string{p.APIVIP},
146+
IngressVIPs: []string{p.IngressVIP},
147147
Network: p.Network,
148148
},
149149
}

‎pkg/controller/machinepool/alibabacloudactuator_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ func TestAlibabaCloudActuator(t *testing.T) {
4141
mockGetAvailableZonesByInstanceType(client, []string{"test-region-1", "test-region-2", "test-region-3"}, testAlibabaInstanceType)
4242
},
4343
expectedMachineSetReplicas: map[string]int32{
44-
generateAlibabaCloudMachineSetName("worker", "1"): 1,
45-
generateAlibabaCloudMachineSetName("worker", "2"): 1,
46-
generateAlibabaCloudMachineSetName("worker", "3"): 1,
44+
generateAlibabaCloudMachineSetName("worker-test-region", "1"): 1,
45+
generateAlibabaCloudMachineSetName("worker-test-region", "2"): 1,
46+
generateAlibabaCloudMachineSetName("worker-test-region", "3"): 1,
4747
},
4848
},
4949
{
@@ -55,9 +55,9 @@ func TestAlibabaCloudActuator(t *testing.T) {
5555
return p
5656
}(),
5757
expectedMachineSetReplicas: map[string]int32{
58-
generateAlibabaCloudMachineSetName("worker", "A"): 1,
59-
generateAlibabaCloudMachineSetName("worker", "B"): 1,
60-
generateAlibabaCloudMachineSetName("worker", "C"): 1,
58+
generateAlibabaCloudMachineSetName("worker-test-region", "A"): 1,
59+
generateAlibabaCloudMachineSetName("worker-test-region", "B"): 1,
60+
generateAlibabaCloudMachineSetName("worker-test-region", "C"): 1,
6161
},
6262
},
6363
{
@@ -84,9 +84,9 @@ func TestAlibabaCloudActuator(t *testing.T) {
8484
mockGetAvailableZonesByInstanceType(client, []string{"test-region-1", "test-region-2", "test-region-3"}, testAlibabaInstanceType)
8585
},
8686
expectedMachineSetReplicas: map[string]int32{
87-
generateAlibabaCloudMachineSetName("worker", "1"): 1,
88-
generateAlibabaCloudMachineSetName("worker", "2"): 1,
89-
generateAlibabaCloudMachineSetName("worker", "3"): 1,
87+
generateAlibabaCloudMachineSetName("worker-test-region", "1"): 1,
88+
generateAlibabaCloudMachineSetName("worker-test-region", "2"): 1,
89+
generateAlibabaCloudMachineSetName("worker-test-region", "3"): 1,
9090
},
9191
},
9292
}

‎pkg/controller/machinepool/azureactuator.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package machinepool
33
import (
44
"context"
55
"fmt"
6+
"github.com/blang/semver/v4"
67
"strings"
78
"time"
89

@@ -21,6 +22,8 @@ import (
2122
"github.com/openshift/hive/pkg/azureclient"
2223
)
2324

25+
var versionsSupportingAzureImageGallery = semver.MustParseRange(">=4.12.0")
26+
2427
// AzureActuator encapsulates the pieces necessary to be able to generate
2528
// a list of MachineSets to sync to the remote cluster.
2629
type AzureActuator struct {
@@ -135,6 +138,11 @@ func (a *AzureActuator) GenerateMachineSets(cd *hivev1.ClusterDeployment, pool *
135138
// The imageID parameter is not used. The image is determined by the infraID.
136139
const imageID = ""
137140

141+
useImageGallery, err := shouldUseImageGallery(cd)
142+
if err != nil {
143+
return nil, false, err
144+
}
145+
138146
installerMachineSets, err := installazure.MachineSets(
139147
cd.Spec.ClusterMetadata.InfraID,
140148
ic,
@@ -143,6 +151,7 @@ func (a *AzureActuator) GenerateMachineSets(cd *hivev1.ClusterDeployment, pool *
143151
workerRole,
144152
workerUserDataName,
145153
capabilities,
154+
useImageGallery,
146155
)
147156
return installerMachineSets, err == nil, errors.Wrap(err, "failed to generate machinesets")
148157
}
@@ -194,3 +203,17 @@ func (a *AzureActuator) gen2ImageExists(infraID, resourceGroupName string) (bool
194203
}
195204
return false, nil
196205
}
206+
207+
func shouldUseImageGallery(cd *hivev1.ClusterDeployment) (bool, error) {
208+
versionString, err := getClusterVersion(cd)
209+
if err != nil {
210+
return true, fmt.Errorf("failed to get cluster semver: %w", err)
211+
}
212+
213+
version, err := semver.ParseTolerant(versionString)
214+
if err != nil {
215+
return true, fmt.Errorf("failed to parse cluster semver: %w", err)
216+
}
217+
218+
return versionsSupportingAzureImageGallery(version), nil
219+
}

‎pkg/controller/machinepool/azureactuator_test.go‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package machinepool
33
import (
44
"context"
55
"fmt"
6+
"github.com/openshift/hive/pkg/constants"
67
"testing"
78

89
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
@@ -29,6 +30,7 @@ func TestAzureActuator(t *testing.T) {
2930
expectedImage *machineapi.Image
3031
expectedErr bool
3132
}{
33+
// < 4.12
3234
{
3335
name: "generate single machineset for single zone",
3436
clusterDeployment: testAzureClusterDeployment(),
@@ -251,6 +253,229 @@ func TestAzureActuator(t *testing.T) {
251253
Type: "MarketplaceWithPlan",
252254
},
253255
},
256+
// >= 4.12
257+
{
258+
name: "generate single machineset for single zone (4.12+)",
259+
clusterDeployment: testAzureClusterDeployment412(),
260+
pool: testAzurePool(),
261+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
262+
mockListResourceSKUs(mockCtrl, client, []string{"zone1"})
263+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
264+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
265+
testAzureImage(compute.HyperVGenerationTypesV1),
266+
})
267+
},
268+
expectedMachineSetReplicas: map[string]int64{
269+
generateAzureMachineSetName("zone1"): 3,
270+
},
271+
},
272+
{
273+
name: "generate machinesets across zones (4.12+)",
274+
clusterDeployment: testAzureClusterDeployment412(),
275+
pool: testAzurePool(),
276+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
277+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
278+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
279+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
280+
testAzureImage(compute.HyperVGenerationTypesV1),
281+
})
282+
},
283+
expectedMachineSetReplicas: map[string]int64{
284+
generateAzureMachineSetName("zone1"): 1,
285+
generateAzureMachineSetName("zone2"): 1,
286+
generateAzureMachineSetName("zone3"): 1,
287+
},
288+
},
289+
{
290+
name: "generate machinesets for specified zones (4.12+)",
291+
clusterDeployment: testAzureClusterDeployment412(),
292+
pool: func() *hivev1.MachinePool {
293+
pool := testAzurePool()
294+
pool.Spec.Platform.Azure.Zones = []string{"zone1", "zone2", "zone3"}
295+
return pool
296+
}(),
297+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
298+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
299+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
300+
testAzureImage(compute.HyperVGenerationTypesV1),
301+
})
302+
},
303+
expectedMachineSetReplicas: map[string]int64{
304+
generateAzureMachineSetName("zone1"): 1,
305+
generateAzureMachineSetName("zone2"): 1,
306+
generateAzureMachineSetName("zone3"): 1,
307+
},
308+
},
309+
{
310+
name: "more replicas than zones (4.12+)",
311+
clusterDeployment: testAzureClusterDeployment412(),
312+
pool: func() *hivev1.MachinePool {
313+
p := testAzurePool()
314+
p.Spec.Replicas = pointer.Int64Ptr(5)
315+
return p
316+
}(),
317+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
318+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
319+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
320+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
321+
testAzureImage(compute.HyperVGenerationTypesV1),
322+
})
323+
},
324+
expectedMachineSetReplicas: map[string]int64{
325+
generateAzureMachineSetName("zone1"): 2,
326+
generateAzureMachineSetName("zone2"): 2,
327+
generateAzureMachineSetName("zone3"): 1,
328+
},
329+
},
330+
{
331+
name: "more zones than replicas (4.12+)",
332+
clusterDeployment: testAzureClusterDeployment412(),
333+
pool: testAzurePool(),
334+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
335+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3", "zone4", "zone5"})
336+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
337+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
338+
testAzureImage(compute.HyperVGenerationTypesV1),
339+
})
340+
},
341+
expectedMachineSetReplicas: map[string]int64{
342+
generateAzureMachineSetName("zone1"): 1,
343+
generateAzureMachineSetName("zone2"): 1,
344+
generateAzureMachineSetName("zone3"): 1,
345+
generateAzureMachineSetName("zone4"): 0,
346+
generateAzureMachineSetName("zone5"): 0,
347+
},
348+
},
349+
{
350+
name: "list zones returns zero (4.12+)",
351+
clusterDeployment: testAzureClusterDeployment412(),
352+
pool: testAzurePool(),
353+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
354+
mockListResourceSKUs(mockCtrl, client, []string{})
355+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
356+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
357+
testAzureImage(compute.HyperVGenerationTypesV1),
358+
})
359+
},
360+
expectedErr: true,
361+
},
362+
{
363+
name: "default V1 image exists and instance supports V1 images (4.12+)",
364+
clusterDeployment: testAzureClusterDeployment412(),
365+
pool: testAzurePool(),
366+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
367+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
368+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
369+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
370+
testAzureImage(compute.HyperVGenerationTypesV1),
371+
})
372+
},
373+
expectedMachineSetReplicas: map[string]int64{
374+
generateAzureMachineSetName("zone1"): 1,
375+
generateAzureMachineSetName("zone2"): 1,
376+
generateAzureMachineSetName("zone3"): 1,
377+
},
378+
// V1 image is chosen for machinepool
379+
expectedImage: &machineapi.Image{
380+
ResourceID: "/resourceGroups/foo-12345-rg/providers/Microsoft.Compute/galleries/gallery_foo_12345/images/foo-12345/versions/latest",
381+
},
382+
},
383+
{
384+
name: "default V1 and V2 images exist but instance only supports V1 images (4.12+)",
385+
clusterDeployment: testAzureClusterDeployment412(),
386+
pool: testAzurePool(),
387+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
388+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
389+
mockGetVMCapabilities(mockCtrl, client, "V1")
390+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
391+
testAzureImage(compute.HyperVGenerationTypesV1),
392+
testAzureImage(compute.HyperVGenerationTypesV2),
393+
})
394+
},
395+
expectedMachineSetReplicas: map[string]int64{
396+
generateAzureMachineSetName("zone1"): 1,
397+
generateAzureMachineSetName("zone2"): 1,
398+
generateAzureMachineSetName("zone3"): 1,
399+
},
400+
// V1 image is chosen for machinepool
401+
expectedImage: &machineapi.Image{
402+
ResourceID: "/resourceGroups/foo-12345-rg/providers/Microsoft.Compute/galleries/gallery_foo_12345/images/foo-12345/versions/latest",
403+
},
404+
},
405+
{
406+
name: "default V1 and V2 images exist and instance supports V1 and V2 images (4.12+)",
407+
clusterDeployment: testAzureClusterDeployment412(),
408+
pool: testAzurePool(),
409+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
410+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
411+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
412+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
413+
testAzureImage(compute.HyperVGenerationTypesV1),
414+
testAzureImage(compute.HyperVGenerationTypesV2),
415+
})
416+
},
417+
expectedMachineSetReplicas: map[string]int64{
418+
generateAzureMachineSetName("zone1"): 1,
419+
generateAzureMachineSetName("zone2"): 1,
420+
generateAzureMachineSetName("zone3"): 1,
421+
},
422+
// V2 ("-gen2") image is chosen for machinepool
423+
expectedImage: &machineapi.Image{
424+
ResourceID: "/resourceGroups/foo-12345-rg/providers/Microsoft.Compute/galleries/gallery_foo_12345/images/foo-12345-gen2/versions/latest",
425+
},
426+
},
427+
{
428+
name: "default V1 and V2 images exist but instance only supports V2 images (4.12+)",
429+
clusterDeployment: testAzureClusterDeployment412(),
430+
pool: testAzurePool(),
431+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
432+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
433+
mockGetVMCapabilities(mockCtrl, client, "V2")
434+
mockListImagesByResourceGroup(mockCtrl, client, []compute.Image{
435+
testAzureImage(compute.HyperVGenerationTypesV1),
436+
testAzureImage(compute.HyperVGenerationTypesV2),
437+
})
438+
},
439+
expectedMachineSetReplicas: map[string]int64{
440+
generateAzureMachineSetName("zone1"): 1,
441+
generateAzureMachineSetName("zone2"): 1,
442+
generateAzureMachineSetName("zone3"): 1,
443+
},
444+
// V2 ("-gen2") image is chosen for machinepool
445+
expectedImage: &machineapi.Image{
446+
ResourceID: "/resourceGroups/foo-12345-rg/providers/Microsoft.Compute/galleries/gallery_foo_12345/images/foo-12345-gen2/versions/latest",
447+
},
448+
},
449+
{
450+
name: "machinepool provides osImage (4.12+)",
451+
clusterDeployment: testAzureClusterDeployment412(),
452+
pool: func() *hivev1.MachinePool {
453+
mp := testAzurePool()
454+
mp.Spec.Platform.Azure.OSImage = &hivev1azure.OSImage{
455+
Publisher: "testpublisher",
456+
Offer: "testoffer",
457+
SKU: "testsku",
458+
Version: "testversion",
459+
}
460+
return mp
461+
}(),
462+
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
463+
mockGetVMCapabilities(mockCtrl, client, "V1,V2")
464+
mockListResourceSKUs(mockCtrl, client, []string{"zone1", "zone2", "zone3"})
465+
},
466+
expectedMachineSetReplicas: map[string]int64{
467+
generateAzureMachineSetName("zone1"): 1,
468+
generateAzureMachineSetName("zone2"): 1,
469+
generateAzureMachineSetName("zone3"): 1,
470+
},
471+
expectedImage: &machineapi.Image{
472+
Publisher: "testpublisher",
473+
Offer: "testoffer",
474+
SKU: "testsku",
475+
Version: "testversion",
476+
Type: "MarketplaceWithPlan",
477+
},
478+
},
254479
}
255480

256481
for _, test := range tests {
@@ -364,6 +589,12 @@ func testAzureClusterDeployment() *hivev1.ClusterDeployment {
364589
return cd
365590
}
366591

592+
func testAzureClusterDeployment412() *hivev1.ClusterDeployment {
593+
cd := testAzureClusterDeployment()
594+
cd.Labels[constants.VersionMajorMinorPatchLabel] = "4.12.0"
595+
return cd
596+
}
597+
367598
func testAzureImage(hyperVGen compute.HyperVGenerationTypes) compute.Image {
368599
return compute.Image{
369600
ImageProperties: &compute.ImageProperties{

‎pkg/controller/machinepool/ibmcloudactuator.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ func (a *IBMCloudActuator) GenerateMachineSets(cd *hivev1.ClusterDeployment, poo
103103
installerMachineSets, err := installibmcloud.MachineSets(
104104
cd.Spec.ClusterMetadata.InfraID,
105105
ic,
106+
107+
// A default empty map will work fine here, see:
108+
// vendor/github.com/openshift/installer/pkg/asset/machines/ibmcloud/machines.go:148
109+
make(map[string]string),
110+
106111
computePool,
107112
workerRole,
108113
workerUserDataName,

0 commit comments

Comments
 (0)