From 471a673271376689740e1f52c5d38b9a070101d5 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 12 Jul 2021 10:37:19 -0400 Subject: [PATCH 1/2] Add AcceptFormats field to PreprovisioningImage CRD --- apis/metal3.io/v1alpha1/preprovisioningimage_types.go | 4 ++++ apis/metal3.io/v1alpha1/zz_generated.deepcopy.go | 7 ++++++- config/crd/bases/metal3.io_preprovisioningimages.yaml | 9 +++++++++ config/render/capm3.yaml | 9 +++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apis/metal3.io/v1alpha1/preprovisioningimage_types.go b/apis/metal3.io/v1alpha1/preprovisioningimage_types.go index 84697a94e4..4b812b8bf6 100644 --- a/apis/metal3.io/v1alpha1/preprovisioningimage_types.go +++ b/apis/metal3.io/v1alpha1/preprovisioningimage_types.go @@ -37,6 +37,10 @@ type PreprovisioningImageSpec struct { // architecture is the processor architecture for which to build the image. // +optional Architecture string `json:"architecture,omitempty"` + + // acceptFormats is a list of acceptable image formats. + // +optional + AcceptFormats []ImageFormat `json:"acceptFormats,omitempty"` } type SecretStatus struct { diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go index 44f8b914d1..0891f4eacf 100644 --- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go +++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go @@ -684,7 +684,7 @@ func (in *PreprovisioningImage) DeepCopyInto(out *PreprovisioningImage) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -741,6 +741,11 @@ func (in *PreprovisioningImageList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PreprovisioningImageSpec) DeepCopyInto(out *PreprovisioningImageSpec) { *out = *in + if in.AcceptFormats != nil { + in, out := &in.AcceptFormats, &out.AcceptFormats + *out = make([]ImageFormat, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PreprovisioningImageSpec. diff --git a/config/crd/bases/metal3.io_preprovisioningimages.yaml b/config/crd/bases/metal3.io_preprovisioningimages.yaml index 180f1f60dc..5447d1796d 100644 --- a/config/crd/bases/metal3.io_preprovisioningimages.yaml +++ b/config/crd/bases/metal3.io_preprovisioningimages.yaml @@ -48,6 +48,15 @@ spec: spec: description: PreprovisioningImageSpec defines the desired state of PreprovisioningImage properties: + acceptFormats: + description: acceptFormats is a list of acceptable image formats. + items: + description: ImageFormat enumerates the allowed image formats + enum: + - iso + - initrd + type: string + type: array architecture: description: architecture is the processor architecture for which to build the image. diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml index 7c994b33d1..72599d260c 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -1413,6 +1413,15 @@ spec: spec: description: PreprovisioningImageSpec defines the desired state of PreprovisioningImage properties: + acceptFormats: + description: acceptFormats is a list of acceptable image formats. + items: + description: ImageFormat enumerates the allowed image formats + enum: + - iso + - initrd + type: string + type: array architecture: description: architecture is the processor architecture for which to build the image. From 94942dd7fef2049049cbc5ff9f607e7c5ad91241 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 12 Jul 2021 10:39:56 -0400 Subject: [PATCH 2/2] Use PreprovisioningImage for all image formats Permit the PreprovisioningImage to return either an ISO or an initrd, and use it to build the image whenever it is enabled, regardless of whether the ISO format is allowed. The format to use is selected through the AllowFormats list in the Spec. --- .../metal3.io/baremetalhost_controller.go | 14 +++ .../baremetalhost_controller_test.go | 97 +++++++++++++++++-- .../preprovisioningimage_controller.go | 37 +++++-- pkg/provisioner/ironic/ironic.go | 10 -- .../ironic/validatemanagementaccess_test.go | 4 +- 5 files changed, 135 insertions(+), 27 deletions(-) diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index ef0b5d322c..d76a3bfe35 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -618,6 +618,19 @@ func (r *BareMetalHostReconciler) preprovImageAvailable(info *reconcileInfo, ima return false, nil } + validFormat := false + for _, f := range image.Spec.AcceptFormats { + if image.Status.Format == f { + validFormat = true + break + } + } + if !validFormat { + info.log.Info("pre-provisioning image format not accepted", + "format", image.Status.Format) + return false, nil + } + if image.Spec.NetworkDataName != "" { secretKey := client.ObjectKey{ Name: image.Spec.NetworkDataName, @@ -681,6 +694,7 @@ func (r *BareMetalHostReconciler) getPreprovImage(info *reconcileInfo, formats [ expectedSpec := metal3v1alpha1.PreprovisioningImageSpec{ NetworkDataName: info.host.Spec.PreprovisioningNetworkDataName, Architecture: getHostArchitecture(info.host), + AcceptFormats: formats, } preprovImage := metal3v1alpha1.PreprovisioningImage{} diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go index b2195dabfb..5931bf21b5 100644 --- a/controllers/metal3.io/baremetalhost_controller_test.go +++ b/controllers/metal3.io/baremetalhost_controller_test.go @@ -2121,13 +2121,15 @@ func TestGetPreprovImageCreateUpdate(t *testing.T) { func TestGetPreprovImage(t *testing.T) { host := newDefaultHost(t) imageURL := "http://example.test/image.iso" + acceptFormats := []metal3v1alpha1.ImageFormat{metal3v1alpha1.ImageFormatISO, metal3v1alpha1.ImageFormatInitRD} image := &metal3v1alpha1.PreprovisioningImage{ ObjectMeta: metav1.ObjectMeta{ Name: host.Name, Namespace: namespace, }, Spec: metal3v1alpha1.PreprovisioningImageSpec{ - Architecture: "x86_64", + Architecture: "x86_64", + AcceptFormats: acceptFormats, }, Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", @@ -2148,7 +2150,7 @@ func TestGetPreprovImage(t *testing.T) { r := newTestReconciler(host, image) i := makeReconcileInfo(host) - imgData, err := r.getPreprovImage(i, []metal3v1alpha1.ImageFormat{metal3v1alpha1.ImageFormatISO}) + imgData, err := r.getPreprovImage(i, acceptFormats) assert.NoError(t, err) assert.NotNil(t, imgData) assert.Equal(t, imageURL, imgData.ImageURL) @@ -2204,7 +2206,8 @@ func TestPreprovImageAvailable(t *testing.T) { { Scenario: "ready no netdata", Spec: metal3v1alpha1.PreprovisioningImageSpec{ - Architecture: "x86_64", + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, }, Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", @@ -2226,6 +2229,7 @@ func TestPreprovImageAvailable(t *testing.T) { Scenario: "ready", Spec: metal3v1alpha1.PreprovisioningImageSpec{ Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, NetworkDataName: "network_secret_1", }, Status: metal3v1alpha1.PreprovisioningImageStatus{ @@ -2248,10 +2252,65 @@ func TestPreprovImageAvailable(t *testing.T) { }, Available: true, }, + { + Scenario: "ready initrd", + Spec: metal3v1alpha1.PreprovisioningImageSpec{ + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"initrd"}, + NetworkDataName: "network_secret_1", + }, + Status: metal3v1alpha1.PreprovisioningImageStatus{ + Architecture: "x86_64", + Format: "initrd", + NetworkData: metal3v1alpha1.SecretStatus{ + Name: "network_secret_1", + Version: "1000", + }, + Conditions: []metav1.Condition{ + { + Type: string(metal3v1alpha1.ConditionImageReady), + Status: metav1.ConditionTrue, + }, + { + Type: string(metal3v1alpha1.ConditionImageError), + Status: metav1.ConditionFalse, + }, + }, + }, + Available: true, + }, + { + Scenario: "ready initrd fallback", + Spec: metal3v1alpha1.PreprovisioningImageSpec{ + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, + NetworkDataName: "network_secret_1", + }, + Status: metal3v1alpha1.PreprovisioningImageStatus{ + Architecture: "x86_64", + Format: "initrd", + NetworkData: metal3v1alpha1.SecretStatus{ + Name: "network_secret_1", + Version: "1000", + }, + Conditions: []metav1.Condition{ + { + Type: string(metal3v1alpha1.ConditionImageReady), + Status: metav1.ConditionTrue, + }, + { + Type: string(metal3v1alpha1.ConditionImageError), + Status: metav1.ConditionFalse, + }, + }, + }, + Available: true, + }, { Scenario: "ready secret outdated", Spec: metal3v1alpha1.PreprovisioningImageSpec{ Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, NetworkDataName: "network_secret_1", }, Status: metal3v1alpha1.PreprovisioningImageStatus{ @@ -2278,6 +2337,7 @@ func TestPreprovImageAvailable(t *testing.T) { Scenario: "ready secret mismatch", Spec: metal3v1alpha1.PreprovisioningImageSpec{ Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, NetworkDataName: "network_secret_1", }, Status: metal3v1alpha1.PreprovisioningImageStatus{ @@ -2303,7 +2363,8 @@ func TestPreprovImageAvailable(t *testing.T) { { Scenario: "ready arch mismatch", Spec: metal3v1alpha1.PreprovisioningImageSpec{ - Architecture: "aarch64", + Architecture: "aarch64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, }, Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", @@ -2322,9 +2383,32 @@ func TestPreprovImageAvailable(t *testing.T) { Available: false, }, { - Scenario: "not ready", + Scenario: "ready format mismatch", Spec: metal3v1alpha1.PreprovisioningImageSpec{ + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"initrd"}, + }, + Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", + Format: "iso", + Conditions: []metav1.Condition{ + { + Type: string(metal3v1alpha1.ConditionImageReady), + Status: metav1.ConditionTrue, + }, + { + Type: string(metal3v1alpha1.ConditionImageError), + Status: metav1.ConditionFalse, + }, + }, + }, + Available: false, + }, + { + Scenario: "not ready", + Spec: metal3v1alpha1.PreprovisioningImageSpec{ + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, }, Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", @@ -2345,7 +2429,8 @@ func TestPreprovImageAvailable(t *testing.T) { { Scenario: "failed", Spec: metal3v1alpha1.PreprovisioningImageSpec{ - Architecture: "x86_64", + Architecture: "x86_64", + AcceptFormats: []metal3v1alpha1.ImageFormat{"iso", "initrd"}, }, Status: metal3v1alpha1.PreprovisioningImageStatus{ Architecture: "x86_64", diff --git a/controllers/metal3.io/preprovisioningimage_controller.go b/controllers/metal3.io/preprovisioningimage_controller.go index 166228310f..344c2fd44d 100644 --- a/controllers/metal3.io/preprovisioningimage_controller.go +++ b/controllers/metal3.io/preprovisioningimage_controller.go @@ -93,7 +93,7 @@ func (r *PreprovisioningImageReconciler) Reconcile(ctx context.Context, req ctrl func (r *PreprovisioningImageReconciler) update(img *metal3.PreprovisioningImage, log logr.Logger) (bool, error) { generation := img.GetGeneration() - url, format, errorMessage := getImageURL() + url, format, errorMessage := getImageURL(img.Spec.AcceptFormats) if errorMessage != "" { log.Info("no suitable image URL available", "preferredFormat", format) return setError(generation, &img.Status, reasonImageConfigurationError, errorMessage), nil @@ -116,12 +116,26 @@ func (r *PreprovisioningImageReconciler) update(img *metal3.PreprovisioningImage return false, err } -func getImageURL() (url string, format metal3.ImageFormat, errorMessage string) { - format = metal3.ImageFormatISO - if iso := os.Getenv("DEPLOY_ISO_URL"); iso != "" { - url = iso - } else { - errorMessage = "No DEPLOY_ISO_URL specified" +func getImageURL(acceptFormats []metal3.ImageFormat) (url string, format metal3.ImageFormat, errorMessage string) { + for _, fmt := range acceptFormats { + switch fmt { + case metal3.ImageFormatISO: + if iso := os.Getenv("DEPLOY_ISO_URL"); iso != "" { + return iso, fmt, "" + } + if errorMessage == "" { + format = fmt + errorMessage = "No DEPLOY_ISO_URL specified" + } + case metal3.ImageFormatInitRD: + if initrd := os.Getenv("DEPLOY_RAMDISK_URL"); initrd != "" { + return initrd, fmt, "" + } + if errorMessage == "" { + format = fmt + errorMessage = "No DEPLOY_RAMDISK_URL specified" + } + } } return } @@ -218,11 +232,16 @@ func setError(generation int64, status *metal3.PreprovisioningImageStatus, reaso } func (r *PreprovisioningImageReconciler) CanStart() bool { + deployKernelURL := os.Getenv("DEPLOY_KERNEL_URL") + deployRamdiskURL := os.Getenv("DEPLOY_RAMDISK_URL") deployISOURL := os.Getenv("DEPLOY_ISO_URL") - hasCfg := deployISOURL != "" + hasCfg := (deployISOURL != "" || + (deployKernelURL != "" && deployRamdiskURL != "")) if hasCfg { r.Log.Info("have deploy image data", - "iso_url", deployISOURL) + "iso_url", deployISOURL, + "ramdisk_url", deployRamdiskURL, + "kernel_url", deployKernelURL) } else { r.Log.Info("not starting preprovisioning image controller; no image data available") } diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index 4c3d94207c..ad9cf1cf60 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -554,14 +554,6 @@ func (p *ironicProvisioner) PreprovisioningImageFormats() ([]metal3v1alpha1.Imag var formats []metal3v1alpha1.ImageFormat if accessDetails.SupportsISOPreprovisioningImage() { formats = append(formats, metal3v1alpha1.ImageFormatISO) - } else { - if p.config.deployKernelURL != "" && p.config.deployRamdiskURL != "" { - // This is a PXE driver (no ISO support) so it shouldn't require any - // network customisation to boot the image, and we have sufficient - // data available to configure. Therefore, do not request an image - // build. - return nil, nil - } } if p.config.deployKernelURL != "" { formats = append(formats, metal3v1alpha1.ImageFormatInitRD) @@ -611,8 +603,6 @@ func setDeployImage(driverInfo map[string]interface{}, config ironicConfig, acce deployImageInfo[deployISOKey] = config.deployISOURL return deployImageInfo } - } - if !config.havePreprovImgBuilder || !allowISO { if allowInitRD && config.deployRamdiskURL != "" { deployImageInfo[deployKernelKey] = config.deployKernelURL deployImageInfo[deployRamdiskKey] = config.deployRamdiskURL diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go index 738e446aa6..b028a13bea 100644 --- a/pkg/provisioner/ironic/validatemanagementaccess_test.go +++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go @@ -827,7 +827,7 @@ func TestPreprovisioningImageFormats(t *testing.T) { Name: "enabled ipmi", Address: "ipmi://example.test", PreprovImgEnabled: true, - Expected: nil, + Expected: []metal3v1alpha1.ImageFormat{"initrd"}, }, { Name: "enabled virtualmedia", @@ -978,7 +978,7 @@ func TestSetDeployImage(t *testing.T) { }, ExpectBuild: false, ExpectISO: false, - ExpectPXE: true, + ExpectPXE: false, }, { Scenario: "pxe build no kernel",