Skip to content

Commit

Permalink
MGMT-18986: Allow ISO type configuration from kube API
Browse files Browse the repository at this point in the history
  • Loading branch information
eliorerz committed Nov 25, 2024
1 parent 52114c7 commit f99bf27
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 21 deletions.
1 change: 1 addition & 0 deletions api/v1beta1/infraenv_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ type InfraEnvSpec struct {
// Supported values include:
// - full-iso: A complete Red Hat CoreOS (RHCOS) ISO, customized with a specific ignition file.
// - minimal-iso: A lightweight ISO that retrieves the remainder of the RHCOS root file system (rootfs) dynamically from the Internet
// +optional
ImageType models.ImageType `json:"imageType,omitempty"`
}

Expand Down
10 changes: 1 addition & 9 deletions internal/bminventory/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -4894,15 +4894,7 @@ func (b *bareMetalInventory) setDefaultRegisterInfraEnvParams(_ context.Context,
params.InfraenvCreateParams.AdditionalNtpSources = &b.Config.DefaultNTPSource
}

// set the default value for REST API case, in case it was not provided in the request
if params.InfraenvCreateParams.ImageType == "" {
if params.InfraenvCreateParams.CPUArchitecture == models.ClusterCPUArchitectureS390x {
b.log.Infof("Found Z architecture, updating ISO image type to %s", models.ImageTypeFullIso)
params.InfraenvCreateParams.ImageType = models.ImageTypeFullIso
} else {
params.InfraenvCreateParams.ImageType = models.ImageType(b.Config.ISOImageType)
}
}
params.InfraenvCreateParams.ImageType = infraenv.GetInfraEnvIsoImageType(b.log, params.InfraenvCreateParams.CPUArchitecture, params.InfraenvCreateParams.ImageType, models.ImageType(b.Config.ISOImageType))

if params.InfraenvCreateParams.CPUArchitecture == "" {
// Specifying architecture in params is optional, fallback to default
Expand Down
25 changes: 15 additions & 10 deletions internal/controller/controllers/infraenv_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/openshift/assisted-service/internal/controller/controllers/mirrorregistry"
"github.com/openshift/assisted-service/internal/gencrypto"
"github.com/openshift/assisted-service/internal/imageservice"
"github.com/openshift/assisted-service/internal/infraenv"
"github.com/openshift/assisted-service/internal/versions"
"github.com/openshift/assisted-service/models"
"github.com/openshift/assisted-service/pkg/auth"
Expand Down Expand Up @@ -146,13 +147,8 @@ func (r *InfraEnvReconciler) Reconcile(origCtx context.Context, req ctrl.Request
return r.ensureISO(ctx, log, infraEnv)
}

func (r *InfraEnvReconciler) getImageType(ctx context.Context, infraEnv *aiv1beta1.InfraEnv) models.ImageType {
// S390X platform does not support minimal ISO, ensure that full ISO is requested instead.
imageType := r.Config.ImageType
if infraEnv.Spec.CpuArchitecture == models.ClusterCPUArchitectureS390x {
imageType = models.ImageTypeFullIso
}
return imageType
func (r *InfraEnvReconciler) getImageType(log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv) models.ImageType {
return infraenv.GetInfraEnvIsoImageType(log, infraEnv.Spec.CpuArchitecture, infraEnv.Spec.ImageType, r.Config.ImageType)
}

func (r *InfraEnvReconciler) updateInfraEnv(ctx context.Context, log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv, internalInfraEnv *common.InfraEnv) (*common.InfraEnv, error) {
Expand All @@ -176,6 +172,9 @@ func (r *InfraEnvReconciler) updateInfraEnv(ctx context.Context, log logrus.Fiel
if len(infraEnv.Spec.AdditionalNTPSources) > 0 {
updateParams.InfraEnvUpdateParams.AdditionalNtpSources = swag.String(strings.Join(infraEnv.Spec.AdditionalNTPSources[:], ","))
}
if infraEnv.Spec.ImageType != common.ImageTypeValue(internalInfraEnv.Type) {
updateParams.InfraEnvUpdateParams.ImageType = infraEnv.Spec.ImageType
}
if infraEnv.Spec.IgnitionConfigOverride != "" {
updateParams.InfraEnvUpdateParams.IgnitionConfigOverride = infraEnv.Spec.IgnitionConfigOverride
}
Expand Down Expand Up @@ -219,8 +218,7 @@ func (r *InfraEnvReconciler) updateInfraEnv(ctx context.Context, log logrus.Fiel
updateParams.InfraEnvUpdateParams.StaticNetworkConfig = []*models.HostStaticNetworkConfig{}
}

updateParams.InfraEnvUpdateParams.ImageType = r.getImageType(ctx, infraEnv)

updateParams.InfraEnvUpdateParams.ImageType = r.getImageType(log, infraEnv)
existingKargs, err := kubeKernelArgs(internalInfraEnv)
if err != nil {
return nil, err
Expand Down Expand Up @@ -502,7 +500,14 @@ func (r *InfraEnvReconciler) createInfraEnv(ctx context.Context, log logrus.Fiel
if cluster != nil {
clusterID = cluster.ID
}
createParams := CreateInfraEnvParams(infraEnv, r.getImageType(ctx, infraEnv), pullSecret, clusterID, osImageVersion)
imageType := r.getImageType(log, infraEnv)
infraEnv.Spec.ImageType = imageType
createParams := CreateInfraEnvParams(infraEnv, imageType, pullSecret, clusterID, osImageVersion)
if err = r.Update(ctx, infraEnv); err != nil {
err = errors.Wrapf(err, "failed to update InfraEnv %s with ImageType %s", infraEnv.Name, imageType)
log.WithError(err)
return nil, err
}

staticNetworkConfig, err := r.processNMStateConfig(ctx, log, infraEnv)
if err != nil {
Expand Down
60 changes: 59 additions & 1 deletion internal/controller/controllers/infraenv_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,9 @@ var _ = Describe("infraEnv reconcile", func() {
ClusterRef: &aiv1beta1.ClusterReference{Name: "clusterDeployment", Namespace: testNamespace},
PullSecretRef: &corev1.LocalObjectReference{Name: "pull-secret"},
})
infraEnvImage.Spec.ImageType = models.ImageTypeFullIso
Expect(c.Create(ctx, infraEnvImage)).To(BeNil())

ir.Config.ImageType = models.ImageTypeFullIso
res, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
Expect(err).To(BeNil())
Expect(res).To(Equal(ctrl.Result{}))
Expand All @@ -257,6 +257,7 @@ var _ = Describe("infraEnv reconcile", func() {
Expect(c.Get(ctx, key, infraEnvImage)).To(BeNil())
Expect(infraEnvImage.Status.ISODownloadURL).To(Equal(downloadURL))
Expect(infraEnvImage.Status.CreatedTime).ToNot(BeNil())
Expect(infraEnvImage.Spec.ImageType).To(Equal(models.ImageTypeFullIso))
Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Message).To(Equal(aiv1beta1.ImageStateCreated))
Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Reason).To(Equal(aiv1beta1.ImageCreatedReason))
Expect(conditionsv1.FindStatusCondition(infraEnvImage.Status.Conditions, aiv1beta1.ImageCreatedCondition).Status).To(Equal(corev1.ConditionTrue))
Expand Down Expand Up @@ -785,6 +786,63 @@ var _ = Describe("infraEnv reconcile", func() {
Expect(infraEnvImage.Status.InfraEnvDebugInfo.EventsURL).To(HavePrefix(eventURL))
})

It("create and update infraenv image type", func() {
clusterDeployment := newClusterDeployment("clusterDeployment", testNamespace, getDefaultClusterDeploymentSpec("clusterDeployment-test", "test-cluster-aci", "pull-secret"))
Expect(c.Create(ctx, clusterDeployment)).To(BeNil())
mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(nil, gorm.ErrRecordNotFound)
//mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil)
mockInstallerInternal.EXPECT().GetClusterByKubeKey(gomock.Any()).Return(backEndCluster, nil)
//mockInstallerInternal.EXPECT().DeregisterInfraEnvInternal(gomock.Any(), gomock.Any()).Return(nil)
mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
backendInfraEnv.Type = common.ImageTypePtr(models.ImageTypeMinimalIso)

mockInstallerInternal.EXPECT().RegisterInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Do(func(ctx context.Context, kubeKey *types.NamespacedName, mirrorRegistryConfiguration *common.MirrorRegistryConfiguration, params installer.RegisterInfraEnvParams) {
Expect(params.InfraenvCreateParams.ImageType).To(Equal(models.ImageTypeMinimalIso))
}).Return(backendInfraEnv, nil)

infraEnvImage := newInfraEnvImage("infraEnvImage", testNamespace, aiv1beta1.InfraEnvSpec{
ClusterRef: &aiv1beta1.ClusterReference{Name: "clusterDeployment", Namespace: testNamespace},
PullSecretRef: &corev1.LocalObjectReference{Name: "pull-secret"},
})
Expect(c.Create(ctx, infraEnvImage)).To(BeNil())

res, err := ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
Expect(err).To(BeNil())
Expect(res).To(Equal(ctrl.Result{}))

key := types.NamespacedName{
Namespace: infraEnvImage.Namespace,
Name: infraEnvImage.Name,
}
Expect(c.Get(ctx, key, infraEnvImage)).To(BeNil())
Expect(infraEnvImage.Spec.ImageType).To(Equal(models.ImageTypeMinimalIso))

//Reconcile and make sure the image type was updated in spec
infraEnvImage.Spec.ImageType = models.ImageTypeFullIso
Expect(c.Update(ctx, infraEnvImage)).To(BeNil())

mockInstallerInternal.EXPECT().GetClusterByKubeKey(gomock.Any()).Return(backEndCluster, nil).AnyTimes()
mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil).AnyTimes()
mockInstallerInternal.EXPECT().GetInfraEnvHostsInternal(gomock.Any(), gomock.Any()).Return([]*common.Host{}, nil).AnyTimes()
mockInstallerInternal.EXPECT().DeregisterInfraEnvInternal(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockInstallerInternal.EXPECT().UpdateInfraEnvInternal(gomock.Any(), gomock.Any(), nil, nil).
Do(func(ctx context.Context, params installer.UpdateInfraEnvParams, internalIgnitionConfig *string, mirrorRegistryConfiguration *common.MirrorRegistryConfiguration) {
Expect(params.InfraEnvID).To(Equal(*backendInfraEnv.ID))
Expect(params.InfraEnvUpdateParams.ImageType).To(Equal(models.ImageTypeFullIso))
}).Return(
&common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: sId, ID: &sId, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1)

// Make sure the image type was updated in spec
res, err = ir.Reconcile(ctx, newInfraEnvRequest(infraEnvImage))
Expect(err).To(BeNil())
Expect(res).To(Equal(ctrl.Result{}))

Expect(c.Get(ctx, key, infraEnvImage)).To(BeNil())
Expect(infraEnvImage.Spec.ImageType).To(Equal(models.ImageTypeFullIso))
})

It("create new image - clusterDeployment not exists", func() {
infraEnvImage := newInfraEnvImage("infraEnvImage", testNamespace, aiv1beta1.InfraEnvSpec{
ClusterRef: &aiv1beta1.ClusterReference{Name: "clusterDeployment", Namespace: testNamespace},
Expand Down
21 changes: 21 additions & 0 deletions internal/infraenv/iso_image_type.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package infraenv

import (
"github.com/openshift/assisted-service/models"
"github.com/sirupsen/logrus"
)

func GetInfraEnvIsoImageType(log logrus.FieldLogger, cpuArchitecture string, paramImageType, defaultImageType models.ImageType) models.ImageType {
// set the default value in case it was not provided in the request

if paramImageType == "" {
if cpuArchitecture == models.ClusterCPUArchitectureS390x {
log.Infof("Found Z architecture, updating ISO image type to %s", models.ImageTypeFullIso)
return models.ImageTypeFullIso
} else {
return defaultImageType
}
}

return paramImageType
}
35 changes: 35 additions & 0 deletions subsystem/kubeapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,41 @@ location = "%s"
}, "1m", "20s").Should(BeTrue())
})

It("Test update infraenv CR image type", func() {
By("Request infraenv")
infraEnvSpec.ClusterRef = nil
infraEnvSpec.CpuArchitecture = models.ClusterCPUArchitectureX8664
deployClusterDeploymentCRD(ctx, kubeClient, clusterDeploymentSpec)
deployAgentClusterInstallCRD(ctx, kubeClient, aciSpec, clusterDeploymentSpec.ClusterInstallRef.Name)
deployInfraEnvCRD(ctx, kubeClient, infraNsName.Name, infraEnvSpec)

By("Verify default image type requested on creation of infra-env")
infraEnvKey := types.NamespacedName{
Namespace: Options.Namespace,
Name: infraNsName.Name,
}
var infraEnv *common.InfraEnv
Eventually(func() bool {
infraEnv = getInfraEnvFromDBByKubeKey(ctx, db, infraEnvKey, waitForReconcileTimeout)
infraEnvCrd := getInfraEnvCRD(ctx, kubeClient, infraEnvKey)
return *infraEnv.InfraEnv.Type == models.ImageTypeMinimalIso && models.ImageTypeMinimalIso == infraEnvCrd.Spec.ImageType
}, "1m", "20s").Should(BeTrue())

By("Set infra-env to full-iso to test update")
infraEnvCR := getInfraEnvCRD(ctx, kubeClient, infraEnvKey)
infraEnvCR.Spec.ImageType = models.ImageTypeFullIso
err := kubeClient.Update(ctx, infraEnvCR)
Expect(err).ToNot(HaveOccurred())

Eventually(func() bool {
infraEnv = getInfraEnvFromDBByKubeKey(ctx, db, types.NamespacedName{
Namespace: Options.Namespace,
Name: infraNsName.Name,
}, waitForReconcileTimeout)
return *infraEnv.InfraEnv.Type == models.ImageTypeFullIso
}, "1m", "20s").Should(BeTrue())
})

It("Pull Secret validation error", func() {
By("setting pull secret with wrong data")
updateSecret(ctx, kubeClient, pullSecretName, map[string]string{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f99bf27

Please sign in to comment.