diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go index db492bebd2..5aca18f85c 100644 --- a/apis/metal3.io/v1alpha1/baremetalhost_types.go +++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go @@ -252,6 +252,11 @@ type BareMetalHostSpec struct { // Image holds the details of the image to be provisioned. Image *Image `json:"image,omitempty"` + // LiveImage holds the details of the live-image to be booted. + // Only one of Image or Live image may be specified, and + // at present UserData cannot be used with LiveImage. + LiveImage *LiveImage `json:"liveImage,omitempty"` + // UserData holds the reference to the Secret containing the user // data to be passed to the host before it boots. UserData *corev1.SecretReference `json:"userData,omitempty"` @@ -309,6 +314,13 @@ type Image struct { DiskFormat *string `json:"format,omitempty"` } +// LiveImage holds the details of a live-image either to boot or that +// has been booted. +type LiveImage struct { + // URL is a location of an iso image to boot. + URL string `json:"url"` +} + // FIXME(dhellmann): We probably want some other module to own these // data structures. @@ -570,6 +582,10 @@ type ProvisionStatus struct { // provisioned to the host. Image Image `json:"image,omitempty"` + // LiveImage holds the details of the last live-image + // successfully booted by the host. + LiveImage LiveImage `json:"liveImage,omitempty"` + // The RootDevicehints set by the user RootDeviceHints *RootDeviceHints `json:"rootDeviceHints,omitempty"` @@ -752,18 +768,27 @@ func (host *BareMetalHost) NeedsProvisioning() bool { // The host is not supposed to be powered on. return false } - if host.Spec.Image == nil { - // Without an image, there is nothing to provision. - return false - } - if host.Spec.Image.URL == "" { - // We have an Image struct but it is empty - return false + if host.Spec.LiveImage != nil { + if host.Spec.LiveImage.URL == "" { + // We have a LiveImage struct but it is empty + return false + } + if host.Status.Provisioning.LiveImage.URL == "" { + // We have an image set, but not provisioned. + return true + } } - if host.Status.Provisioning.Image.URL == "" { - // We have an image set, but not provisioned. - return true + if host.Spec.Image != nil { + if host.Spec.Image.URL == "" { + // We have an Image struct but it is empty + return false + } + if host.Status.Provisioning.Image.URL == "" { + // We have an image set, but not provisioned. + return true + } } + // Without an image, there is nothing to provision. return false } @@ -777,6 +802,10 @@ func (host *BareMetalHost) WasProvisioned() bool { // We have an image provisioned. return true } + if host.Status.Provisioning.LiveImage.URL != "" { + // We have an image provisioned. + return true + } return false } diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go index dddb47b607..45782dc152 100644 --- a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go +++ b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go @@ -206,6 +206,23 @@ func TestHostNeedsProvisioning(t *testing.T) { Expected: true, }, + { + Scenario: "with live image url, online", + Host: BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myhost", + Namespace: "myns", + }, + Spec: BareMetalHostSpec{ + LiveImage: &LiveImage{ + URL: "not-empty", + }, + Online: true, + }, + }, + Expected: true, + }, + { Scenario: "with image url, offline", Host: BareMetalHost{ @@ -224,7 +241,24 @@ func TestHostNeedsProvisioning(t *testing.T) { }, { - Scenario: "already provisioned", + Scenario: "with live-image url, offline", + Host: BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myhost", + Namespace: "myns", + }, + Spec: BareMetalHostSpec{ + LiveImage: &LiveImage{ + URL: "not-empty", + }, + Online: false, + }, + }, + Expected: false, + }, + + { + Scenario: "image already provisioned", Host: BareMetalHost{ ObjectMeta: metav1.ObjectMeta{ Name: "myhost", @@ -247,6 +281,30 @@ func TestHostNeedsProvisioning(t *testing.T) { Expected: false, }, + { + Scenario: "live-image already provisioned", + Host: BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myhost", + Namespace: "myns", + }, + Spec: BareMetalHostSpec{ + LiveImage: &LiveImage{ + URL: "not-empty", + }, + Online: true, + }, + Status: BareMetalHostStatus{ + Provisioning: ProvisionStatus{ + LiveImage: LiveImage{ + URL: "also-not-empty", + }, + }, + }, + }, + Expected: false, + }, + { Scenario: "externally provisioned", Host: BareMetalHost{ diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go index 928d79202c..356c15c64f 100644 --- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go +++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go @@ -140,6 +140,11 @@ func (in *BareMetalHostSpec) DeepCopyInto(out *BareMetalHostSpec) { *out = new(Image) (*in).DeepCopyInto(*out) } + if in.LiveImage != nil { + in, out := &in.LiveImage, &out.LiveImage + *out = new(LiveImage) + **out = **in + } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(v1.SecretReference) @@ -316,6 +321,21 @@ func (in *Image) DeepCopy() *Image { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LiveImage) DeepCopyInto(out *LiveImage) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LiveImage. +func (in *LiveImage) DeepCopy() *LiveImage { + if in == nil { + return nil + } + out := new(LiveImage) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NIC) DeepCopyInto(out *NIC) { *out = *in @@ -376,6 +396,7 @@ func (in *OperationMetric) DeepCopy() *OperationMetric { func (in *ProvisionStatus) DeepCopyInto(out *ProvisionStatus) { *out = *in in.Image.DeepCopyInto(&out.Image) + out.LiveImage = in.LiveImage if in.RootDeviceHints != nil { in, out := &in.RootDeviceHints, &out.RootDeviceHints *out = new(RootDeviceHints) diff --git a/config/crd/bases/metal3.io_baremetalhosts.yaml b/config/crd/bases/metal3.io_baremetalhosts.yaml index 9a0b6b86a4..dd9a758206 100644 --- a/config/crd/bases/metal3.io_baremetalhosts.yaml +++ b/config/crd/bases/metal3.io_baremetalhosts.yaml @@ -155,6 +155,15 @@ spec: - checksum - url type: object + liveImage: + description: LiveImage holds the details of the live-image to be booted. Only one of Image or Live image may be specified, and at present UserData cannot be used with LiveImage. + properties: + url: + description: URL is a location of an iso image to boot. + type: string + required: + - url + type: object metaData: description: MetaData holds the reference to the Secret containing host metadata (e.g. meta_data.json which is passed to Config Drive). properties: @@ -568,6 +577,15 @@ spec: - checksum - url type: object + liveImage: + description: LiveImage holds the details of the last live-image successfully booted by the host. + properties: + url: + description: URL is a location of an iso image to boot. + type: string + required: + - url + type: object rootDeviceHints: description: The RootDevicehints set by the user properties: diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml index dd32637d66..22c0186743 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -153,6 +153,15 @@ spec: - checksum - url type: object + liveImage: + description: LiveImage holds the details of the live-image to be booted. Only one of Image or Live image may be specified, and at present UserData cannot be used with LiveImage. + properties: + url: + description: URL is a location of an iso image to boot. + type: string + required: + - url + type: object metaData: description: MetaData holds the reference to the Secret containing host metadata (e.g. meta_data.json which is passed to Config Drive). properties: @@ -566,6 +575,15 @@ spec: - checksum - url type: object + liveImage: + description: LiveImage holds the details of the last live-image successfully booted by the host. + properties: + url: + description: URL is a location of an iso image to boot. + type: string + required: + - url + type: object rootDeviceHints: description: The RootDevicehints set by the user properties: diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index a1cf3e9c9d..9cbe1f3b32 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -546,9 +546,17 @@ func (r *BareMetalHostReconciler) actionProvisioning(prov provisioner.Provisione } // If the provisioner had no work, ensure the image settings match. - if info.host.Status.Provisioning.Image != *(info.host.Spec.Image) { - info.log.Info("updating deployed image in status") - info.host.Status.Provisioning.Image = *(info.host.Spec.Image) + if info.host.Spec.Image != nil { + if info.host.Status.Provisioning.Image != *(info.host.Spec.Image) { + info.log.Info("updating deployed image in status") + info.host.Status.Provisioning.Image = *(info.host.Spec.Image) + } + } + if info.host.Spec.LiveImage != nil { + if info.host.Status.Provisioning.LiveImage != *(info.host.Spec.LiveImage) { + info.log.Info("updating booted liveImage in status") + info.host.Status.Provisioning.LiveImage = *(info.host.Spec.LiveImage) + } } // After provisioning we always requeue to ensure we enter the diff --git a/controllers/metal3.io/host_state_machine.go b/controllers/metal3.io/host_state_machine.go index 1835ca4761..98b99873fc 100644 --- a/controllers/metal3.io/host_state_machine.go +++ b/controllers/metal3.io/host_state_machine.go @@ -293,16 +293,25 @@ func (hsm *hostStateMachine) provisioningCancelled() bool { if hsm.Host.HasError() { return true } - if hsm.Host.Spec.Image == nil { + if hsm.Host.Spec.Image == nil && hsm.Host.Spec.LiveImage == nil { return true } - if hsm.Host.Spec.Image.URL == "" { + if hsm.Host.Spec.LiveImage != nil && hsm.Host.Spec.LiveImage.URL == "" { return true } - if hsm.Host.Status.Provisioning.Image.URL == "" { + if hsm.Host.Spec.LiveImage != nil && hsm.Host.Status.Provisioning.LiveImage.URL == "" { return false } - if hsm.Host.Spec.Image.URL != hsm.Host.Status.Provisioning.Image.URL { + if hsm.Host.Spec.LiveImage != nil && hsm.Host.Spec.LiveImage.URL != hsm.Host.Status.Provisioning.LiveImage.URL { + return true + } + if hsm.Host.Spec.Image != nil && hsm.Host.Spec.Image.URL == "" { + return true + } + if hsm.Host.Spec.Image != nil && hsm.Host.Status.Provisioning.Image.URL == "" { + return false + } + if hsm.Host.Spec.Image != nil && hsm.Host.Spec.Image.URL != hsm.Host.Status.Provisioning.Image.URL { return true } return false diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go index 7f717658e7..2bc64cd824 100644 --- a/controllers/metal3.io/host_state_machine_test.go +++ b/controllers/metal3.io/host_state_machine_test.go @@ -61,6 +61,38 @@ func TestProvisioningCancelled(t *testing.T) { Expected: true, }, + { + Scenario: "with liveimage url, unprovisioned", + Host: metal3v1alpha1.BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myhost", + Namespace: "myns", + }, + Spec: metal3v1alpha1.BareMetalHostSpec{ + LiveImage: &metal3v1alpha1.LiveImage{ + URL: "not-empty", + }, + Online: true, + }, + }, + Expected: false, + }, + + { + Scenario: "with liveimage, unprovisioned", + Host: metal3v1alpha1.BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myhost", + Namespace: "myns", + }, + Spec: metal3v1alpha1.BareMetalHostSpec{ + LiveImage: &metal3v1alpha1.LiveImage{}, + Online: true, + }, + }, + Expected: true, + }, + { Scenario: "without, unprovisioned", Host: metal3v1alpha1.BareMetalHost{ diff --git a/docs/api.md b/docs/api.md index 3a16ea4320..8fb8df2ada 100644 --- a/docs/api.md +++ b/docs/api.md @@ -99,6 +99,14 @@ Even though the image sub-fields are required by Ironic, when the host provisioning is managed externally via `externallyProvisioned: true`, and power control isn't needed, the fields can be left empty. +#### liveImage + +Holds details for the live-image to be booted on a given host. + +The sub-fields are + +* *url* -- The URL of an iso image to boot. + #### userData A reference to the Secret containing the cloudinit user data and its diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index 95be528753..d42eb75368 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -369,6 +369,7 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged bool) (r BootInterface: p.bmcAccess.BootInterface(), Name: p.host.Name, DriverInfo: driverInfo, + DeployInterface: p.deployInterface(), InspectInterface: "inspector", ManagementInterface: p.bmcAccess.ManagementInterface(), PowerInterface: p.bmcAccess.PowerInterface(), @@ -413,72 +414,25 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged bool) (r // case may mean we are re-adopting a host that was already // known but removed/lost because the pod restarted. var imageData *metal3v1alpha1.Image + var liveImageData *metal3v1alpha1.LiveImage switch { + case p.host.Status.Provisioning.LiveImage.URL != "": + liveImageData = &p.host.Status.Provisioning.LiveImage + case p.host.Spec.LiveImage != nil && p.host.Spec.LiveImage.URL != "": + liveImageData = p.host.Spec.LiveImage case p.host.Status.Provisioning.Image.URL != "": imageData = &p.host.Status.Provisioning.Image case p.host.Spec.Image != nil && p.host.Spec.Image.URL != "": imageData = p.host.Spec.Image } - checksum, checksumType, ok := imageData.GetChecksum() - - if ok { - p.log.Info("setting instance info", - "image_source", imageData.URL, - "image_os_hash_value", checksum, - "image_os_hash_algo", checksumType, - ) - - updates := nodes.UpdateOpts{ - nodes.UpdateOperation{ - Op: nodes.AddOp, - Path: "/instance_info/image_source", - Value: imageData.URL, - }, - nodes.UpdateOperation{ - Op: nodes.AddOp, - Path: "/instance_info/image_os_hash_value", - Value: checksum, - }, - nodes.UpdateOperation{ - Op: nodes.AddOp, - Path: "/instance_info/image_os_hash_algo", - Value: checksumType, - }, - nodes.UpdateOperation{ - Op: nodes.ReplaceOp, - Path: "/instance_uuid", - Value: string(p.host.ObjectMeta.UID), - }, - } - - // image_checksum - // - // FIXME: For older versions of ironic that do not have - // https://review.opendev.org/#/c/711816/ failing to - // include the 'image_checksum' causes ironic to refuse to - // provision the image, even if the other hash value - // parameters are given. We only want to do that for MD5, - // however, because those versions of ironic only support - // MD5 checksums. - if checksumType == string(metal3v1alpha1.MD5) { - updates = append( - updates, - nodes.UpdateOperation{ - Op: nodes.AddOp, - Path: "/instance_info/image_checksum", - Value: checksum, - }, - ) + _, _, ok := imageData.GetChecksum() + if ok || liveImageData != nil { + updates, err := p.getImageUpdateOptsForNode(ironicNode, imageData, liveImageData) + if err != nil { + return result, errors.Wrap(err, "Could not get Image options for node") } - if imageData.DiskFormat != nil { - updates = append(updates, nodes.UpdateOperation{ - Op: nodes.AddOp, - Path: "/instance_info/image_disk_format", - Value: *imageData.DiskFormat, - }) - } _, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract() switch err.(type) { case nil: @@ -775,18 +729,35 @@ func (p *ironicProvisioner) UpdateHardwareState() (result provisioner.Result, er return result, nil } -func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (updates nodes.UpdateOpts, err error) { - - hwProf, err := hardware.GetProfile(p.host.HardwareProfile()) - - if err != nil { - return updates, errors.Wrap(err, - fmt.Sprintf("Could not start provisioning with bad hardware profile %s", - p.host.HardwareProfile())) +func (p *ironicProvisioner) getImageUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, liveImageData *metal3v1alpha1.LiveImage) (updates nodes.UpdateOpts, err error) { + var op nodes.UpdateOp + if liveImageData != nil { + if _, ok := ironicNode.InstanceInfo["boot_iso"]; !ok { + op = nodes.AddOp + p.log.Info("adding boot_iso") + } else { + op = nodes.ReplaceOp + p.log.Info("updating boot_iso") + } + updates = append( + updates, + nodes.UpdateOperation{ + Op: op, + Path: "/instance_info/boot_iso", + Value: liveImageData.URL, + }, + ) + updates = append( + updates, + nodes.UpdateOperation{ + Op: nodes.ReplaceOp, + Path: "/deploy_interface", + Value: "ramdisk", + }, + ) + return updates, nil } - // image_source - var op nodes.UpdateOp if _, ok := ironicNode.InstanceInfo["image_source"]; !ok { op = nodes.AddOp p.log.Info("adding image_source") @@ -799,7 +770,7 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (update nodes.UpdateOperation{ Op: op, Path: "/instance_info/image_source", - Value: p.host.Spec.Image.URL, + Value: imageData.URL, }, ) @@ -865,13 +836,31 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (update ) } - if p.host.Spec.Image.DiskFormat != nil { + if imageData.DiskFormat != nil { updates = append(updates, nodes.UpdateOperation{ Op: nodes.AddOp, Path: "/instance_info/image_disk_format", - Value: *p.host.Spec.Image.DiskFormat, + Value: *imageData.DiskFormat, }) } + return updates, nil +} + +func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (updates nodes.UpdateOpts, err error) { + + hwProf, err := hardware.GetProfile(p.host.HardwareProfile()) + + if err != nil { + return updates, errors.Wrap(err, + fmt.Sprintf("Could not start provisioning with bad hardware profile %s", + p.host.HardwareProfile())) + } + + imageOpts, err := p.getImageUpdateOptsForNode(ironicNode, p.host.Spec.Image, p.host.Spec.LiveImage) + if err != nil { + return updates, errors.Wrap(err, "Could not get Image options for node") + } + updates = append(updates, imageOpts...) // instance_uuid p.log.Info("setting instance_uuid") @@ -889,6 +878,7 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (update // FIXME(dhellmann): We have to provide something for the disk // size until https://storyboard.openstack.org/#!/story/2005165 is // fixed in ironic. + var op nodes.UpdateOp if _, ok := ironicNode.InstanceInfo["root_gb"]; !ok { op = nodes.AddOp p.log.Info("adding root_gb") @@ -1080,10 +1070,27 @@ func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, hostCon "deploy step", ironicNode.DeployStep, ) p.publisher("ProvisioningStarted", - fmt.Sprintf("Image provisioning started for %s", p.host.Spec.Image.URL)) + fmt.Sprintf("Image provisioning started for %s", p.imageURL())) return } +func (p *ironicProvisioner) imageURL() (url string) { + if p.host.Spec.LiveImage != nil && p.host.Spec.LiveImage.URL != "" { + url = p.host.Spec.LiveImage.URL + } else if p.host.Spec.Image != nil && p.host.Spec.Image.URL != "" { + url = p.host.Spec.Image.URL + } + return url +} + +func (p *ironicProvisioner) deployInterface() (result string) { + result = "direct" + if p.host.Spec.LiveImage != nil && p.host.Spec.LiveImage.URL != "" { + result = "ramdisk" + } + return result +} + // Adopt allows an externally-provisioned server to be adopted by Ironic. func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err error) { var ironicNode *nodes.Node @@ -1129,6 +1136,30 @@ func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err er return } +func (p *ironicProvisioner) ironicHasSameImage(ironicNode *nodes.Node) (sameImage bool) { + // To make it easier to test if ironic is configured with + // the same image we are trying to provision to the host. + if p.host.Spec.LiveImage != nil && p.host.Spec.LiveImage.URL != "" { + sameImage = (ironicNode.InstanceInfo["boot_iso"] == p.host.Spec.LiveImage.URL) + p.log.Info("checking image settings", + "boot_iso", ironicNode.InstanceInfo["boot_iso"], + "same", sameImage, + "provisionState", ironicNode.ProvisionState) + } else { + checksum, checksumType, _ := p.host.GetImageChecksum() + sameImage = (ironicNode.InstanceInfo["image_source"] == p.host.Spec.Image.URL && + ironicNode.InstanceInfo["image_os_hash_algo"] == checksumType && + ironicNode.InstanceInfo["image_os_hash_value"] == checksum) + p.log.Info("checking image settings", + "source", ironicNode.InstanceInfo["image_source"], + "image_os_hash_algo", checksumType, + "image_os_has_value", checksum, + "same", sameImage, + "provisionState", ironicNode.ProvisionState) + } + return sameImage +} + // Provision writes the image from the host spec to the host. It may // be called multiple times, and should return true for its dirty flag // until the deprovisioning operation is completed. @@ -1144,20 +1175,7 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu p.log.Info("provisioning image to host", "state", ironicNode.ProvisionState) - checksum, checksumType, _ := p.host.GetImageChecksum() - - // Local variable to make it easier to test if ironic is - // configured with the same image we are trying to provision to - // the host. - ironicHasSameImage := (ironicNode.InstanceInfo["image_source"] == p.host.Spec.Image.URL && - ironicNode.InstanceInfo["image_os_hash_algo"] == checksumType && - ironicNode.InstanceInfo["image_os_hash_value"] == checksum) - p.log.Info("checking image settings", - "source", ironicNode.InstanceInfo["image_source"], - "image_os_hash_algo", checksumType, - "image_os_has_value", checksum, - "same", ironicHasSameImage, - "provisionState", ironicNode.ProvisionState) + ironicHasSameImage := p.ironicHasSameImage(ironicNode) result.RequeueAfter = provisionRequeueDelay @@ -1274,7 +1292,7 @@ func (p *ironicProvisioner) Provision(hostConf provisioner.HostConfigData) (resu case nodes.Active: // provisioning is done p.publisher("ProvisioningComplete", - fmt.Sprintf("Image provisioning completed for %s", p.host.Spec.Image.URL)) + fmt.Sprintf("Image provisioning completed for %s", p.imageURL())) p.log.Info("finished provisioning") return result, nil diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go index fbf2cba020..225ee9dece 100644 --- a/pkg/provisioner/ironic/ironic_test.go +++ b/pkg/provisioner/ironic/ironic_test.go @@ -71,5 +71,14 @@ func makeHost() *metal3v1alpha1.BareMetalHost { } } +func makeHostLiveImage() (host *metal3v1alpha1.BareMetalHost) { + host = makeHost() + host.Spec.LiveImage = &metal3v1alpha1.LiveImage{ + URL: "liveimage-not-empty", + } + host.Spec.Image = nil + return host +} + // Implements provisioner.EventPublisher to swallow events for tests. func nullEventPublisher(reason, message string) {} diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go index 6b6038ffc6..653eafbbc0 100644 --- a/pkg/provisioner/ironic/provision_test.go +++ b/pkg/provisioner/ironic/provision_test.go @@ -8,6 +8,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/baremetalintrospection/v1/introspection" "github.com/stretchr/testify/assert" + "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" "github.com/metal3-io/baremetal-operator/pkg/bmc" "github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture" "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients" @@ -219,3 +220,136 @@ func TestDeprovision(t *testing.T) { }) } } + +func TestIronicHasSameImage(t *testing.T) { + nodeUUID := "33ce8659-7400-4c68-9535-d10766f07a58" + cases := []struct { + name string + expected bool + node nodes.Node + liveImage bool + hostImage string + hostChecksum string + hostChecksumType v1alpha1.ChecksumType + }{ + { + name: "image same", + expected: true, + liveImage: false, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "image_source": "theimage", + "image_os_hash_value": "thechecksum", + "image_os_hash_algo": "md5", + }, + }, + hostImage: "theimage", + hostChecksum: "thechecksum", + hostChecksumType: v1alpha1.MD5, + }, + { + name: "image different", + expected: false, + liveImage: false, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "image_source": "theimage", + "image_os_hash_value": "thechecksum", + "image_os_hash_algo": "md5", + }, + }, + hostImage: "different", + hostChecksum: "thechecksum", + hostChecksumType: v1alpha1.MD5, + }, + { + name: "image checksum different", + expected: false, + liveImage: false, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "image_source": "theimage", + "image_os_hash_value": "thechecksum", + "image_os_hash_algo": "md5", + }, + }, + hostImage: "theimage", + hostChecksum: "different", + hostChecksumType: v1alpha1.MD5, + }, + { + name: "image checksum type different", + expected: false, + liveImage: false, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "image_source": "theimage", + "image_os_hash_value": "thechecksum", + "image_os_hash_algo": "md5", + }, + }, + hostImage: "theimage", + hostChecksum: "thechecksum", + hostChecksumType: v1alpha1.SHA512, + }, + { + name: "live image same", + liveImage: true, + expected: true, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "boot_iso": "theimage", + }, + }, + hostImage: "theimage", + }, + { + name: "live image different", + liveImage: true, + expected: false, + node: nodes.Node{ + InstanceInfo: map[string]interface{}{ + "boot_iso": "theimage", + }, + }, + hostImage: "different", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ironic := testserver.NewIronic(t).WithDefaultResponses().Node(tc.node) + ironic.Start() + defer ironic.Stop() + + inspector := testserver.NewInspector(t).Ready().WithIntrospection(nodeUUID, introspection.Introspection{ + Finished: false, + }) + inspector.Start() + defer inspector.Stop() + + var host *v1alpha1.BareMetalHost + if tc.liveImage { + host = makeHostLiveImage() + host.Spec.LiveImage.URL = tc.hostImage + } else { + host = makeHost() + host.Spec.Image.URL = tc.hostImage + host.Spec.Image.Checksum = tc.hostChecksum + host.Spec.Image.ChecksumType = tc.hostChecksumType + } + publisher := func(reason, message string) {} + auth := clients.AuthConfig{Type: clients.NoAuth} + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, publisher, + ironic.Endpoint(), auth, inspector.Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + prov.status.ID = nodeUUID + sameImage := prov.ironicHasSameImage(&tc.node) + assert.Equal(t, tc.expected, sameImage) + }) + } +} diff --git a/pkg/provisioner/ironic/updateopts_test.go b/pkg/provisioner/ironic/updateopts_test.go index 88714db735..02ca85106f 100644 --- a/pkg/provisioner/ironic/updateopts_test.go +++ b/pkg/provisioner/ironic/updateopts_test.go @@ -286,3 +286,73 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) { }) } } + +func TestGetUpdateOptsForNodeLiveImage(t *testing.T) { + eventPublisher := func(reason, message string) {} + auth := clients.AuthConfig{Type: clients.NoAuth} + + prov, err := newProvisionerWithSettings(makeHostLiveImage(), bmc.Credentials{}, eventPublisher, + "https://ironic.test", auth, "https://ironic.test", auth, + ) + if err != nil { + t.Fatal(err) + } + ironicNode := &nodes.Node{} + + patches, err := prov.getUpdateOptsForNode(ironicNode) + if err != nil { + t.Fatal(err) + } + + t.Logf("patches: %v", patches) + + expected := []struct { + Path string // the node property path + Key string // if value is a map, the key we care about + Value interface{} // the value being passed to ironic (or value associated with the key) + }{ + { + Path: "/instance_info/boot_iso", + Value: "liveimage-not-empty", + }, + { + Path: "/deploy_interface", + Value: "ramdisk", + }, + { + Path: "/instance_uuid", + Value: "27720611-e5d1-45d3-ba3a-222dcfaa4ca2", + }, + { + Path: "/instance_info/root_gb", + Value: 10, + }, + { + Path: "/properties/cpu_arch", + Value: "x86_64", + }, + { + Path: "/properties/local_gb", + Value: 50, + }, + } + + for _, e := range expected { + t.Run(e.Path, func(t *testing.T) { + t.Logf("expected: %v", e) + var update nodes.UpdateOperation + for _, patch := range patches { + update = patch.(nodes.UpdateOperation) + if update.Path == e.Path { + break + } + } + if update.Path != e.Path { + t.Errorf("did not find %q in updates", e.Path) + return + } + t.Logf("update: %v", update) + assert.Equal(t, e.Value, update.Value, fmt.Sprintf("%s does not match", e.Path)) + }) + } +} diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go index 522b0becd9..4027871300 100644 --- a/pkg/provisioner/ironic/validatemanagementaccess_test.go +++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go @@ -102,6 +102,81 @@ func TestValidateManagementAccessCreateNode(t *testing.T) { assert.Equal(t, "", result.ErrorMessage) assert.NotEqual(t, "", createdNode.UUID) assert.Equal(t, createdNode.UUID, host.Status.Provisioning.ID) + assert.Equal(t, createdNode.DeployInterface, "direct") +} + +func TestValidateManagementAccessCreateWithImage(t *testing.T) { + // Create a host with Image specified in the Spec + host := makeHost() + host.Status.Provisioning.ID = "" // so we don't lookup by uuid + host.Spec.Image.URL = "theimagefoo" + host.Spec.Image.Checksum = "thechecksumxyz" + + var createdNode *nodes.Node + + createCallback := func(node nodes.Node) { + createdNode = &node + } + + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic.AddDefaultResponse("/v1/nodes/node-0", "PATCH", http.StatusOK, "{}") + ironic.Start() + defer ironic.Stop() + + auth := clients.AuthConfig{Type: clients.NoAuth} + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, + ironic.Endpoint(), auth, testserver.NewInspector(t).Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + result, err := prov.ValidateManagementAccess(false) + if err != nil { + t.Fatalf("error from ValidateManagementAccess: %s", err) + } + assert.Equal(t, "", result.ErrorMessage) + assert.Equal(t, createdNode.DeployInterface, "direct") + updates, _ := ironic.GetLastRequestFor("/v1/nodes/node-0", http.MethodPatch) + assert.Contains(t, updates, "/instance_info/image_source") + assert.Contains(t, updates, host.Spec.Image.URL) + assert.Contains(t, updates, "/instance_info/image_checksum") + assert.Contains(t, updates, host.Spec.Image.Checksum) +} + +func TestValidateManagementAccessCreateWithLiveImage(t *testing.T) { + // Create a host with Image specified in the Spec + host := makeHostLiveImage() + host.Status.Provisioning.ID = "" // so we don't lookup by uuid + + var createdNode *nodes.Node + + createCallback := func(node nodes.Node) { + createdNode = &node + } + + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic.AddDefaultResponse("/v1/nodes/node-0", "PATCH", http.StatusOK, "{}") + ironic.Start() + defer ironic.Stop() + + auth := clients.AuthConfig{Type: clients.NoAuth} + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, + ironic.Endpoint(), auth, testserver.NewInspector(t).Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + result, err := prov.ValidateManagementAccess(false) + if err != nil { + t.Fatalf("error from ValidateManagementAccess: %s", err) + } + assert.Equal(t, "", result.ErrorMessage) + assert.Equal(t, createdNode.DeployInterface, "ramdisk") + updates, _ := ironic.GetLastRequestFor("/v1/nodes/node-0", http.MethodPatch) + assert.Contains(t, updates, "/instance_info/boot_iso") + assert.Contains(t, updates, host.Spec.LiveImage.URL) } func TestValidateManagementAccessExistingNode(t *testing.T) {