Repository: metal3-io/baremetal-operator
Pull request #759 (2021-01-11 18:43:05)

Add live-iso support
Alternative to #754 

@dhellmann @zaneb PTAL - if you're happy with this interface I'll update the design doc PR

This is working but I'm planning to add some more tests and try to clean up the diff a little before removing WIP

metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#" title="Author self-approved">hardys</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [hardys]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
hardys: The new interface looks like:

```
  image:
    url: http://172.22.0.1/images/fedora-coreos-33.20201214.2.0-live.x86_64.iso
    format: live-iso
```
metal3-io-bot: @hardys: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
generate | de55f3105d9ab6808a76330daa5cfeb7555880fb | [link](https://prow.apps.ci.metal3.io/view/gcs/metal3-prow-artifacts/pr-logs/pull/metal3-io_baremetal-operator/759/generate/1349050624494276609) | `/test generate`



<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
hardys: /retitle Add live-iso support

hardys: /test-integration
hardys: /test-integration
metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#pullrequestreview-568276984" title="Approved">dhellmann</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#" title="Author self-approved">hardys</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [dhellmann,hardys]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
metal3-io-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#pullrequestreview-568276984" title="Approved">dhellmann</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#" title="Author self-approved">hardys</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/759#pullrequestreview-570791218" title="Approved">imain</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [dhellmann,hardys]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index aa63aefb..f37471e7 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -297,15 +297,18 @@ type Image struct {
 	URL string `json:"url"`
 
 	// Checksum is the checksum for the image.
-	Checksum string `json:"checksum"`
+	Checksum string `json:"checksum,omitempty"`
 
 	// ChecksumType is the checksum algorithm for the image.
 	// e.g md5, sha256, sha512
 	ChecksumType ChecksumType `json:"checksumType,omitempty"`
 
-	// DiskFormat contains the format of the image (raw, qcow2, ...)
-	// Needs to be set to raw for raw images streaming
-	// +kubebuilder:validation:Enum=raw;qcow2;vdi;vmdk
+	// DiskFormat contains the format of the image (raw, qcow2, ...).
+	// Needs to be set to raw for raw images streaming.
+	// Note live-iso means an iso referenced by the url will be live-booted
+	// and not deployed to disk, and in this case the checksum options
+	// are not required and if specified will be ignored.
+	// +kubebuilder:validation:Enum=raw;qcow2;vdi;vmdk;live-iso
 	DiskFormat *string `json:"format,omitempty"`
 }
 
@@ -812,6 +815,12 @@ func (image *Image) GetChecksum() (checksum, checksumType string, ok bool) {
 		return
 	}
 
+	if image.DiskFormat != nil && *image.DiskFormat == "live-iso" {
+		// Checksum is not required for live-iso
+		ok = true
+		return
+	}
+
 	if image.Checksum == "" {
 		// Return empty if checksum is not provided
 		return
diff --git a/config/crd/bases/metal3.io_baremetalhosts.yaml b/config/crd/bases/metal3.io_baremetalhosts.yaml
index c957b003..a1d263cd 100644
--- a/config/crd/bases/metal3.io_baremetalhosts.yaml
+++ b/config/crd/bases/metal3.io_baremetalhosts.yaml
@@ -141,18 +141,18 @@ spec:
                     - sha512
                     type: string
                   format:
-                    description: DiskFormat contains the format of the image (raw, qcow2, ...) Needs to be set to raw for raw images streaming
+                    description: DiskFormat contains the format of the image (raw, qcow2, ...). Needs to be set to raw for raw images streaming. Note live-iso means an iso referenced by the url will be live-booted and not deployed to disk, and in this case the checksum options are not required and if specified will be ignored.
                     enum:
                     - raw
                     - qcow2
                     - vdi
                     - vmdk
+                    - live-iso
                     type: string
                   url:
                     description: URL is a location of an image to deploy.
                     type: string
                 required:
-                - checksum
                 - url
                 type: object
               metaData:
@@ -554,18 +554,18 @@ spec:
                         - sha512
                         type: string
                       format:
-                        description: DiskFormat contains the format of the image (raw, qcow2, ...) Needs to be set to raw for raw images streaming
+                        description: DiskFormat contains the format of the image (raw, qcow2, ...). Needs to be set to raw for raw images streaming. Note live-iso means an iso referenced by the url will be live-booted and not deployed to disk, and in this case the checksum options are not required and if specified will be ignored.
                         enum:
                         - raw
                         - qcow2
                         - vdi
                         - vmdk
+                        - live-iso
                         type: string
                       url:
                         description: URL is a location of an image to deploy.
                         type: string
                     required:
-                    - checksum
                     - url
                     type: object
                   rootDeviceHints:
diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml
index 88532073..7614d1ea 100644
--- a/config/render/capm3.yaml
+++ b/config/render/capm3.yaml
@@ -139,18 +139,18 @@ spec:
                     - sha512
                     type: string
                   format:
-                    description: DiskFormat contains the format of the image (raw, qcow2, ...) Needs to be set to raw for raw images streaming
+                    description: DiskFormat contains the format of the image (raw, qcow2, ...). Needs to be set to raw for raw images streaming. Note live-iso means an iso referenced by the url will be live-booted and not deployed to disk, and in this case the checksum options are not required and if specified will be ignored.
                     enum:
                     - raw
                     - qcow2
                     - vdi
                     - vmdk
+                    - live-iso
                     type: string
                   url:
                     description: URL is a location of an image to deploy.
                     type: string
                 required:
-                - checksum
                 - url
                 type: object
               metaData:
@@ -552,18 +552,18 @@ spec:
                         - sha512
                         type: string
                       format:
-                        description: DiskFormat contains the format of the image (raw, qcow2, ...) Needs to be set to raw for raw images streaming
+                        description: DiskFormat contains the format of the image (raw, qcow2, ...). Needs to be set to raw for raw images streaming. Note live-iso means an iso referenced by the url will be live-booted and not deployed to disk, and in this case the checksum options are not required and if specified will be ignored.
                         enum:
                         - raw
                         - qcow2
                         - vdi
                         - vmdk
+                        - live-iso
                         type: string
                       url:
                         description: URL is a location of an image to deploy.
                         type: string
                     required:
-                    - checksum
                     - url
                     type: object
                   rootDeviceHints:
diff --git a/docs/api.md b/docs/api.md
index 3a16ea43..f5011ff5 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -92,8 +92,10 @@ The sub-fields are
   only `md5`, `sha256`, `sha512` are recognized. If nothing is specified
   `md5` is assumed.
 * *format* -- This is the disk format of the image. It can be one of `raw`,
-  `qcow2`, `vdi`, `vmdk`, or be left unset. Setting it to raw enables raw
-  image streaming in Ironic agent for that image.
+  `qcow2`, `vdi`, `vmdk`, `live-iso` or be left unset.
+  Setting it to raw enables raw image streaming in Ironic agent for that image.
+  Setting it to live-iso enables iso images to live boot without deploying
+  to disk, in this case the checksum fields are ignored.
 
 Even though the image sub-fields are required by Ironic,
 when the host provisioning is managed externally via `externallyProvisioned: true`,
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 6b3eca6b..9e364fcb 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -372,6 +372,7 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b
 				BootInterface:       p.bmcAccess.BootInterface(),
 				Name:                p.host.Name,
 				DriverInfo:          driverInfo,
+				DeployInterface:     p.deployInterface(),
 				InspectInterface:    "inspector",
 				ManagementInterface: p.bmcAccess.ManagementInterface(),
 				PowerInterface:      p.bmcAccess.PowerInterface(),
@@ -736,8 +737,72 @@ func (p *ironicProvisioner) getImageUpdateOptsForNode(ironicNode *nodes.Node, im
 			Value: string(p.host.ObjectMeta.UID),
 		},
 	)
-	// image_source
+	// live-iso format
 	var op nodes.UpdateOp
+	if imageData.DiskFormat != nil && *imageData.DiskFormat == "live-iso" {
+		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: imageData.URL,
+			},
+		)
+		updates = append(
+			updates,
+			nodes.UpdateOperation{
+				Op:    nodes.ReplaceOp,
+				Path:  "/deploy_interface",
+				Value: "ramdisk",
+			},
+		)
+		// remove any image_source or checksum options
+		removals := []string{
+			"image_source", "image_os_hash_value", "image_os_hash_algo", "image_checksum"}
+		op = nodes.RemoveOp
+		for _, item := range removals {
+			if _, ok := ironicNode.InstanceInfo[item]; ok {
+				p.log.Info("removing " + item)
+				updates = append(
+					updates,
+					nodes.UpdateOperation{
+						Op:   op,
+						Path: "/instance_info/" + item,
+					},
+				)
+			}
+		}
+		return updates, nil
+	}
+
+	// Set deploy_interface direct when not booting a live-iso
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    nodes.ReplaceOp,
+			Path:  "/deploy_interface",
+			Value: "direct",
+		},
+	)
+	// Remove any boot_iso field
+	if _, ok := ironicNode.InstanceInfo["boot_iso"]; ok {
+		p.log.Info("removing boot_iso")
+		updates = append(
+			updates,
+			nodes.UpdateOperation{
+				Op:   nodes.RemoveOp,
+				Path: "/instance_info/boot_iso",
+			},
+		)
+	}
+	// image_source
 	if _, ok := ironicNode.InstanceInfo["image_source"]; !ok {
 		op = nodes.AddOp
 		p.log.Info("adding image_source")
@@ -1016,6 +1081,14 @@ func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, hostCon
 	return
 }
 
+func (p *ironicProvisioner) deployInterface() (result string) {
+	result = "direct"
+	if p.host.Spec.Image != nil && p.host.Spec.Image.DiskFormat != nil && *p.host.Spec.Image.DiskFormat == "live-iso" {
+		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
@@ -1058,6 +1131,30 @@ func (p *ironicProvisioner) Adopt(force bool) (result provisioner.Result, err er
 	return operationComplete()
 }
 
+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.Image != nil && p.host.Spec.Image.DiskFormat != nil && *p.host.Spec.Image.DiskFormat == "live-iso" {
+		sameImage = (ironicNode.InstanceInfo["boot_iso"] == p.host.Spec.Image.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.
@@ -1073,20 +1170,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)
 
 	// Ironic has the settings it needs, see if it finds any issues
 	// with them.


Test patch:
diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go
index 864609a3..f4d7bb7c 100644
--- a/pkg/provisioner/ironic/ironic_test.go
+++ b/pkg/provisioner/ironic/ironic_test.go
@@ -71,5 +71,12 @@ func makeHost() metal3v1alpha1.BareMetalHost {
 	}
 }
 
+func makeHostLiveIso() (host metal3v1alpha1.BareMetalHost) {
+	host = makeHost()
+	format := "live-iso"
+	host.Spec.Image.DiskFormat = &format
+	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 a8f3d693..7f578915 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"
@@ -237,3 +238,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 = makeHostLiveIso()
+				host.Spec.Image.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 e34936f8..92fce010 100644
--- a/pkg/provisioner/ironic/updateopts_test.go
+++ b/pkg/provisioner/ironic/updateopts_test.go
@@ -278,3 +278,216 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 		})
 	}
 }
+
+func TestGetUpdateOptsForNodeLiveIso(t *testing.T) {
+	eventPublisher := func(reason, message string) {}
+	auth := clients.AuthConfig{Type: clients.NoAuth}
+
+	prov, err := newProvisionerWithSettings(makeHostLiveIso(), 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)
+		Op    nodes.UpdateOp // The operation add/replace/remove
+	}{
+		{
+			Path:  "/instance_info/boot_iso",
+			Value: "not-empty",
+			Op:    nodes.AddOp,
+		},
+		{
+			Path:  "/deploy_interface",
+			Value: "ramdisk",
+			Op:    nodes.ReplaceOp,
+		},
+	}
+
+	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))
+		})
+	}
+}
+
+func TestGetUpdateOptsForNodeImageToLiveIso(t *testing.T) {
+	eventPublisher := func(reason, message string) {}
+	auth := clients.AuthConfig{Type: clients.NoAuth}
+
+	prov, err := newProvisionerWithSettings(makeHostLiveIso(), bmc.Credentials{}, eventPublisher,
+		"https://ironic.test", auth, "https://ironic.test", auth,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ironicNode := &nodes.Node{
+		InstanceInfo: map[string]interface{}{
+			"image_source":        "oldimage",
+			"image_os_hash_value": "thechecksum",
+			"image_os_hash_algo":  "md5",
+		},
+	}
+
+	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)
+		Op    nodes.UpdateOp // The operation add/replace/remove
+	}{
+		{
+			Path:  "/instance_info/boot_iso",
+			Value: "not-empty",
+			Op:    nodes.AddOp,
+		},
+		{
+			Path:  "/deploy_interface",
+			Value: "ramdisk",
+			Op:    nodes.ReplaceOp,
+		},
+		{
+			Path: "/instance_info/image_source",
+			Op:   nodes.RemoveOp,
+		},
+		{
+			Path: "/instance_info/image_os_hash_algo",
+			Op:   nodes.RemoveOp,
+		},
+		{
+			Path: "/instance_info/image_os_hash_value",
+			Op:   nodes.RemoveOp,
+		},
+	}
+
+	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 value does not match", e.Path))
+			assert.Equal(t, e.Op, update.Op, fmt.Sprintf("%s operation does not match", e.Path))
+		})
+	}
+}
+
+func TestGetUpdateOptsForNodeLiveIsoToImage(t *testing.T) {
+	eventPublisher := func(reason, message string) {}
+	auth := clients.AuthConfig{Type: clients.NoAuth}
+
+	host := makeHost()
+	host.Spec.Image.URL = "newimage"
+	host.Spec.Image.Checksum = "thechecksum"
+	host.Spec.Image.ChecksumType = metal3v1alpha1.MD5
+	prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, eventPublisher,
+		"https://ironic.test", auth, "https://ironic.test", auth,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ironicNode := &nodes.Node{
+		InstanceInfo: map[string]interface{}{
+			"boot_iso": "oldimage",
+		},
+	}
+
+	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)
+		Op    nodes.UpdateOp // The operation add/replace/remove
+	}{
+		{
+			Path: "/instance_info/boot_iso",
+			Op:   nodes.RemoveOp,
+		},
+		{
+			Path:  "/deploy_interface",
+			Value: "direct",
+			Op:    nodes.ReplaceOp,
+		},
+		{
+			Path:  "/instance_info/image_source",
+			Value: "newimage",
+			Op:    nodes.AddOp,
+		},
+		{
+			Path:  "/instance_info/image_os_hash_algo",
+			Value: "md5",
+			Op:    nodes.AddOp,
+		},
+		{
+			Path:  "/instance_info/image_os_hash_value",
+			Value: "thechecksum",
+			Op:    nodes.AddOp,
+		},
+	}
+
+	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 value does not match", e.Path))
+			assert.Equal(t, e.Op, update.Op, fmt.Sprintf("%s operation does not match", e.Path))
+		})
+	}
+}
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go
index 378d62ff..40624a4d 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go
@@ -102,6 +102,83 @@ func TestValidateManagementAccessCreateNode(t *testing.T) {
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", createdNode.UUID)
 	assert.Equal(t, createdNode.UUID, provID)
+	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, provID, err := prov.ValidateManagementAccess(false, false)
+	if err != nil {
+		t.Fatalf("error from ValidateManagementAccess: %s", err)
+	}
+	assert.Equal(t, "", result.ErrorMessage)
+	assert.Equal(t, createdNode.UUID, provID)
+	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 TestValidateManagementAccessCreateWithLiveIso(t *testing.T) {
+	// Create a host with Image specified in the Spec
+	host := makeHostLiveIso()
+	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, provID, err := prov.ValidateManagementAccess(false, false)
+	if err != nil {
+		t.Fatalf("error from ValidateManagementAccess: %s", err)
+	}
+	assert.Equal(t, "", result.ErrorMessage)
+	assert.Equal(t, createdNode.UUID, provID)
+	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.Image.URL)
 }
 
 func TestValidateManagementAccessExistingNode(t *testing.T) {