diff --git a/go.mod b/go.mod index 0abbadbe75d6..753d1aeffcd4 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,7 @@ require ( github.com/openshift/custom-resource-status v1.1.2 github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b298f github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a - github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e + github.com/openshift/image-customization-controller v0.0.0-20240307203510-394809633b6b github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b github.com/patrickmn/go-cache v2.1.0+incompatible github.com/pelletier/go-toml v1.9.5 diff --git a/go.sum b/go.sum index b42b089943c8..0ca52d7214d6 100644 --- a/go.sum +++ b/go.sum @@ -1379,8 +1379,8 @@ github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b2 github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b298f/go.mod h1:/CLsleDcQ6AFTGKJe9VL3Y4rB9DqX3fQwQv47q2/ZJc= github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a h1:E+XPJs/aVvYsrlJzo2ED38ZTR2RTNUlFMmOaFAAdMZg= github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a/go.mod h1:E1bgquRiwfugdArdecPbpYIrAdve5kTzMaJb0+8jMXI= -github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e h1:x1j5gWm4PdTTXTmuX3S7VAmsnazbbMiefQyCLdyjH74= -github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e/go.mod h1:rTvO75VGcFhEuE2HgQe10OijcR9HBDNC+CnKei8p1HE= +github.com/openshift/image-customization-controller v0.0.0-20240307203510-394809633b6b h1:SDdbzE3oW2qs7AJj4DT5gk8HDkZYR8DTT1ZfyBhECZ8= +github.com/openshift/image-customization-controller v0.0.0-20240307203510-394809633b6b/go.mod h1:G9yHOogitMOA0R5UWFmFup00E6nik8Sns/tCYYBU7jE= github.com/openshift/library-go v0.0.0-20191003152030-97c62d8a2901/go.mod h1:NBttNjZpWwup/nthuLbPAPSYC8Qyo+BBK5bCtFoyYjo= github.com/openshift/library-go v0.0.0-20200831114015-2ab0c61c15de/go.mod h1:6vwp+YhYOIlj8MpkQKkebTTSn2TuYyvgiAFQ206jIEQ= github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b h1:BmunS/b02dKSwQ0H21+3EbXDfDr9t8UDlO54WxKkOts= diff --git a/internal/controller/controllers/bmo_utils.go b/internal/controller/controllers/bmo_utils.go index 93aad648e296..2ca5e5f1dc2a 100644 --- a/internal/controller/controllers/bmo_utils.go +++ b/internal/controller/controllers/bmo_utils.go @@ -12,17 +12,32 @@ import ( "github.com/openshift/cluster-baremetal-operator/provisioning" "github.com/pkg/errors" "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client" ) -const MinimalVersionForConvergedFlow = "4.12.0-0.alpha" +const ( + MinimalVersionForConvergedFlow = "4.12.0-0.alpha" + iccNamespace = "openshift-machine-api" + iccSecretName = "metal3-image-customization-config" // #nosec G101 + ironicBaseURLKey = "IRONIC_BASE_URL" + ironicInspectorBaseURLKey = "IRONIC_INSPECTOR_BASE_URL" + ironicAgentImageKey = "IRONIC_AGENT_IMAGE" +) //go:generate mockgen --build_flags=--mod=mod -package=controllers -destination=mock_bmo_utils.go . BMOUtils type BMOUtils interface { ConvergedFlowAvailable() bool GetIronicIPs() ([]string, []string, error) + getICCConfig(ctx context.Context) (*ICCConfig, error) +} + +type ICCConfig struct { + IronicBaseURL string + IronicInspectorBaseUrl string + IronicAgentImage string } type bmoUtils struct { @@ -91,6 +106,32 @@ func (r *bmoUtils) GetIronicIPs() ([]string, []string, error) { return ironicIPs, inspectorIPs, nil } +func (r *bmoUtils) getICCConfig(ctx context.Context) (*ICCConfig, error) { + const configKeyNotFoundError string = "Failed to get '%s' key from secret %s in namespace %s" + + secret := &corev1.Secret{} + namespacedName := types.NamespacedName{Name: iccSecretName, Namespace: iccNamespace} + if err := r.c.Get(ctx, namespacedName, secret); err != nil { + return nil, fmt.Errorf("Failed to get secret %s in namespace %s: %w", iccSecretName, iccNamespace, err) + } + + ironicBaseURL, ok := secret.Data[ironicBaseURLKey] + if !ok { + return nil, fmt.Errorf(configKeyNotFoundError, ironicBaseURLKey, secret.Name, secret.Namespace) + } + + ironicAgentImage, ok := secret.Data[ironicAgentImageKey] + if !ok { + return nil, fmt.Errorf(configKeyNotFoundError, ironicAgentImageKey, secret.Name, secret.Namespace) + } + + return &ICCConfig{ + IronicBaseURL: string(ironicBaseURL), + IronicInspectorBaseUrl: string(secret.Data[ironicInspectorBaseURLKey]), + IronicAgentImage: string(ironicAgentImage), + }, nil +} + func (r *bmoUtils) getProvisioningInfo() (*provisioning.ProvisioningInfo, error) { // Fetch the Provisioning instance instance := &metal3iov1alpha1.Provisioning{} diff --git a/internal/controller/controllers/bmo_utils_test.go b/internal/controller/controllers/bmo_utils_test.go index e6065db4e5b7..0a672a5b8025 100644 --- a/internal/controller/controllers/bmo_utils_test.go +++ b/internal/controller/controllers/bmo_utils_test.go @@ -10,6 +10,7 @@ import ( v1 "github.com/openshift/api/config/v1" "github.com/openshift/assisted-service/internal/common" metal3iov1alpha1 "github.com/openshift/cluster-baremetal-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -122,6 +123,102 @@ var _ = Describe("bmoUtils", func() { }) }) + Context("getICCConfig", func() { + It("success", func() { + bmoUtils := &bmoUtils{ + c: c, + log: log, + kubeAPIEnabled: true, + } + ironicURLs := getUrlFromIP("10.10.10.11") + inspectorURLs := getUrlFromIP("10.10.10.10") + agentImage := "quay.io/some/agent:image" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: iccSecretName, + Namespace: iccNamespace, + }, + Data: map[string][]byte{ + ironicBaseURLKey: []byte(ironicURLs), + ironicInspectorBaseURLKey: []byte(inspectorURLs), + ironicAgentImageKey: []byte(agentImage), + }, + } + Expect(c.Create(context.Background(), secret)).To(BeNil()) + iccConfig, err := bmoUtils.getICCConfig(context.Background()) + Expect(err).Should(BeNil()) + Expect(iccConfig.IronicBaseURL).Should(Equal(ironicURLs)) + Expect(iccConfig.IronicInspectorBaseUrl).Should(Equal(inspectorURLs)) + Expect(iccConfig.IronicAgentImage).Should(Equal(agentImage)) + }) + + It("succeeds when only the inspector url is missing", func() { + bmoUtils := &bmoUtils{ + c: c, + log: log, + kubeAPIEnabled: true, + } + ironicURLs := getUrlFromIP("10.10.10.11") + agentImage := "quay.io/some/agent:image" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: iccSecretName, + Namespace: iccNamespace, + }, + Data: map[string][]byte{ + ironicBaseURLKey: []byte(ironicURLs), + ironicAgentImageKey: []byte(agentImage), + }, + } + Expect(c.Create(context.Background(), secret)).To(BeNil()) + iccConfig, err := bmoUtils.getICCConfig(context.Background()) + Expect(err).Should(BeNil()) + Expect(iccConfig.IronicBaseURL).Should(Equal(ironicURLs)) + Expect(iccConfig.IronicAgentImage).Should(Equal(agentImage)) + }) + + It("throws an error when secret is missing", func() { + bmoUtils := &bmoUtils{ + c: c, + log: log, + kubeAPIEnabled: true, + } + _, err := bmoUtils.getICCConfig(context.Background()) + Expect(err).Should(Not(BeNil())) + }) + + DescribeTable("throws an error when config is incomplete", + func(ironicURLs []byte, inspectorURLs []byte, agentImage []byte) { + bmoUtils := &bmoUtils{ + c: c, + log: log, + kubeAPIEnabled: true, + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: iccSecretName, + Namespace: iccNamespace, + }, + Data: map[string][]byte{}, + } + if ironicURLs != nil { + secret.Data[ironicBaseURLKey] = ironicURLs + } + if inspectorURLs != nil { + secret.Data[ironicInspectorBaseURLKey] = inspectorURLs + } + if agentImage != nil { + secret.Data[ironicAgentImageKey] = agentImage + } + + Expect(c.Create(context.Background(), secret)).To(BeNil()) + _, err := bmoUtils.getICCConfig(context.Background()) + Expect(err).Should(Not(BeNil())) + }, + Entry("ironicURLs is missing", nil, []byte("some"), []byte("some")), + Entry("ironicAgentImage is missing", []byte("some"), []byte("some"), nil), + ) + }) }) func CreateCBO(version string) *v1.ClusterOperator { diff --git a/internal/controller/controllers/mock_bmo_utils.go b/internal/controller/controllers/mock_bmo_utils.go index 7ef3e463bc32..7a992c5423fc 100644 --- a/internal/controller/controllers/mock_bmo_utils.go +++ b/internal/controller/controllers/mock_bmo_utils.go @@ -5,6 +5,7 @@ package controllers import ( + context "context" reflect "reflect" gomock "github.com/golang/mock/gomock" @@ -62,3 +63,18 @@ func (mr *MockBMOUtilsMockRecorder) GetIronicIPs() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIronicIPs", reflect.TypeOf((*MockBMOUtils)(nil).GetIronicIPs)) } + +// getICCConfig mocks base method. +func (m *MockBMOUtils) getICCConfig(arg0 context.Context) (*ICCConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getICCConfig", arg0) + ret0, _ := ret[0].(*ICCConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// getICCConfig indicates an expected call of getICCConfig. +func (mr *MockBMOUtilsMockRecorder) getICCConfig(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getICCConfig", reflect.TypeOf((*MockBMOUtils)(nil).getICCConfig), arg0) +} diff --git a/internal/controller/controllers/preprovisioningimage_controller.go b/internal/controller/controllers/preprovisioningimage_controller.go index cd548980b164..564722af5680 100644 --- a/internal/controller/controllers/preprovisioningimage_controller.go +++ b/internal/controller/controllers/preprovisioningimage_controller.go @@ -126,6 +126,11 @@ func (r *PreprovisioningImageReconciler) Reconcile(origCtx context.Context, req return ctrl.Result{}, nil } + log = log.WithFields(logrus.Fields{ + "infra_env": infraEnv.Name, + "infra_env_namespace": infraEnv.Namespace, + }) + imageArch := common.NormalizeCPUArchitecture(image.Spec.Architecture) infraArch := common.NormalizeCPUArchitecture(infraEnv.Spec.CpuArchitecture) if infraArch != imageArch { @@ -396,6 +401,125 @@ func (r *PreprovisioningImageReconciler) mapInfraEnvPPI() func(ctx context.Conte return mapInfraEnvPPI } +func (r *PreprovisioningImageReconciler) getIronicConfigFromUserOverride(log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv) *ICCConfig { + ironicAgentImage, ok := infraEnv.GetAnnotations()[ironicAgentImageOverrideAnnotation] + if ok && ironicAgentImage != "" { + log.Infof("Using override ironic agent image (%s) from infraEnv", ironicAgentImage) + return &ICCConfig{ + IronicAgentImage: ironicAgentImage, + } + } + + return nil + +} + +func (r *PreprovisioningImageReconciler) getIronicConfigFromBMOConfig(ctx context.Context, log logrus.FieldLogger, infraEnvInternal *common.InfraEnv) *ICCConfig { + iccConfig, err := r.BMOUtils.getICCConfig(ctx) + if err != nil { + log.WithError(err).Info("ICC configuration is not available") + return nil + } + + var architectures []string + architectures, err = r.OcRelease.GetImageArchitecture(log, iccConfig.IronicAgentImage, infraEnvInternal.PullSecret) + if err == nil && funk.Contains(architectures, infraEnvInternal.CPUArchitecture) { + log.Infof("Setting ironic agent image (%s) from ICC config", iccConfig.IronicAgentImage) + return iccConfig + } + + log.Infof("CPU architecture (%v) of Ironic agent image (%s) from ICC config is not available for infraEnv with arch (%s)", + architectures, + iccConfig.IronicAgentImage, + infraEnvInternal.CPUArchitecture) + return nil +} + +func (r *PreprovisioningImageReconciler) getIronicConfigFromHUB(ctx context.Context, log logrus.FieldLogger, infraEnvInternal *common.InfraEnv) *ICCConfig { + image := r.hubIronicAgentImage + architectures := r.hubReleaseArchitectures + if image == "" { + cv := &configv1.ClusterVersion{} + err := r.Get(ctx, types.NamespacedName{Name: "version"}, cv) + if err != nil { + log.WithError(err).Warningf("Failed to get ClusterVersion") + return nil + } + architectures, err = r.OcRelease.GetReleaseArchitecture(log, cv.Status.Desired.Image, r.ReleaseImageMirror, infraEnvInternal.PullSecret) + if err != nil { + log.WithError(err).Warningf("Failed to get release architecture for (%s)", cv.Status.Desired.Image) + return nil + } + image, err = r.OcRelease.GetIronicAgentImage(log, cv.Status.Desired.Image, r.ReleaseImageMirror, infraEnvInternal.PullSecret) + if err != nil { + log.WithError(err).Warningf("Failed to get ironic agent image from release (%s)", cv.Status.Desired.Image) + return nil + } + r.hubIronicAgentImage = image + r.hubReleaseArchitectures = architectures + log.Infof("Caching hub ironic agent image %s for architectures %v", image, architectures) + } + + if !funk.Contains(architectures, infraEnvInternal.CPUArchitecture) { + log.Warningf("Release image architectures %v do not match infraEnv architecture %s", architectures, infraEnvInternal.CPUArchitecture) + return nil + } + + log.Infof("Setting ironic agent image (%s) from HUB cluster", image) + return &ICCConfig{ + IronicAgentImage: image, + } +} + +func (r *PreprovisioningImageReconciler) getIronicDefaultConfig(log logrus.FieldLogger, infraEnvInternal *common.InfraEnv) *ICCConfig { + var ironicAgentImage string + if infraEnvInternal.CPUArchitecture == common.ARM64CPUArchitecture { + ironicAgentImage = r.Config.BaremetalIronicAgentImageForArm + } else { + ironicAgentImage = r.Config.BaremetalIronicAgentImage + } + + log.Infof("Setting default ironic agent image (%s)", ironicAgentImage) + return &ICCConfig{ + IronicAgentImage: ironicAgentImage, + } +} + +func (r *PreprovisioningImageReconciler) getIronicConfig(ctx context.Context, log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv, infraEnvInternal *common.InfraEnv) (*ICCConfig, error) { + var iccConfig *ICCConfig + + iccConfig = r.getIronicConfigFromUserOverride(log, infraEnv) + + if iccConfig == nil { + iccConfig = r.getIronicConfigFromBMOConfig(ctx, log, infraEnvInternal) + } + + if iccConfig == nil { + iccConfig = r.getIronicConfigFromHUB(ctx, log, infraEnvInternal) + } + + if iccConfig == nil { + iccConfig = r.getIronicDefaultConfig(log, infraEnvInternal) + } + + if iccConfig == nil { + return nil, fmt.Errorf("Failed to determine ironic config") + } + + if iccConfig.IronicBaseURL == "" && iccConfig.IronicInspectorBaseUrl == "" { + err := r.fillIronicServiceURLs(ctx, infraEnvInternal, iccConfig) + if err != nil { + return nil, err + } + } + + log.Infof("Ironic Agent Image is (%s) Ironic URL is (%s) Inspector URL is (%s)", + iccConfig.IronicAgentImage, + iccConfig.IronicBaseURL, + iccConfig.IronicInspectorBaseUrl) + return iccConfig, nil +} + // AddIronicAgentToInfraEnv updates the infra-env in the database with the ironic agent ignition config if required // returns true when the infra-env was updated, false otherwise func (r *PreprovisioningImageReconciler) AddIronicAgentToInfraEnv(ctx context.Context, log logrus.FieldLogger, infraEnv *aiv1beta1.InfraEnv) (bool, error) { @@ -409,35 +533,14 @@ func (r *PreprovisioningImageReconciler) AddIronicAgentToInfraEnv(ctx context.Co log.WithError(err).Error("failed to get corresponding infraEnv") return false, err } - ironicAgentImage, ok := infraEnv.GetAnnotations()[ironicAgentImageOverrideAnnotation] - if !ok || ironicAgentImage == "" { - ironicAgentImage, err = r.getIronicAgentImageByRelease(ctx, log, infraEnvInternal) - if err != nil { - log.WithError(err).Warningf("Failed to get ironic agent image by release for infraEnv: %s", infraEnv.Name) - } - } else { - log.Infof("Using override ironic agent image (%s) for infraEnv %s", ironicAgentImage, infraEnv.Name) - } - // if ironicAgentImage can't be found by version use the default - if ironicAgentImage == "" { - if infraEnvInternal.CPUArchitecture == common.ARM64CPUArchitecture { - ironicAgentImage = r.Config.BaremetalIronicAgentImageForArm - } else { - ironicAgentImage = r.Config.BaremetalIronicAgentImage - } - log.Infof("Setting default ironic agent image (%s) for infraEnv %s", ironicAgentImage, infraEnv.Name) - } - - ironicServiceURL, inspectorURL, err := r.getIronicServiceURLs(ctx, infraEnvInternal) + iccConfig, err := r.getIronicConfig(ctx, log, infraEnv, infraEnvInternal) if err != nil { - log.WithError(err).Error("failed to get IronicServiceURLs") + log.WithError(err).Errorf("Failed to get Ironic configuration") return false, err } - r.Log.Infof("Ironic URL is: %s", ironicServiceURL) - r.Log.Infof("Inspector URL is: %s", inspectorURL) - conf, err := ignition.GenerateIronicConfig(ironicServiceURL, inspectorURL, *infraEnvInternal, ironicAgentImage) + conf, err := ignition.GenerateIronicConfig(iccConfig.IronicBaseURL, iccConfig.IronicInspectorBaseUrl, *infraEnvInternal, iccConfig.IronicAgentImage) if err != nil { log.WithError(err).Error("failed to generate Ironic ignition config") return false, err @@ -456,7 +559,7 @@ func (r *PreprovisioningImageReconciler) AddIronicAgentToInfraEnv(ctx context.Co setAnnotation(&infraEnv.ObjectMeta, EnableIronicAgentAnnotation, "true") if err := r.Client.Update(ctx, infraEnv); err != nil { // Just warn here if the update fails as this annotation is just informational - log.WithError(err).Warnf("failed to set %s annotation on infraEnv %s", EnableIronicAgentAnnotation, infraEnv.Name) + log.WithError(err).Warnf("failed to set %s annotation on infraEnv", EnableIronicAgentAnnotation) } } @@ -474,10 +577,10 @@ func (r *PreprovisioningImageReconciler) getIPFamilyForInfraEnv(ctx context.Cont return network.GetConfiguredAddressFamilies(cluster) } -func (r *PreprovisioningImageReconciler) getIronicServiceURLs(ctx context.Context, infraEnv *common.InfraEnv) (string, string, error) { +func (r *PreprovisioningImageReconciler) fillIronicServiceURLs(ctx context.Context, infraEnv *common.InfraEnv, iccConfig *ICCConfig) error { ironicIPs, inspectorIPs, err := r.BMOUtils.GetIronicIPs() if err != nil { - return "", "", err + return err } // default to the first IP returned @@ -495,36 +598,10 @@ func (r *PreprovisioningImageReconciler) getIronicServiceURLs(ctx context.Contex inspectorURL = getUrlFromIP(inspectorIPs[1]) } } - return ironicURL, inspectorURL, nil -} -func (r *PreprovisioningImageReconciler) getIronicAgentImageByRelease(ctx context.Context, log logrus.FieldLogger, infraEnv *common.InfraEnv) (string, error) { - image := r.hubIronicAgentImage - architectures := r.hubReleaseArchitectures - if image == "" { - cv := &configv1.ClusterVersion{} - if err := r.Get(ctx, types.NamespacedName{Name: "version"}, cv); err != nil { - return "", err - } - var err error - architectures, err = r.OcRelease.GetReleaseArchitecture(log, cv.Status.Desired.Image, r.ReleaseImageMirror, infraEnv.PullSecret) - if err != nil { - return "", err - } - image, err = r.OcRelease.GetIronicAgentImage(log, cv.Status.Desired.Image, r.ReleaseImageMirror, infraEnv.PullSecret) - if err != nil { - return "", err - } - r.hubIronicAgentImage = image - r.hubReleaseArchitectures = architectures - log.Infof("Caching hub ironic agent image %s for architectures %v", image, architectures) - } - - if !funk.Contains(architectures, infraEnv.CPUArchitecture) { - return "", fmt.Errorf("release image architectures %v do not match infraEnv architecture %s", architectures, infraEnv.CPUArchitecture) - } - - return image, nil + iccConfig.IronicBaseURL = ironicURL + iccConfig.IronicInspectorBaseUrl = inspectorURL + return nil } func (r *PreprovisioningImageReconciler) setBMHRebootAnnotation(ctx context.Context, image *metal3_v1alpha1.PreprovisioningImage) error { diff --git a/internal/controller/controllers/preprovisioningimage_controller_test.go b/internal/controller/controllers/preprovisioningimage_controller_test.go index deabfd849fb2..ab5244dae8cf 100644 --- a/internal/controller/controllers/preprovisioningimage_controller_test.go +++ b/internal/controller/controllers/preprovisioningimage_controller_test.go @@ -193,6 +193,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }).Return( &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) Expect(res).To(Equal(ctrl.Result{})) @@ -219,6 +220,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(*internalIgnitionConfig).Should(ContainSubstring(url.QueryEscape(ironicInspectorIPs[1]))) }).Return( &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -241,6 +243,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }} Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -269,6 +272,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }} Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -305,6 +309,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -336,6 +341,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }} Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -367,6 +373,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { } Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -394,6 +401,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { SetImageUrl(ppi, *infraEnv) Expect(c.Update(ctx, ppi)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -411,7 +419,80 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(c.Get(ctx, bmhKey, bmh)).To(BeNil()) Expect(bmh.Annotations).NotTo(HaveKey("reboot.metal3.io")) }) + It("Add the ironic Ignition to the infraEnv using the ironic agent image from the ICC configuration", func() { + Expect(c.Create(ctx, clusterVersion)).To(Succeed()) + Expect(c.Create(ctx, infraEnv)).To(BeNil()) + + iccConfig := ICCConfig{ + IronicAgentImage: "ironic-image:4.12.0", + IronicBaseURL: "https://10.0.0.1:6534,https://[2001::1]:6534", + IronicInspectorBaseUrl: "https://10.0.0.2:6534/v1/continue,https://[2001::2]:6534/v1/continue", + } + backendInfraEnv.CPUArchitecture = "x86_64" + backendInfraEnv.PullSecret = "mypullsecret" + mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil) + + mockOcRelease.EXPECT().GetImageArchitecture(gomock.Any(), iccConfig.IronicAgentImage, backendInfraEnv.PullSecret).Return([]string{"x86_64"}, nil) + mockInstallerInternal.EXPECT().UpdateInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()). + Do(func(ctx context.Context, params installer.UpdateInfraEnvParams, internalIgnitionConfig *string) { + Expect(params.InfraEnvID).To(Equal(*backendInfraEnv.ID)) + Expect(params.InfraEnvUpdateParams.IgnitionConfigOverride).To(Equal("")) + Expect(*internalIgnitionConfig).Should(ContainSubstring("ironic")) + Expect(*internalIgnitionConfig).Should(ContainSubstring(iccConfig.IronicAgentImage)) + Expect(*internalIgnitionConfig).Should(ContainSubstring(url.QueryEscape(iccConfig.IronicBaseURL))) + Expect(*internalIgnitionConfig).Should(ContainSubstring(url.QueryEscape(iccConfig.IronicInspectorBaseUrl))) + }).Return( + &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) + mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(&iccConfig, nil) + res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) + Expect(err).To(BeNil()) + Expect(res).To(Equal(ctrl.Result{})) + key := types.NamespacedName{ + Namespace: testNamespace, + Name: "testInfraEnv", + } + Expect(c.Get(ctx, key, infraEnv)).To(BeNil()) + Expect(infraEnv.ObjectMeta.Annotations[EnableIronicAgentAnnotation]).To(Equal("true")) + }) + It("uses the default ironic agent image when the infraenv arch isn't supported by the agent image in the ICC config", func() { + Expect(c.Create(ctx, clusterVersion)).To(Succeed()) + Expect(c.Create(ctx, infraEnv)).To(BeNil()) + + iccConfig := ICCConfig{ + IronicAgentImage: "ironic-image:4.12.0", + IronicBaseURL: "https://10.0.0.1:6534,https://[2001::1]:6534", + IronicInspectorBaseUrl: "https://10.0.0.2:6534/v1/continue,https://[2001::2]:6534/v1/continue", + } + backendInfraEnv.CPUArchitecture = "x86_64" + backendInfraEnv.PullSecret = "mypullsecret" + mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil) + + mockOcRelease.EXPECT().GetImageArchitecture(gomock.Any(), iccConfig.IronicAgentImage, backendInfraEnv.PullSecret).Times(1).Return([]string{"arm64"}, nil) + mockOcRelease.EXPECT().GetReleaseArchitecture(gomock.Any(), hubReleaseImage, "", backendInfraEnv.PullSecret).Times(1).Return([]string{"arm64"}, nil) + mockOcRelease.EXPECT().GetIronicAgentImage(gomock.Any(), hubReleaseImage, "", backendInfraEnv.PullSecret).Return("ironic-image:4.12.0", nil) + mockInstallerInternal.EXPECT().UpdateInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()). + Do(func(ctx context.Context, params installer.UpdateInfraEnvParams, internalIgnitionConfig *string) { + Expect(params.InfraEnvID).To(Equal(*backendInfraEnv.ID)) + Expect(params.InfraEnvUpdateParams.IgnitionConfigOverride).To(Equal("")) + Expect(*internalIgnitionConfig).Should(ContainSubstring("ironic")) + Expect(*internalIgnitionConfig).Should(ContainSubstring(defaultIronicImage)) + }).Return( + &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) + mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(&iccConfig, nil) + res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) + Expect(err).To(BeNil()) + Expect(res).To(Equal(ctrl.Result{})) + + key := types.NamespacedName{ + Namespace: testNamespace, + Name: "testInfraEnv", + } + Expect(c.Get(ctx, key, infraEnv)).To(BeNil()) + Expect(infraEnv.ObjectMeta.Annotations[EnableIronicAgentAnnotation]).To(Equal("true")) + }) It("Add the ironic Ignition to the infraEnv using the ironic agent image from the hub release", func() { Expect(c.Create(ctx, clusterVersion)).To(Succeed()) Expect(c.Create(ctx, infraEnv)).To(BeNil()) @@ -431,6 +512,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }).Return( &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) Expect(res).To(Equal(ctrl.Result{})) @@ -460,6 +542,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }).Return( &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(1) mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(1) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) Expect(res).To(Equal(ctrl.Result{})) @@ -491,6 +574,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { }).Return( &common.InfraEnv{InfraEnv: models.InfraEnv{ClusterID: clusterID, ID: &infraEnvID, DownloadURL: downloadURL, CPUArchitecture: infraEnvArch}, GeneratedAt: strfmt.DateTime(time.Now())}, nil).Times(2) mockCRDEventsHandler.EXPECT().NotifyInfraEnvUpdates(infraEnv.Name, infraEnv.Namespace).Times(2) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(2).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -547,6 +631,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(c.Update(ctx, ppi)).To(Succeed()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -568,6 +653,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { Expect(c.Update(ctx, ppi)).To(Succeed()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -585,6 +671,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { infraEnv.Status = aiv1beta1.InfraEnvStatus{} Expect(c.Create(ctx, infraEnv)).To(BeNil()) setInfraEnvIronicConfig() + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(BeNil()) @@ -638,6 +725,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { backendInfraEnv.ClusterID = "" mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) mockBMOUtils.EXPECT().GetIronicIPs().Return(nil, nil, fmt.Errorf("failed to get urls")) res, err := pr.Reconcile(ctx, newPreprovisioningImageRequest(ppi)) Expect(err).To(HaveOccurred()) @@ -652,6 +740,7 @@ var _ = Describe("PreprovisioningImage reconcile", func() { backendInfraEnv.ClusterID = "" mockInstallerInternal.EXPECT().GetInfraEnvByKubeKey(gomock.Any()).Return(backendInfraEnv, nil) Expect(c.Create(ctx, infraEnv)).To(BeNil()) + mockBMOUtils.EXPECT().getICCConfig(gomock.Any()).Times(1).Return(nil, errors.Errorf("ICC configuration is not available")) mockBMOUtils.EXPECT().GetIronicIPs().AnyTimes().Return(ironicServiceIPs, ironicInspectorIPs, nil) mockInstallerInternal.EXPECT().UpdateInfraEnvInternal(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("Failed to update infraEnvInternal")) diff --git a/internal/oc/mock_release.go b/internal/oc/mock_release.go index c1184ffbbfe2..d930df38c8dc 100644 --- a/internal/oc/mock_release.go +++ b/internal/oc/mock_release.go @@ -49,6 +49,21 @@ func (mr *MockReleaseMockRecorder) Extract(log, releaseImage, releaseImageMirror return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Extract", reflect.TypeOf((*MockRelease)(nil).Extract), log, releaseImage, releaseImageMirror, cacheDir, pullSecret, ocpVersion) } +// GetImageArchitecture mocks base method. +func (m *MockRelease) GetImageArchitecture(log logrus.FieldLogger, image, pullSecret string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetImageArchitecture", log, image, pullSecret) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetImageArchitecture indicates an expected call of GetImageArchitecture. +func (mr *MockReleaseMockRecorder) GetImageArchitecture(log, image, pullSecret interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetImageArchitecture", reflect.TypeOf((*MockRelease)(nil).GetImageArchitecture), log, image, pullSecret) +} + // GetIronicAgentImage mocks base method. func (m *MockRelease) GetIronicAgentImage(log logrus.FieldLogger, releaseImage, releaseImageMirror, pullSecret string) (string, error) { m.ctrl.T.Helper() diff --git a/internal/oc/release.go b/internal/oc/release.go index 926b9f7efc3e..d204755a93fa 100644 --- a/internal/oc/release.go +++ b/internal/oc/release.go @@ -48,6 +48,7 @@ type Release interface { GetOpenshiftVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetMajorMinorVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetReleaseArchitecture(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) ([]string, error) + GetImageArchitecture(log logrus.FieldLogger, image string, pullSecret string) ([]string, error) GetReleaseBinaryPath(releaseImage string, cacheDir string, ocpVersion string) (workdir string, binary string, path string, err error) Extract(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, ocpVersion string) (string, error) } @@ -197,6 +198,10 @@ func (r *release) GetReleaseArchitecture(log logrus.FieldLogger, releaseImage st return nil, errors.New("no releaseImage nor releaseImageMirror provided") } + return r.GetImageArchitecture(log, image, pullSecret) +} + +func (r *release) GetImageArchitecture(log logrus.FieldLogger, image string, pullSecret string) ([]string, error) { icspFile, err := r.getIcspFileFromRegistriesConfig(log) if err != nil { return nil, errors.Wrap(err, "failed to create file ICSP file from registries config") diff --git a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go index 8d69471b8577..9e966e4f1d49 100644 --- a/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go +++ b/vendor/github.com/openshift/image-customization-controller/pkg/ignition/builder.go @@ -37,9 +37,6 @@ func New(nmStateData, registriesConf []byte, ironicBaseURL, ironicInspectorBaseU if ironicBaseURL == "" { return nil, errors.New("ironicBaseURL is required") } - if ironicInspectorBaseURL == "" { - ironicInspectorBaseURL = ironicBaseURL - } if ironicAgentImage == "" { return nil, errors.New("ironicAgentImage is required") } diff --git a/vendor/modules.txt b/vendor/modules.txt index 7e2d5ece95f7..a70e49585662 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -816,8 +816,8 @@ github.com/openshift/hive/apis/hive/v1/openstack github.com/openshift/hive/apis/hive/v1/ovirt github.com/openshift/hive/apis/hive/v1/vsphere github.com/openshift/hive/apis/scheme -# github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e -## explicit; go 1.19 +# github.com/openshift/image-customization-controller v0.0.0-20240307203510-394809633b6b +## explicit; go 1.21 github.com/openshift/image-customization-controller/pkg/ignition # github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b ## explicit; go 1.20