Skip to content

Commit 6ef5dd2

Browse files
CrystalChunCrystalChun
committed
MGMT-23339: Update register cluster to use release image URL if specified
1 parent 89885bf commit 6ef5dd2

File tree

2 files changed

+100
-12
lines changed

2 files changed

+100
-12
lines changed

‎internal/bminventory/inventory.go‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -564,11 +564,9 @@ func (b *bareMetalInventory) getOLMMonitoredOperators(log *logrus.Entry, cluster
564564
}
565565

566566
func (b *bareMetalInventory) getNewClusterReleaseImage(ctx context.Context, params *models.ClusterCreateParams, arch string) (*models.ReleaseImage, error) {
567-
releaseImage, err := b.versionsHandler.GetReleaseImage(ctx, swag.StringValue(params.OpenshiftVersion),
568-
arch, swag.StringValue(params.PullSecret))
567+
releaseImage, err := b.getReleaseImage(ctx, params, arch)
569568
if err != nil {
570-
return nil, errors.Wrapf(err, "Openshift version %s for CPU architecture %s is not supported",
571-
swag.StringValue(params.OpenshiftVersion), arch)
569+
return nil, err
572570
}
573571

574572
if !b.EnableImageService {
@@ -590,6 +588,37 @@ func (b *bareMetalInventory) getNewClusterReleaseImage(ctx context.Context, para
590588
return releaseImage, nil
591589
}
592590

591+
func (b *bareMetalInventory) getReleaseImage(ctx context.Context, params *models.ClusterCreateParams, arch string) (*models.ReleaseImage, error) {
592+
if params.OcpReleaseImage != "" {
593+
releaseImage, err := b.versionsHandler.GetReleaseImageByURL(ctx, params.OcpReleaseImage, swag.StringValue(params.PullSecret))
594+
if err != nil {
595+
return nil, errors.Wrapf(err, "failed to get release image by URL %s during cluster registration", params.OcpReleaseImage)
596+
}
597+
598+
normalizedArch := common.NormalizeCPUArchitecture(arch)
599+
if normalizedArch == common.MultiCPUArchitecture {
600+
if len(releaseImage.CPUArchitectures) <= 1 {
601+
return nil, errors.Errorf("release image URL %s does not support requested CPU architecture %s", params.OcpReleaseImage, normalizedArch)
602+
}
603+
return releaseImage, nil
604+
}
605+
for _, cpuArch := range releaseImage.CPUArchitectures {
606+
normalizedCpuArch := common.NormalizeCPUArchitecture(cpuArch)
607+
if normalizedCpuArch == normalizedArch {
608+
return releaseImage, nil
609+
}
610+
}
611+
return nil, errors.Errorf("Requested CPU architecture %s is not supported for release image URL %s. Supported architectures in release image: %v", normalizedArch, params.OcpReleaseImage, releaseImage.CPUArchitectures)
612+
}
613+
614+
releaseImage, err := b.versionsHandler.GetReleaseImage(ctx, swag.StringValue(params.OpenshiftVersion), arch, swag.StringValue(params.PullSecret))
615+
if err != nil {
616+
return nil, errors.Wrapf(err, "Openshift version %s for CPU architecture %s is not supported",
617+
swag.StringValue(params.OpenshiftVersion), arch)
618+
}
619+
return releaseImage, nil
620+
}
621+
593622
func MarshalNewClusterParamsNoPullSecret(params installer.V2RegisterClusterParams, log *logrus.Entry) []byte {
594623
// Masked the Pull Secret to prevent its value from being displayed in the logs.
595624
clusterParamsNoPullSecret := *params.NewClusterParams

‎internal/bminventory/inventory_test.go‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,17 +265,30 @@ func toMac(macStr string) *strfmt.MAC {
265265
return &mac
266266
}
267267

268-
func mockClusterRegisterSteps() {
268+
func mockClusterRegisterSteps(withReleaseImageURL bool) {
269269
mockSecretValidator.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(1)
270-
mockVersions.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(common.TestDefaultConfig.ReleaseImage, nil).Times(1)
270+
if withReleaseImageURL {
271+
mockVersions.EXPECT().GetReleaseImageByURL(gomock.Any(), gomock.Any(), gomock.Any()).Return(common.TestDefaultConfig.ReleaseImage, nil).Times(1)
272+
} else {
273+
mockVersions.EXPECT().GetReleaseImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(common.TestDefaultConfig.ReleaseImage, nil).Times(1)
274+
}
271275

272276
mockOSImages.EXPECT().GetOsImage(gomock.Any(), gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).Times(1)
273277
mockOperatorManager.EXPECT().GetSupportedOperatorsByType(models.OperatorTypeBuiltin).Return([]*models.MonitoredOperator{&common.TestDefaultConfig.MonitoredOperator}).Times(1)
274278
mockProviderRegistry.EXPECT().SetPlatformUsages(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
275279
}
276280

277281
func mockClusterRegisterSuccess(withEvents bool) {
278-
mockClusterRegisterSteps()
282+
mockClusterRegisterSteps(false)
283+
mockMetric.EXPECT().ClusterRegistered().Times(1)
284+
285+
if withEvents {
286+
mockEvents.EXPECT().SendClusterEvent(gomock.Any(), eventstest.NewEventMatcher(
287+
eventstest.WithNameMatcher(eventgen.ClusterRegistrationSucceededEventName))).Times(1)
288+
}
289+
}
290+
func mockClusterRegisterSuccessWithReleaseImageURL(withEvents bool) {
291+
mockClusterRegisterSteps(true)
279292
mockMetric.EXPECT().ClusterRegistered().Times(1)
280293

281294
if withEvents {
@@ -14676,7 +14689,7 @@ var _ = Describe("RegisterCluster", func() {
1467614689
})
1467714690
Context("Pull secret validation", func() {
1467814691
It("Successfully validates the pull secret if it does not contain auth for a mirrored registry", func() {
14679-
mockClusterRegisterSuccess(true)
14692+
mockClusterRegisterSuccessWithReleaseImageURL(true)
1468014693
mockAMSSubscription(ctx)
1468114694
conf, _ := getMirrorRegistryConfigurations(getSecureRegistryToml("fake-registry.example.com", mirrorRegistry), mirrorRegistryCertificate)
1468214695
params := getClusterCreateParams()
@@ -16373,7 +16386,7 @@ var _ = Describe("RegisterCluster", func() {
1637316386
It("cluster api failed to register", func() {
1637416387
bm.clusterApi = mockClusterApi
1637516388
mockClusterApi.EXPECT().RegisterCluster(ctx, gomock.Any()).Return(errors.Errorf("error")).Times(1)
16376-
mockClusterRegisterSteps()
16389+
mockClusterRegisterSteps(false)
1637716390

1637816391
reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{
1637916392
NewClusterParams: getDefaultClusterCreateParams(),
@@ -16433,6 +16446,20 @@ var _ = Describe("RegisterCluster", func() {
1643316446
Expect(actual.Payload.OcpReleaseImage).To(Equal(common.TestDefaultConfig.ReleaseImageUrl))
1643416447
})
1643516448

16449+
It("successfully registers cluster with openshift release image URL defined in the cluster create params", func() {
16450+
mockClusterRegisterSuccessWithReleaseImageURL(true)
16451+
mockAMSSubscription(ctx)
16452+
clusterParams := getDefaultClusterCreateParams()
16453+
clusterParams.OcpReleaseImage = common.TestDefaultConfig.ReleaseImageUrl
16454+
reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{
16455+
NewClusterParams: clusterParams,
16456+
})
16457+
Expect(reflect.TypeOf(reply)).Should(Equal(reflect.TypeOf(installer.NewV2RegisterClusterCreated())))
16458+
actual := reply.(*installer.V2RegisterClusterCreated)
16459+
Expect(actual.Payload.OpenshiftVersion).To(Equal(common.TestDefaultConfig.ReleaseVersion))
16460+
Expect(actual.Payload.OcpReleaseImage).To(Equal(common.TestDefaultConfig.ReleaseImageUrl))
16461+
})
16462+
1643616463
It("Register cluster with default CPU architecture", func() {
1643716464
mockClusterRegisterSuccess(true)
1643816465
mockAMSSubscription(ctx)
@@ -16531,6 +16558,38 @@ var _ = Describe("RegisterCluster", func() {
1653116558
actual := reply.(*installer.V2RegisterClusterCreated)
1653216559
Expect(actual.Payload.CPUArchitecture).To(Equal(common.TestDefaultConfig.CPUArchitecture))
1653316560
})
16561+
Context("Register with release image URL", func() {
16562+
It("should register cluster with release image URL successfully", func() {
16563+
mockClusterRegisterSuccessWithReleaseImageURL(true)
16564+
mockAMSSubscription(ctx)
16565+
clusterParams := getDefaultClusterCreateParams()
16566+
clusterParams.OcpReleaseImage = common.TestDefaultConfig.ReleaseImageUrl
16567+
reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{
16568+
NewClusterParams: clusterParams,
16569+
})
16570+
Expect(reflect.TypeOf(reply)).Should(Equal(reflect.TypeOf(installer.NewV2RegisterClusterCreated())))
16571+
actual := reply.(*installer.V2RegisterClusterCreated)
16572+
Expect(actual.Payload.OpenshiftVersion).To(Equal(common.TestDefaultConfig.ReleaseVersion))
16573+
Expect(actual.Payload.OcpReleaseImage).To(Equal(common.TestDefaultConfig.ReleaseImageUrl))
16574+
})
16575+
16576+
It("should fail if openshift release image is different from cluster architecture", func() {
16577+
mockVersions.EXPECT().GetReleaseImageByURL(gomock.Any(), gomock.Any(), gomock.Any()).Return(&models.ReleaseImage{
16578+
CPUArchitecture: swag.String(common.ARM64CPUArchitecture),
16579+
CPUArchitectures: []string{common.ARM64CPUArchitecture},
16580+
OpenshiftVersion: swag.String("4.12.0"),
16581+
URL: swag.String(common.TestDefaultConfig.ReleaseImageUrl),
16582+
Version: swag.String("4.12.0"),
16583+
}, nil).Times(1)
16584+
16585+
clusterParams := getDefaultClusterCreateParams()
16586+
clusterParams.OcpReleaseImage = common.TestDefaultConfig.ReleaseImageUrl
16587+
reply := bm.V2RegisterCluster(ctx, installer.V2RegisterClusterParams{
16588+
NewClusterParams: clusterParams,
16589+
})
16590+
Expect(reply).Should(BeAssignableToTypeOf(common.NewApiError(http.StatusBadRequest, errors.Errorf("Requested CPU architecture %s is not supported for release image URL %s. Supported architectures in release image: %v", common.ARM64CPUArchitecture, common.TestDefaultConfig.ReleaseImageUrl, []string{common.ARM64CPUArchitecture}))))
16591+
})
16592+
})
1653416593

1653516594
Context("Cluster Tags", func() {
1653616595
It("Register cluster with tags", func() {
@@ -16649,7 +16708,7 @@ var _ = Describe("RegisterCluster", func() {
1664916708
payload.Username = userName1
1665016709
payload.Organization = orgID1
1665116710
authCtx = context.WithValue(ctx, restapi.AuthKey, payload)
16652-
mockClusterRegisterSteps()
16711+
mockClusterRegisterSteps(false)
1665316712
mockUsageReports()
1665416713
mockAMSSubscription(authCtx)
1665516714
mockMetric.EXPECT().ClusterRegistered().Times(1)
@@ -17512,7 +17571,7 @@ var _ = Describe("AMS subscriptions", func() {
1751217571
It("register cluster - deregister if we failed to create AMS subscription", func() {
1751317572
bm.clusterApi = mockClusterApi
1751417573
mockClusterApi.EXPECT().RegisterCluster(ctx, gomock.Any()).Return(nil)
17515-
mockClusterRegisterSteps()
17574+
mockClusterRegisterSteps(false)
1751617575
mockAccountsMgmt.EXPECT().CreateSubscription(ctx, gomock.Any(), clusterName).Return(nil, errors.New("dummy"))
1751717576
mockClusterApi.EXPECT().DeregisterCluster(ctx, gomock.Any())
1751817577

@@ -17527,7 +17586,7 @@ var _ = Describe("AMS subscriptions", func() {
1752717586
It("register cluster - delete AMS subscription if we failed to patch DB with ams_subscription_id", func() {
1752817587
bm.clusterApi = mockClusterApi
1752917588
mockClusterApi.EXPECT().RegisterCluster(ctx, gomock.Any()).Return(nil)
17530-
mockClusterRegisterSteps()
17589+
mockClusterRegisterSteps(false)
1753117590
mockAMSSubscription(ctx)
1753217591
mockClusterApi.EXPECT().UpdateAmsSubscriptionID(ctx, gomock.Any(), strfmt.UUID("")).Return(common.NewApiError(http.StatusInternalServerError, errors.New("dummy")))
1753317592
mockClusterApi.EXPECT().DeregisterCluster(ctx, gomock.Any())

0 commit comments

Comments
 (0)