MGMT-14923: add OSImageVersion to InfraEnvSpec (openshift#5365)

Introduced a new property to InfraEnvSpec: 'OSImageVersion'.
The property can be used for creating an InfraEnv through kube-api
with a specific OS image version.

The 'OSImageVersion' is useful for HyperShift flows which
use late binding, hence, the InfraEnv isn't created according
to a cluster version (thus latest OS image is used as a fallback).

Notes:
* The specified version should refer to an OSImage from the list
  in AgentServiceConfig, otherwise the condition is set to failure.
* When a ClusterRef is provided, we still use the cluster's version
  for creating the InfraEnv. Thus, The property should not be
  provided along with OSImageVersion to avoid a conflict
  (validated with a WebHook).

api/v1beta1/infraenv_types.go

@@ -110,6 +110,13 @@ type InfraEnvSpec struct {
 	// certificates in this bundle.
 	// +optional
 	AdditionalTrustBundle string `json:"additionalTrustBundle,omitempty"`
+
+	// OSImageVersion is the version of OS image to use when generating the InfraEnv.
+	// The version should refer to an OSImage specified in the AgentServiceConfig
+	// (i.e. OSImageVersion should equal to an OpenshiftVersion in OSImages list).
+	// Note: OSImageVersion can't be specified along with ClusterRef. 
+	// +optional
+	OSImageVersion string `json:"osImageVersion,omitempty"`
 }
 
 type KernelArgument struct {

config/crd/bases/agent-install.openshift.io_infraenvs.yaml

@@ -170,6 +170,13 @@ spec:
                       are ANDed.
                     type: object
                 type: object
+              osImageVersion:
+                description: 'OSImageVersion is the version of OS image to use when
+                  generating the InfraEnv. The version should refer to an OSImage
+                  specified in the AgentServiceConfig (i.e. OSImageVersion should
+                  equal to an OpenshiftVersion in OSImages list). Note: OSImageVersion
+                  can''t be specified along with ClusterRef.'
+                type: string
               proxy:
                 description: Proxy defines the proxy settings for agents and clusters
                   that use the InfraEnv. If unset, the agents and clusters will not

config/crd/resources.yaml

@@ -2573,6 +2573,13 @@ spec:
                       are ANDed.
                     type: object
                 type: object
+              osImageVersion:
+                description: 'OSImageVersion is the version of OS image to use when
+                  generating the InfraEnv. The version should refer to an OSImage
+                  specified in the AgentServiceConfig (i.e. OSImageVersion should
+                  equal to an OpenshiftVersion in OSImages list). Note: OSImageVersion
+                  can''t be specified along with ClusterRef.'
+                type: string
               proxy:
                 description: Proxy defines the proxy settings for agents and clusters
                   that use the InfraEnv. If unset, the agents and clusters will not

deploy/olm-catalog/manifests/agent-install.openshift.io_infraenvs.yaml

@@ -168,6 +168,13 @@ spec:
                       are ANDed.
                     type: object
                 type: object
+              osImageVersion:
+                description: 'OSImageVersion is the version of OS image to use when
+                  generating the InfraEnv. The version should refer to an OSImage
+                  specified in the AgentServiceConfig (i.e. OSImageVersion should
+                  equal to an OpenshiftVersion in OSImages list). Note: OSImageVersion
+                  can''t be specified along with ClusterRef.'
+                type: string
               proxy:
                 description: Proxy defines the proxy settings for agents and clusters
                   that use the InfraEnv. If unset, the agents and clusters will not

internal/controller/controllers/infraenv_controller.go

@@ -370,13 +370,7 @@ func (r *InfraEnvReconciler) ensureISO(ctx context.Context, log logrus.FieldLogg
 	infraEnvInternal, err := r.Installer.GetInfraEnvByKubeKey(key)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			var clusterID *strfmt.UUID
-			var openshiftVersion string
-			if cluster != nil {
-				clusterID = cluster.ID
-				openshiftVersion = cluster.OpenshiftVersion
-			}
-			infraEnvInternal, err = r.createInfraEnv(ctx, log, &key, infraEnv, clusterID, openshiftVersion, infraEnv.Spec.AdditionalTrustBundle)
+			infraEnvInternal, err = r.createInfraEnv(ctx, log, &key, infraEnv, cluster, infraEnv.Spec.AdditionalTrustBundle)
 			if err != nil {
 				log.Errorf("fail to create InfraEnv: %s, ", infraEnv.Name)
 				return r.handleEnsureISOErrors(ctx, log, infraEnv, err, nil)
@@ -430,16 +424,48 @@ func CreateInfraEnvParams(infraEnv *aiv1beta1.InfraEnv, imageType models.ImageTy
 	return createParams
 }
 
+// Returns OSImageVersion according to the specified property in InfraEnv
+// If there's a cluster reference, return cluster's OpenshiftVersion
+// If OsImageVersion is specified, return value or fallback to latest if missing from ASC
+func (r *InfraEnvReconciler) getOSImageVersion(log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv, cluster *common.Cluster) (string, error) {
+	osImageVersion := infraEnv.Spec.OSImageVersion
+
+	if cluster != nil {
+		return cluster.OpenshiftVersion, nil
+	}
+
+	if osImageVersion != "" {
+		if _, err := r.OsImages.GetOsImage(osImageVersion, infraEnv.Spec.CpuArchitecture); err != nil {
+			msg := "Specified OSImageVersion is missing from AgentServiceConfig"
+			log.WithError(err).Error(msg)
+			return "", common.NewApiError(http.StatusNotFound, errors.New(msg))
+		}
+		return osImageVersion, nil
+	}
+
+	return "", nil
+}
+
 func (r *InfraEnvReconciler) createInfraEnv(ctx context.Context, log logrus.FieldLogger, key *types.NamespacedName,
-	infraEnv *aiv1beta1.InfraEnv, clusterID *strfmt.UUID, openshiftVersion string, additionalTrustBundle string) (*common.InfraEnv, error) {
+	infraEnv *aiv1beta1.InfraEnv, cluster *common.Cluster, additionalTrustBundle string) (*common.InfraEnv, error) {
+
+	osImageVersion, err := r.getOSImageVersion(log, infraEnv, cluster)
+	if err != nil {
+		log.WithError(err).Error("failed to get OS image version")
+		return nil, err
+	}
 
 	pullSecret, err := r.PullSecretHandler.GetValidPullSecret(ctx, getPullSecretKey(infraEnv.Namespace, infraEnv.Spec.PullSecretRef))
 	if err != nil {
 		log.WithError(err).Error("failed to get pull secret")
 		return nil, err
 	}
 
-	createParams := CreateInfraEnvParams(infraEnv, r.Config.ImageType, pullSecret, clusterID, openshiftVersion)
+	var clusterID *strfmt.UUID
+	if cluster != nil {
+		clusterID = cluster.ID
+	}
+	createParams := CreateInfraEnvParams(infraEnv, r.Config.ImageType, pullSecret, clusterID, osImageVersion)
 
 	staticNetworkConfig, err := r.processNMStateConfig(ctx, log, infraEnv)
 	if err != nil {

internal/controller/controllers/infraenv_controller_test.go

@@ -1026,6 +1026,107 @@ var _ = Describe("infraEnv reconcile", func() {
 		Expect(infraEnvImage.Finalizers).ToNot(BeNil())
 	})
 
+	It("InfraEnv is created when doesn't exist in DB - custom OSImageVersion, no cluster ref", func() {
+		key := types.NamespacedName{
+			Namespace: testNamespace,
+			Name:      "infraEnvImage",
+		}
+		osImageVersion := "4.13"
+		infraEnvImage := newInfraEnvImage(key.Name, testNamespace, aiv1beta1.InfraEnvSpec{
+			OSImageVersion: osImageVersion,
+			PullSecretRef:  &corev1.LocalObjectReference{Name: "pull-secret"},
+		})
+		Expect(c.Create(ctx, infraEnvImage)).To(BeNil())
+		mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(nil, gorm.ErrRecordNotFound)
+		mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any()).Return(nil).Times(1)
+		mockInstallerInternal.EXPECT().RegisterInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()).
+			Do(func(ctx context.Context, kubeKey *types.NamespacedName, params installer.RegisterInfraEnvParams) {
+				Expect(params.InfraenvCreateParams.OpenshiftVersion).To(Equal(osImageVersion))
+			}).Return(backendInfraEnv, nil)
+		mockOSImages.EXPECT().GetOsImage(gomock.Any(), gomock.Any()).Return(&models.OsImage{CPUArchitecture: swag.String(infraEnvArch), OpenshiftVersion: swag.String(osImageVersion)}, nil).AnyTimes()
+
+		result, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
+		Expect(err).To(BeNil())
+		Expect(result).To(Equal(ctrl.Result{}))
+	})
+
+	It("failed to create InfraEnv - missing OSImageVersion from AgentServiceConfig", func() {
+		key := types.NamespacedName{
+			Namespace: testNamespace,
+			Name:      "infraEnvImage",
+		}
+		osImageVersion := "4.13"
+		infraEnvImage := newInfraEnvImage(key.Name, testNamespace, aiv1beta1.InfraEnvSpec{
+			OSImageVersion: osImageVersion,
+			PullSecretRef:  &corev1.LocalObjectReference{Name: "pull-secret"},
+		})
+		Expect(c.Create(ctx, infraEnvImage)).To(BeNil())
+		mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(nil, gorm.ErrRecordNotFound)
+		mockOSImages.EXPECT().GetOsImage(gomock.Any(), gomock.Any()).Return(nil, gorm.ErrRecordNotFound).AnyTimes()
+
+		result, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
+		Expect(err).To(BeNil())
+		Expect(result).To(Equal(ctrl.Result{}))
+
+		Expect(c.Get(ctx, key, infraEnvImage)).To(BeNil())
+		Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Message).To(ContainSubstring(aiv1beta1.ImageStateFailedToCreate))
+		Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Reason).To(Equal(aiv1beta1.ImageCreationErrorReason))
+		Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Status).To(Equal(corev1.ConditionFalse))
+	})
+
+	It("InfraEnv is created when doesn't exist in DB - OSImageVersion not specified", func() {
+		key := types.NamespacedName{
+			Namespace: testNamespace,
+			Name:      "infraEnvImage",
+		}
+		infraEnvImage := newInfraEnvImage(key.Name, testNamespace, aiv1beta1.InfraEnvSpec{
+			PullSecretRef: &corev1.LocalObjectReference{Name: "pull-secret"},
+		})
+		Expect(c.Create(ctx, infraEnvImage)).To(BeNil())
+		mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(nil, gorm.ErrRecordNotFound)
+		mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any()).Return(nil).Times(1)
+		mockInstallerInternal.EXPECT().RegisterInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()).
+			Do(func(ctx context.Context, kubeKey *types.NamespacedName, params installer.RegisterInfraEnvParams) {
+				Expect(params.InfraenvCreateParams.OpenshiftVersion).To(Equal(""))
+			}).Return(backendInfraEnv, nil)
+
+		result, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
+		Expect(err).To(BeNil())
+		Expect(result).To(Equal(ctrl.Result{}))
+	})
+
+	It("InfraEnv is created when doesn't exist in DB - ClusterRef is specified", func() {
+		sId = strfmt.UUID(uuid.New().String())
+		backEndCluster = &common.Cluster{Cluster: models.Cluster{
+			ID:               &sId,
+			OpenshiftVersion: ocpVersion,
+		}}
+		mockInstallerInternal.EXPECT().GetClusterByKubeKey(gomock.Any()).Return(backEndCluster, nil)
+		clusterDeployment := newClusterDeployment("clusterDeployment", testNamespace, getDefaultClusterDeploymentSpec("clusterDeployment-test", "test-cluster-aci", "pull-secret"))
+		Expect(c.Create(ctx, clusterDeployment)).To(BeNil())
+
+		key := types.NamespacedName{
+			Namespace: testNamespace,
+			Name:      "infraEnvImage",
+		}
+		infraEnvImage := newInfraEnvImage(key.Name, testNamespace, aiv1beta1.InfraEnvSpec{
+			ClusterRef:    &aiv1beta1.ClusterReference{Name: "clusterDeployment", Namespace: testNamespace},
+			PullSecretRef: &corev1.LocalObjectReference{Name: "pull-secret"},
+		})
+		Expect(c.Create(ctx, infraEnvImage)).To(BeNil())
+
+		mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(nil, gorm.ErrRecordNotFound)
+		mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any()).Return(nil).Times(1)
+		mockInstallerInternal.EXPECT().RegisterInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()).
+			Do(func(ctx context.Context, kubeKey *types.NamespacedName, params installer.RegisterInfraEnvParams) {
+				Expect(params.InfraenvCreateParams.OpenshiftVersion).To(Equal(ocpVersion))
+			}).Return(backendInfraEnv, nil)
+
+		result, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
+		Expect(err).To(BeNil())
+		Expect(result).To(Equal(ctrl.Result{}))
+	})
+
 	Context("with ipxe http route", func() {
 		BeforeEach(func() {
 			ir.InsecureIPXEURLs = false

pkg/webhooks/agentinstall/v1beta1/infraenv_admission_hook.go

@@ -82,6 +82,10 @@ func (a *InfraEnvValidatingAdmissionHook) Validate(admissionSpec *admissionv1.Ad
 
 	contextLogger.Info("Validating request")
 
+	if admissionSpec.Operation == admissionv1.Create {
+		return a.validateCreate(admissionSpec)
+	}
+
 	if admissionSpec.Operation == admissionv1.Update {
 		return a.validateUpdate(admissionSpec)
 	}
@@ -134,6 +138,48 @@ func areClusterRefsEqual(clusterRef1 *v1beta1.ClusterReference, clusterRef2 *v1b
 	}
 }
 
+// validateUpdate specifically validates create operations for InfraEnv objects.
+func (a *InfraEnvValidatingAdmissionHook) validateCreate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
+	contextLogger := log.WithFields(log.Fields{
+		"operation": admissionSpec.Operation,
+		"group":     admissionSpec.Resource.Group,
+		"version":   admissionSpec.Resource.Version,
+		"resource":  admissionSpec.Resource.Resource,
+		"method":    "validateCreate",
+	})
+
+	newObject := &v1beta1.InfraEnv{}
+	if err := a.decoder.DecodeRaw(admissionSpec.Object, newObject); err != nil {
+		contextLogger.Errorf("Failed unmarshaling Object: %v", err.Error())
+		return &admissionv1.AdmissionResponse{
+			Allowed: false,
+			Result: &metav1.Status{
+				Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
+				Message: err.Error(),
+			},
+		}
+	}
+
+	// Ensure that ClusterRef and OSImageVersion are not both specified
+	if newObject.Spec.ClusterRef != nil && newObject.Spec.OSImageVersion != "" {
+		message := "Either Spec.ClusterRef or Spec.OSImageVersion should be specified (not both)."
+		contextLogger.Infof("Failed validation: %v", message)
+		contextLogger.Error(message)
+		return &admissionv1.AdmissionResponse{
+			Allowed: false,
+			Result: &metav1.Status{
+				Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
+				Message: message,
+			},
+		}
+	}
+
+	contextLogger.Info("Successful validation")
+	return &admissionv1.AdmissionResponse{
+		Allowed: true,
+	}
+}
+
 // validateUpdate specifically validates update operations for InfraEnv objects.
 func (a *InfraEnvValidatingAdmissionHook) validateUpdate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
 	contextLogger := log.WithFields(log.Fields{

pkg/webhooks/agentinstall/v1beta1/infraenv_admission_hook_test.go

@@ -204,6 +204,37 @@ var _ = Describe("infraenv web validate", func() {
 			operation:       admissionv1.Update,
 			expectedAllowed: true,
 		},
+		{
+			name: "Test can't specify both Spec.ClusterRef and Spec.OSImageVersion",
+			newSpec: v1beta1.InfraEnvSpec{
+				ClusterRef: &v1beta1.ClusterReference{
+					Name:      "newName",
+					Namespace: "newName",
+				},
+				OSImageVersion: "4.14",
+			},
+			operation:       admissionv1.Create,
+			expectedAllowed: false,
+		},
+		{
+			name: "Test InfraEnv create does not fail when only Spec.ClusterRef is specified",
+			newSpec: v1beta1.InfraEnvSpec{
+				ClusterRef: &v1beta1.ClusterReference{
+					Name:      "newName",
+					Namespace: "newName",
+				},
+			},
+			operation:       admissionv1.Create,
+			expectedAllowed: true,
+		},
+		{
+			name: "Test InfraEnv create does not fail when only Spec.OSImageVersion is specified",
+			newSpec: v1beta1.InfraEnvSpec{
+				OSImageVersion: "4.14",
+			},
+			operation:       admissionv1.Create,
+			expectedAllowed: true,
+		},
 	}
 
 	for i := range cases {