Repository: openshift/assisted-service (language: python)

Issue:
ACM-14502: Allow updates to InfraEnv version
Allow updating the InfraEnv OpenshiftVersion in both the REST and kube APIs.

This is useful when after initial install a user wants to add a new node to a cluster, but the cluster has been upgraded between install and the scale out. Previously a user would need to create a new infraenv with identical settings to the old one and a newer version.

With this change users will be able to update the infraenv os image version just like other attributes.

For some background:

The field was added to the InfraEnv CR in https://github.com/openshift/assisted-service/pull/5365 for https://issues.redhat.com//browse/MGMT-14923 and I don't see any discussion there about updates.

Based on the PR that added infraenv update (https://github.com/openshift/assisted-service/pull/2319) the update params were originally created without the version with no discussion I could find.

Based on these it doesn't seem like this was an intentional omission, so it should be safe to add it here.

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [x] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [ ] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [ ] Automation (CI, tools, etc)
- [x] Cloud
- [x] Operator Managed Deployments
- [ ] None

## How was this code tested?

Deployed assisted service kube-api, created an infraenv, updated `osImageVersion` several times.
Observed the URL change accordingly each time and an error in the infraenv status when a non-existent version was requested.

Additionally subsystem tests were added to cover this.

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [x] dev-scripts environment
- [ ] Reviewer's test appreciated
- [x] Waiting for CI to do a full test run
- [x] Manual (Elaborate on how it was tested)
- [ ] No tests needed

## Checklist

- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [x] This change does not require a documentation update (docstring, `docs`, README, etc)
- [x] Does this change include unit-tests (note that code changes require unit-tests)

## Reviewers Checklist

- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?



Discussion / hints:

Fix patch:
diff --git a/api/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go b/api/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
index fa7c2188547..a80b0fbb3a9 100644
--- a/api/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
+++ b/api/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
@@ -36,6 +36,9 @@ type InfraEnvUpdateParams struct {
 	// kernel arguments
 	KernelArguments KernelArguments `json:"kernel_arguments"`
 
+	// Version of the OS image
+	OpenshiftVersion *string `json:"openshift_version,omitempty"`
+
 	// proxy
 	Proxy *Proxy `json:"proxy,omitempty" gorm:"embedded;embeddedPrefix:proxy_"`
 
diff --git a/client/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go b/client/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
index fa7c2188547..a80b0fbb3a9 100644
--- a/client/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
+++ b/client/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
@@ -36,6 +36,9 @@ type InfraEnvUpdateParams struct {
 	// kernel arguments
 	KernelArguments KernelArguments `json:"kernel_arguments"`
 
+	// Version of the OS image
+	OpenshiftVersion *string `json:"openshift_version,omitempty"`
+
 	// proxy
 	Proxy *Proxy `json:"proxy,omitempty" gorm:"embedded;embeddedPrefix:proxy_"`
 
diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go
index 3f2f0c39082..e072e379015 100644
--- a/internal/bminventory/inventory.go
+++ b/internal/bminventory/inventory.go
@@ -4272,12 +4272,12 @@ func validateProxySettings(httpProxy, httpsProxy, noProxy, ocpVersion *string) e
 	return nil
 }
 
-// validateArchitectureAndVersion validates if architecture specified inside Infraenv matches one
+// validateClusterArchitectureAndVersion validates if architecture specified inside Infraenv matches one
 // specified for the cluster. For single-arch clusters the validation needs to only compare values
 // of the params. For multiarch cluster we want to see if the multiarch release image contains the
 // the architecture specifically requested by the InfraEnv. We don't need to explicitly validate if
 // the OS image exists because if not, this will be detected by the function generating the ISO.
-func validateArchitectureAndVersion(v versions.Handler, c *common.Cluster, cpuArch, ocpVersion string) error {
+func validateClusterArchitectureAndVersion(v versions.Handler, c *common.Cluster, cpuArch, ocpVersion string) error {
 	// For late-binding we don't know the cluster yet
 	if c == nil {
 		return nil
@@ -4715,7 +4715,7 @@ func (b *bareMetalInventory) RegisterInfraEnvInternal(
 func (b *bareMetalInventory) validateInfraEnvCreateParams(ctx context.Context, params installer.RegisterInfraEnvParams, cluster *common.Cluster) error {
 	var err error
 
-	if err = validateArchitectureAndVersion(b.versionsHandler, cluster, params.InfraenvCreateParams.CPUArchitecture, params.InfraenvCreateParams.OpenshiftVersion); err != nil {
+	if err = validateClusterArchitectureAndVersion(b.versionsHandler, cluster, params.InfraenvCreateParams.CPUArchitecture, params.InfraenvCreateParams.OpenshiftVersion); err != nil {
 		return err
 	}
 
@@ -4940,6 +4940,16 @@ func (b *bareMetalInventory) UpdateInfraEnvInternal(ctx context.Context, params
 			return common.NewApiError(http.StatusBadRequest, err)
 		}
 
+		openshiftVersion := infraEnv.OpenshiftVersion
+		if params.InfraEnvUpdateParams.OpenshiftVersion != nil {
+			openshiftVersion = *params.InfraEnvUpdateParams.OpenshiftVersion
+		}
+
+		_, err = b.osImages.GetOsImageOrLatest(openshiftVersion, infraEnv.CPUArchitecture)
+		if err != nil {
+			return common.NewApiError(http.StatusBadRequest, err)
+		}
+
 		var cluster *common.Cluster
 		clusterId := infraEnv.ClusterID
 		if clusterId != "" {
@@ -4951,7 +4961,7 @@ func (b *bareMetalInventory) UpdateInfraEnvInternal(ctx context.Context, params
 				cluster = nil
 			}
 		}
-		if err = validateArchitectureAndVersion(b.versionsHandler, cluster, infraEnv.CPUArchitecture, infraEnv.OpenshiftVersion); err != nil {
+		if err = validateClusterArchitectureAndVersion(b.versionsHandler, cluster, infraEnv.CPUArchitecture, openshiftVersion); err != nil {
 			return err
 		}
 		if err = featuresupport.ValidateIncompatibleFeatures(log, infraEnv.CPUArchitecture, cluster, &infraEnv.InfraEnv, params.InfraEnvUpdateParams); err != nil {
@@ -5017,33 +5027,12 @@ func (b *bareMetalInventory) validateDiscoveryIgnitionImageSize(ctx context.Cont
 
 func (b *bareMetalInventory) updateInfraEnvData(infraEnv *common.InfraEnv, params installer.UpdateInfraEnvParams, internalIgnitionConfig *string, db *gorm.DB, log logrus.FieldLogger) error {
 	updates := map[string]interface{}{}
-	if params.InfraEnvUpdateParams.Proxy != nil {
-		proxyHash, err := computeProxyHash(params.InfraEnvUpdateParams.Proxy)
-		if err != nil {
-			return err
-		}
-		if proxyHash != infraEnv.ProxyHash {
-			optionalParam(params.InfraEnvUpdateParams.Proxy.HTTPProxy, "proxy_http_proxy", updates)
-			optionalParam(params.InfraEnvUpdateParams.Proxy.HTTPSProxy, "proxy_https_proxy", updates)
-			optionalParam(params.InfraEnvUpdateParams.Proxy.NoProxy, "proxy_no_proxy", updates)
-			updates["proxy_hash"] = proxyHash
-		}
+	if err := b.updateInfraEnvProxy(params, infraEnv, updates); err != nil {
+		return err
 	}
-	if params.InfraEnvUpdateParams.KernelArguments != nil {
-		if len(params.InfraEnvUpdateParams.KernelArguments) > 0 {
-			b, err := json.Marshal(&params.InfraEnvUpdateParams.KernelArguments)
-			if err != nil {
-				return common.NewApiError(http.StatusBadRequest, errors.Wrap(err, "failed to format kernel arguments as json"))
-			}
-			updates["kernel_arguments"] = string(b)
-		} else {
-			updates["kernel_arguments"] = gorm.Expr("NULL")
-		}
-		if infraEnv.ClusterID != "" {
-			if err := b.setDiscoveryKernelArgumentsUsage(db, infraEnv.ClusterID, params.InfraEnvUpdateParams.KernelArguments); err != nil {
-				log.WithError(err).Warnf("failed to set discovery kernel arguments usage for cluster %s", infraEnv.ClusterID)
-			}
-		}
+
+	if err := b.updateInfraEnvKernelArguments(params, infraEnv, updates, log, db); err != nil {
+		return err
 	}
 
 	inputSSHKey := swag.StringValue(params.InfraEnvUpdateParams.SSHAuthorizedKey)
@@ -5051,6 +5040,11 @@ func (b *bareMetalInventory) updateInfraEnvData(infraEnv *common.InfraEnv, param
 		updates["ssh_authorized_key"] = inputSSHKey
 	}
 
+	inputVersion := swag.StringValue(params.InfraEnvUpdateParams.OpenshiftVersion)
+	if inputVersion != "" && inputVersion != infraEnv.OpenshiftVersion {
+		updates["openshift_version"] = inputVersion
+	}
+
 	if err := b.updateInfraEnvNtpSources(params, infraEnv, updates, log); err != nil {
 		return err
 	}
@@ -5188,6 +5182,49 @@ func (b *bareMetalInventory) updateInfraEnvNtpSources(params installer.UpdateInf
 	return nil
 }
 
+func (b *bareMetalInventory) updateInfraEnvProxy(params installer.UpdateInfraEnvParams, infraEnv *common.InfraEnv, updates map[string]interface{}) error {
+	if params.InfraEnvUpdateParams.Proxy == nil {
+		return nil
+	}
+
+	proxyHash, err := computeProxyHash(params.InfraEnvUpdateParams.Proxy)
+	if err != nil {
+		return err
+	}
+
+	if proxyHash != infraEnv.ProxyHash {
+		optionalParam(params.InfraEnvUpdateParams.Proxy.HTTPProxy, "proxy_http_proxy", updates)
+		optionalParam(params.InfraEnvUpdateParams.Proxy.HTTPSProxy, "proxy_https_proxy", updates)
+		optionalParam(params.InfraEnvUpdateParams.Proxy.NoProxy, "proxy_no_proxy", updates)
+		updates["proxy_hash"] = proxyHash
+	}
+
+	return nil
+}
+
+func (b *bareMetalInventory) updateInfraEnvKernelArguments(params installer.UpdateInfraEnvParams, infraEnv *common.InfraEnv, updates map[string]interface{}, log logrus.FieldLogger, db *gorm.DB) error {
+	if params.InfraEnvUpdateParams.KernelArguments == nil {
+		return nil
+	}
+
+	if len(params.InfraEnvUpdateParams.KernelArguments) > 0 {
+		b, err := json.Marshal(&params.InfraEnvUpdateParams.KernelArguments)
+		if err != nil {
+			return common.NewApiError(http.StatusBadRequest, errors.Wrap(err, "failed to format kernel arguments as json"))
+		}
+		updates["kernel_arguments"] = string(b)
+	} else {
+		updates["kernel_arguments"] = gorm.Expr("NULL")
+	}
+	if infraEnv.ClusterID != "" {
+		if err := b.setDiscoveryKernelArgumentsUsage(db, infraEnv.ClusterID, params.InfraEnvUpdateParams.KernelArguments); err != nil {
+			log.WithError(err).Warnf("failed to set discovery kernel arguments usage for cluster %s", infraEnv.ClusterID)
+		}
+	}
+
+	return nil
+}
+
 func (b *bareMetalInventory) GetInfraEnvByKubeKey(key types.NamespacedName) (*common.InfraEnv, error) {
 	infraEnv, err := common.GetInfraEnvFromDBWhere(b.db, "name = ? and kube_key_namespace = ?", key.Name, key.Namespace)
 	if err != nil {
diff --git a/internal/controller/controllers/infraenv_controller.go b/internal/controller/controllers/infraenv_controller.go
index 9e805e96f60..cffd0834e9a 100644
--- a/internal/controller/controllers/infraenv_controller.go
+++ b/internal/controller/controllers/infraenv_controller.go
@@ -179,6 +179,9 @@ func (r *InfraEnvReconciler) updateInfraEnv(ctx context.Context, log logrus.Fiel
 	if strings.TrimSpace(infraEnv.Spec.AdditionalTrustBundle) != internalInfraEnv.AdditionalTrustBundle {
 		updateParams.InfraEnvUpdateParams.AdditionalTrustBundle = &infraEnv.Spec.AdditionalTrustBundle
 	}
+	if infraEnv.Spec.OSImageVersion != "" && infraEnv.Spec.OSImageVersion != internalInfraEnv.OpenshiftVersion {
+		updateParams.InfraEnvUpdateParams.OpenshiftVersion = &infraEnv.Spec.OSImageVersion
+	}
 
 	pullSecretKey := getPullSecretKey(infraEnv.Namespace, infraEnv.Spec.PullSecretRef)
 	pullSecret, err := r.PullSecretHandler.GetValidPullSecret(ctx, pullSecretKey)
diff --git a/models/infra_env_update_params.go b/models/infra_env_update_params.go
index fa7c2188547..a80b0fbb3a9 100644
--- a/models/infra_env_update_params.go
+++ b/models/infra_env_update_params.go
@@ -36,6 +36,9 @@ type InfraEnvUpdateParams struct {
 	// kernel arguments
 	KernelArguments KernelArguments `json:"kernel_arguments"`
 
+	// Version of the OS image
+	OpenshiftVersion *string `json:"openshift_version,omitempty"`
+
 	// proxy
 	Proxy *Proxy `json:"proxy,omitempty" gorm:"embedded;embeddedPrefix:proxy_"`
 
diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go
index a9c4bede28c..0b922d21ea4 100644
--- a/restapi/embedded_spec.go
+++ b/restapi/embedded_spec.go
@@ -8759,6 +8759,11 @@ func init() {
         "kernel_arguments": {
           "$ref": "#/definitions/kernel_arguments"
         },
+        "openshift_version": {
+          "description": "Version of the OS image",
+          "type": "string",
+          "x-nullable": true
+        },
         "proxy": {
           "$ref": "#/definitions/proxy"
         },
@@ -19386,6 +19391,11 @@ func init() {
         "kernel_arguments": {
           "$ref": "#/definitions/kernel_arguments"
         },
+        "openshift_version": {
+          "description": "Version of the OS image",
+          "type": "string",
+          "x-nullable": true
+        },
         "proxy": {
           "$ref": "#/definitions/proxy"
         },
diff --git a/swagger.yaml b/swagger.yaml
index 13560c65af2..e6b3048cecd 100644
--- a/swagger.yaml
+++ b/swagger.yaml
@@ -7322,6 +7322,10 @@ definitions:
         description: Allows users to change the additional_trust_bundle infra-env field
         x-nullable: true
         maxLength: 65535
+      openshift_version:
+        type: string
+        description: Version of the OS image
+        x-nullable: true
 
   ip:
     type: string
diff --git a/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go b/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
index fa7c2188547..a80b0fbb3a9 100644
--- a/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
+++ b/vendor/github.com/openshift/assisted-service/models/infra_env_update_params.go
@@ -36,6 +36,9 @@ type InfraEnvUpdateParams struct {
 	// kernel arguments
 	KernelArguments KernelArguments `json:"kernel_arguments"`
 
+	// Version of the OS image
+	OpenshiftVersion *string `json:"openshift_version,omitempty"`
+
 	// proxy
 	Proxy *Proxy `json:"proxy,omitempty" gorm:"embedded;embeddedPrefix:proxy_"`
 


Test patch:
diff --git a/internal/bminventory/inventory_test.go b/internal/bminventory/inventory_test.go
index d4af91f04f5..409565b658e 100644
--- a/internal/bminventory/inventory_test.go
+++ b/internal/bminventory/inventory_test.go
@@ -8909,6 +8909,22 @@ var _ = Describe("infraEnvs", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(i.IgnitionConfigOverride).To(Equal(override))
 			})
+			It("Update OpenshiftVersion", func() {
+				mockInfraEnvUpdateSuccess()
+				updateOCPVersion := "4.18"
+				Expect(i.OpenshiftVersion).ToNot(Equal(updateOCPVersion))
+				reply := bm.UpdateInfraEnv(ctx, installer.UpdateInfraEnvParams{
+					InfraEnvID: *i.ID,
+					InfraEnvUpdateParams: &models.InfraEnvUpdateParams{
+						OpenshiftVersion: swag.String(updateOCPVersion),
+					},
+				})
+				Expect(reply).To(BeAssignableToTypeOf(installer.NewUpdateInfraEnvCreated()))
+				var err error
+				i, err = bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: *i.ID})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(i.OpenshiftVersion).To(HaveValue(Equal(updateOCPVersion)))
+			})
 			It("Update Image type", func() {
 				var err error
 				mockInfraEnvUpdateSuccess()
@@ -9415,7 +9431,7 @@ var _ = Describe("infraEnvs", func() {
 				i, err := bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: infraEnvID})
 				Expect(err).ToNot(HaveOccurred())
 
-				mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil)
+				mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 				mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil).Times(1)
 				mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
 					eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -9460,7 +9476,7 @@ var _ = Describe("infraEnvs", func() {
 				err := db.Create(boundedInfraEnv).Error
 				Expect(err).ToNot(HaveOccurred())
 
-				mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil)
+				mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 				mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil).Times(1)
 				mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
 					eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -9500,7 +9516,7 @@ var _ = Describe("infraEnvs", func() {
 				It("sets a valid image_token", func() {
 					i, err := bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: infraEnvID})
 					Expect(err).ToNot(HaveOccurred())
-					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil)
+					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 					mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil)
 					mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
 						eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -9528,7 +9544,7 @@ var _ = Describe("infraEnvs", func() {
 				It("updates the infra-env expires_at time", func() {
 					i, err := bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: infraEnvID})
 					Expect(err).ToNot(HaveOccurred())
-					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil)
+					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 					mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil)
 					mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
 						eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -9581,7 +9597,7 @@ var _ = Describe("infraEnvs", func() {
 				It("does not update the image service url if nothing changed", func() {
 					i, err := bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: infraEnvID})
 					Expect(err).ToNot(HaveOccurred())
-					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).Times(2)
+					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 					mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil).Times(1)
 					mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
 						eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -9597,7 +9613,7 @@ var _ = Describe("infraEnvs", func() {
 				It("updates the image service url when things change", func() {
 					i, err := bm.GetInfraEnvInternal(ctx, installer.GetInfraEnvParams{InfraEnvID: infraEnvID})
 					Expect(err).ToNot(HaveOccurred())
-					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).Times(7)
+					mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, "").Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 					mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, true, bm.authHandler.AuthType(), gomock.Any()).Return("ignitionconfigforlogging", nil).Times(7)
 					mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), bm.IgnitionConfig, false, bm.authHandler.AuthType(), gomock.Any()).Return(discovery_ignition_3_1, nil).AnyTimes()
 					mockEvents.EXPECT().SendInfraEnvEvent(ctx, eventstest.NewEventMatcher(
@@ -9732,6 +9748,7 @@ var _ = Describe("infraEnvs", func() {
 
 				It("Invalid NTP source", func() {
 					ntpSource := "inject'"
+					mockOSImages.EXPECT().GetOsImageOrLatest(gomock.Any(), gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 					reply := bm.UpdateInfraEnv(ctx, installer.UpdateInfraEnvParams{
 						InfraEnvID: infraEnvID,
 						InfraEnvUpdateParams: &models.InfraEnvUpdateParams{
@@ -11909,7 +11926,7 @@ var _ = Describe("V2DownloadInfraEnvFiles", func() {
 			"p3=\"this is an argument\"",
 		}
 		initialKernelArguments := `random.trust_cpu=on rd.luks.options=discard ignition.firstboot ignition.platform.id=metal console=tty1 console=ttyS1,115200n8 coreos.inst.persistent-kargs="console=tty1 console=ttyS1,115200n8"`
-		mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).Times(3)
+		mockOSImages.EXPECT().GetOsImageOrLatest(common.TestDefaultConfig.OpenShiftVersion, gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 		content := getResponseData("ipxe-script", false, nil, "")
 		lines := strings.Split(string(content), "\n")
 
@@ -12223,7 +12240,7 @@ var _ = Describe("UpdateInfraEnv - Ignition", func() {
 			InfraEnvUpdateParams: &models.InfraEnvUpdateParams{IgnitionConfigOverride: override},
 		}
 		mockUsageReports()
-		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).Times(1)
+		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 		mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(discovery_ignition_3_1, nil).AnyTimes()
 		mockEvents.EXPECT().SendInfraEnvEvent(gomock.Any(), eventstest.NewEventMatcher(
 			eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -12297,7 +12314,7 @@ var _ = Describe("UpdateInfraEnv - Ignition", func() {
 			InfraEnvID:           *infraEnv.ID,
 			InfraEnvUpdateParams: &models.InfraEnvUpdateParams{IgnitionConfigOverride: override},
 		}
-		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).Times(1)
+		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 		mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(discovery_ignition_3_1, nil).AnyTimes()
 		mockEvents.EXPECT().SendInfraEnvEvent(gomock.Any(), eventstest.NewEventMatcher(
 			eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -12330,7 +12347,7 @@ var _ = Describe("UpdateInfraEnv - Ignition", func() {
 			InfraEnvID:           *infraEnv.ID,
 			InfraEnvUpdateParams: &models.InfraEnvUpdateParams{IgnitionConfigOverride: override},
 		}
-		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).Times(1)
+		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 		mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(discovery_ignition_3_1, nil).Times(1)
 		mockEvents.EXPECT().SendInfraEnvEvent(gomock.Any(), eventstest.NewEventMatcher(
 			eventstest.WithNameMatcher(eventgen.ImageInfoUpdatedEventName),
@@ -12351,6 +12368,7 @@ var _ = Describe("UpdateInfraEnv - Ignition", func() {
 		largeDiscoveryIgnition := fmt.Sprintf(`{"ignition": {"version": "3.1.0"}, "storage": {"files": [{"path": "/tmp/example", "contents": {"source": "data:text/plain;base64,%s"}}]}}`, content)
 		override := `{"ignition": {"version": "3.1.0"}, "storage": {"files": [{"path": "/tmp/example", "contents": {"source": "data:text/plain;base64,aGVscGltdHJhcHBlZGluYXN3YWdnZXJzcGVj"}}]}}`
 
+		mockOSImages.EXPECT().GetOsImageOrLatest("", gomock.Any()).Return(common.TestDefaultConfig.OsImage, nil).AnyTimes()
 		mockIgnitionBuilder.EXPECT().FormatDiscoveryIgnitionFile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(largeDiscoveryIgnition, nil).AnyTimes()
 		mockUsage.EXPECT().Add(gomock.Any(), usage.IgnitionConfigOverrideUsage, gomock.Any()).Times(1)
 		mockUsage.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).Times(1)
diff --git a/subsystem/infra_env_test.go b/subsystem/infra_env_test.go
index 412c35248d7..e0659d2556a 100644
--- a/subsystem/infra_env_test.go
+++ b/subsystem/infra_env_test.go
@@ -261,6 +261,7 @@ var _ = Describe("Infra_Env", func() {
 				IgnitionConfigOverride: `{"ignition": {"version": "3.1.0"}, "storage": {"files": [{"path": "/tmp/example", "contents": {"source": "data:text/plain;base64,aGVscGltdHJhcHBlZGluYXN3YWdnZXJzcGVj"}}]}}`,
 				SSHAuthorizedKey:       swag.String(newSshKey),
 				Proxy:                  &models.Proxy{HTTPProxy: swag.String("http://proxy.proxy"), HTTPSProxy: nil, NoProxy: swag.String("proxy.proxy")},
+				OpenshiftVersion:       swag.String("4.16"),
 			},
 		}
 
@@ -272,6 +273,7 @@ var _ = Describe("Infra_Env", func() {
 		Expect(swag.StringValue(updateInfraEnv.Proxy.HTTPSProxy)).To(BeEmpty())
 		Expect(swag.StringValue(updateInfraEnv.Proxy.NoProxy)).To(Equal("proxy.proxy"))
 		Expect(common.ImageTypeValue(updateInfraEnv.Type)).To(Equal(models.ImageTypeMinimalIso))
+		Expect(updateInfraEnv.OpenshiftVersion).To(Equal("4.16"))
 	})
 
 	It("download minimal-iso image success", func() {
@@ -505,4 +507,15 @@ var _ = Describe("Infra_Env", func() {
 		_, err := userBMClient.Installer.UpdateInfraEnv(ctx, updateParams)
 		Expect(err).To(HaveOccurred())
 	})
+
+	It("fails when given invalid version", func() {
+		updateParams := &installer.UpdateInfraEnvParams{
+			InfraEnvID: infraEnvID,
+			InfraEnvUpdateParams: &models.InfraEnvUpdateParams{
+				OpenshiftVersion: swag.String("5.99"),
+			},
+		}
+		_, err := userBMClient.Installer.UpdateInfraEnv(ctx, updateParams)
+		Expect(err).To(HaveOccurred())
+	})
 })
diff --git a/subsystem/kubeapi_test.go b/subsystem/kubeapi_test.go
index cb22efc57e4..29d92bc79cb 100644
--- a/subsystem/kubeapi_test.go
+++ b/subsystem/kubeapi_test.go
@@ -5176,6 +5176,33 @@ spec:
 			hiveext.ClusterSpecSyncedCondition)
 		Expect(condition.Message).To(ContainSubstring("User Managed Networking cannot be set with API VIP"))
 	})
+
+	It("updating infraenv OSImageVersion updates the image URL", func() {
+		infraEnvSpec.ClusterRef = nil
+		infraEnvSpec.OSImageVersion = "4.14"
+		deployInfraEnvCRD(ctx, kubeClient, infraNsName.Name, infraEnvSpec)
+
+		var url string
+		Eventually(func() string {
+			url = getInfraEnvCRD(ctx, kubeClient, infraNsName).Status.ISODownloadURL
+			return url
+		}, "15s", "1s").Should(Not(BeEmpty()))
+
+		Eventually(func() error {
+			infraEnv := getInfraEnvCRD(ctx, kubeClient, infraNsName)
+			infraEnv.Spec.OSImageVersion = "4.16"
+			return kubeClient.Update(ctx, infraEnv)
+		}, "1m", "20s").Should(BeNil())
+
+		var newURL string
+		Eventually(func() string {
+			newURL = getInfraEnvCRD(ctx, kubeClient, infraNsName).Status.ISODownloadURL
+			return newURL
+		}, "15s", "1s").Should(Not(Equal(url)))
+
+		updatedInfraEnv := getInfraEnvFromDBByKubeKey(ctx, db, infraNsName, waitForReconcileTimeout)
+		Expect(updatedInfraEnv.OpenshiftVersion).To(Equal("4.16"))
+	})
 })
 
 var _ = Describe("bmac reconcile flow", func() {