From 4671cf3e7832c293af66e6ce66a2cf649ce6b68c Mon Sep 17 00:00:00 2001 From: Juan Hernandez Date: Fri, 31 Jan 2025 11:55:32 +0100 Subject: [PATCH] ACM-17553: Save version of imported clusters 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 --- internal/bminventory/inventory.go | 1 + internal/bminventory/inventory_test.go | 4 ++-- .../controller/controllers/clusterdeployments_controller.go | 6 +++++- .../controllers/clusterdeployments_controller_test.go | 1 + 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go index 54c72389ab0..55a7da7489b 100644 --- a/internal/bminventory/inventory.go +++ b/internal/bminventory/inventory.go @@ -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), diff --git a/internal/bminventory/inventory_test.go b/internal/bminventory/inventory_test.go index 6b02e8cff1b..9435b000c93 100644 --- a/internal/bminventory/inventory_test.go +++ b/internal/bminventory/inventory_test.go @@ -12670,7 +12670,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())) @@ -12697,7 +12697,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())) diff --git a/internal/controller/controllers/clusterdeployments_controller.go b/internal/controller/controllers/clusterdeployments_controller.go index 0237b343a35..d570007ed09 100644 --- a/internal/controller/controllers/clusterdeployments_controller.go +++ b/internal/controller/controllers/clusterdeployments_controller.go @@ -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) @@ -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) { @@ -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 { diff --git a/internal/controller/controllers/clusterdeployments_controller_test.go b/internal/controller/controllers/clusterdeployments_controller_test.go index 8e501d0b287..fb8423e1b2f 100644 --- a/internal/controller/controllers/clusterdeployments_controller_test.go +++ b/internal/controller/controllers/clusterdeployments_controller_test.go @@ -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().