Skip to content

Commit

Permalink
ACM-17553: Save version of imported clusters (#7250)
Browse files Browse the repository at this point in the history
This is a backport of OCPBUGS-42059 for ACM 2.12.

Currently when a cluster is imported the `openshift_version` column of
the database is not populated. As a result that version will not be
available for validations, in particular for the validation that checks
if dual-stack VIPs are supported. A side effect of that is that adding a
node to a cluster with dual-stack VIPs will always fail with an error
message like this, regardless of the version:

```
"dual-stack VIPs are not supported in OpenShift "
```

That error happens because we are trying to see if the version is less
than 4.12, but the result is always true because the version is empty.

To avoid that this patch changes the cluster import code so that it
populates the `openshift_version` database column.

Related: https://issues.redhat.com/browse/ACM-17553
Related: https://issues.redhat.com/browse/OCPBUGS-42059

Signed-off-by: Juan Hernandez <[email protected]>
  • Loading branch information
jhernand authored Feb 4, 2025
1 parent da7af47 commit b80d433
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 4 deletions.
1 change: 1 addition & 0 deletions internal/bminventory/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,7 @@ func (b *bareMetalInventory) V2ImportClusterInternal(ctx context.Context, kubeKe
Kind: swag.String(models.ClusterKindAddHostsCluster),
Name: clusterName,
OpenshiftClusterID: common.StrFmtUUIDVal(params.NewImportClusterParams.OpenshiftClusterID),
OpenshiftVersion: params.NewImportClusterParams.OpenshiftVersion,
UserName: ocm.UserNameFromContext(ctx),
OrgID: ocm.OrgIDFromContext(ctx),
EmailDomain: ocm.EmailDomainFromContext(ctx),
Expand Down
4 changes: 2 additions & 2 deletions internal/bminventory/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12679,7 +12679,7 @@ var _ = Describe("Register AddHostsCluster test", func() {

Expect(actual.Payload.HostNetworks).To(Equal(defaultHostNetworks))
Expect(actual.Payload.Hosts).To(Equal(defaultHosts))
Expect(actual.Payload.OpenshiftVersion).To(BeEmpty())
Expect(actual.Payload.OpenshiftVersion).To(Equal(common.TestDefaultConfig.OpenShiftVersion))
Expect(actual.Payload.OcpReleaseImage).To(BeEmpty())
Expect(actual.Payload.OpenshiftClusterID).To(Equal(openshiftClusterID))
Expect(res).Should(BeAssignableToTypeOf(installer.NewV2ImportClusterCreated()))
Expand All @@ -12706,7 +12706,7 @@ var _ = Describe("Register AddHostsCluster test", func() {

Expect(actual.Payload.HostNetworks).To(Equal(defaultHostNetworks))
Expect(actual.Payload.Hosts).To(Equal(defaultHosts))
Expect(actual.Payload.OpenshiftVersion).To(BeEmpty())
Expect(actual.Payload.OpenshiftVersion).To(Equal(common.TestDefaultConfig.OpenShiftVersion))
Expect(actual.Payload.OcpReleaseImage).To(BeEmpty())
Expect(actual.Payload.OpenshiftClusterID).To(Equal(openshiftClusterID))
Expect(res).Should(BeAssignableToTypeOf(installer.NewV2ImportClusterCreated()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (r *ClusterDeploymentsReconciler) Reconcile(origCtx context.Context, req ct
return r.createNewCluster(ctx, log, req.NamespacedName, pullSecret, releaseImage, clusterDeployment, clusterInstall)
}

return r.createNewDay2Cluster(ctx, log, req.NamespacedName, clusterDeployment, clusterInstall)
return r.createNewDay2Cluster(ctx, log, req.NamespacedName, releaseImage, clusterDeployment, clusterInstall)
}
if err != nil {
return r.updateStatus(ctx, log, clusterInstall, clusterDeployment, cluster, err)
Expand Down Expand Up @@ -1410,6 +1410,7 @@ func (r *ClusterDeploymentsReconciler) createNewDay2Cluster(
ctx context.Context,
log logrus.FieldLogger,
key types.NamespacedName,
releaseImage *models.ReleaseImage,
clusterDeployment *hivev1.ClusterDeployment,
clusterInstall *hiveext.AgentClusterInstall) (ctrl.Result, error) {

Expand All @@ -1422,6 +1423,9 @@ func (r *ClusterDeploymentsReconciler) createNewDay2Cluster(
APIVipDnsname: swag.String(apiVipDnsname),
Name: swag.String(spec.ClusterName),
}
if releaseImage != nil && releaseImage.OpenshiftVersion != nil {
clusterParams.OpenshiftVersion = *releaseImage.OpenshiftVersion
}

// add optional parameter
if clusterInstall.Spec.ClusterMetadata != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3387,6 +3387,7 @@ var _ = Describe("cluster reconcile", func() {
V2ImportClusterInternal := func(ctx context.Context, kubeKey *types.NamespacedName, id *strfmt.UUID,
params installer.V2ImportClusterParams) (*common.Cluster, error) {
Expect(string(*params.NewImportClusterParams.OpenshiftClusterID)).To(Equal(cid))
Expect(params.NewImportClusterParams.OpenshiftVersion).To(Equal(ocpVersion))
return clusterReply, nil
}
mockInstallerInternal.EXPECT().
Expand Down
2 changes: 1 addition & 1 deletion subsystem/day2_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var _ = Describe("Day2 v2 cluster tests", func() {
Expect(err).NotTo(HaveOccurred())
Expect(swag.StringValue(cluster.GetPayload().Status)).Should(Equal("adding-hosts"))
Expect(swag.StringValue(cluster.GetPayload().StatusInfo)).Should(Equal(statusInfoAddingHosts))
Expect(swag.StringValue(&cluster.GetPayload().OpenshiftVersion)).Should(BeEmpty())
Expect(swag.StringValue(&cluster.GetPayload().OpenshiftVersion)).Should(Equal(openshiftVersion))
Expect(swag.StringValue(&cluster.GetPayload().OcpReleaseImage)).Should(BeEmpty())
Expect(cluster.GetPayload().StatusUpdatedAt).ShouldNot(Equal(strfmt.DateTime(time.Time{})))

Expand Down

0 comments on commit b80d433

Please sign in to comment.