Repository: metal3-io/baremetal-operator
Pull request #533 (2020-05-20 06:24:35)

Add Image disk format in api and ironic updates
In order for ironic to use raw image streaming, it needs to be aware that the image is in raw format. This gives the user the possibility to specify that the image is raw. If left empty, the previous behaviour (copy in RAM) is maintained, for qcow2 images etc.

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/533#" title="Author self-approved">maelk</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)~~ [maelk]

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":[]} -->
maelk: /cc @dhellmann 
/cc @zaneb 
maelk: /test-integration
zaneb: /approve
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/533#" title="Author self-approved">maelk</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/533#issuecomment-632335557" title="Approved">zaneb</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)~~ [maelk,zaneb]

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":[]} -->
maelk: /hold testing with CAPM3 in progress
maelk: This requires a change from ironic side to support raw image streaming : https://review.opendev.org/#/c/730317/
I will update the PR according to your comments. I was trying to be as exhaustive as possible, but we can probably limit the list to a minimum set and add new formats when needed.
maelk: /test-integration

Patch:
diff --git a/deploy/crds/metal3.io_baremetalhosts_crd.yaml b/deploy/crds/metal3.io_baremetalhosts_crd.yaml
index f0d07d26..adef06cc 100644
--- a/deploy/crds/metal3.io_baremetalhosts_crd.yaml
+++ b/deploy/crds/metal3.io_baremetalhosts_crd.yaml
@@ -157,6 +157,15 @@ spec:
                   - sha256
                   - sha512
                   type: string
+                format:
+                  description: DiskFormat contains the format of the image (raw, qcow2,
+                    ...) Needs to be set to raw for raw images streaming
+                  enum:
+                  - raw
+                  - qcow2
+                  - vdi
+                  - vmdk
+                  type: string
                 url:
                   description: URL is a location of an image to deploy.
                   type: string
@@ -547,6 +556,15 @@ spec:
                       - sha256
                       - sha512
                       type: string
+                    format:
+                      description: DiskFormat contains the format of the image (raw,
+                        qcow2, ...) Needs to be set to raw for raw images streaming
+                      enum:
+                      - raw
+                      - qcow2
+                      - vdi
+                      - vmdk
+                      type: string
                     url:
                       description: URL is a location of an image to deploy.
                       type: string
diff --git a/docs/api.md b/docs/api.md
index b92c75c6..c6663995 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -76,6 +76,9 @@ mainly, but not only, provisioning details.
   * *checksumType* -- Checksum algorithms can be specified. Currently
     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.
 
 * *userData* -- A reference to the Secret containing the cloudinit user data
   and its namespace, so it can be attached to the host before it boots
diff --git a/go.mod b/go.mod
index a72aa878..f5dea03e 100644
--- a/go.mod
+++ b/go.mod
@@ -25,7 +25,7 @@ require (
 	k8s.io/apimachinery v0.17.4
 	k8s.io/client-go v12.0.0+incompatible
 	k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a
-	k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab // indirect
+	k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab
 	sigs.k8s.io/controller-runtime v0.5.2
 	sigs.k8s.io/yaml v1.2.0
 
diff --git a/hack/unit.sh b/hack/unit.sh
index 5f1cca74..0fce1f70 100755
--- a/hack/unit.sh
+++ b/hack/unit.sh
@@ -18,9 +18,9 @@ else
     --env DEPLOY_RAMDISK_URL=http://172.22.0.1/images/ironic-python-agent.initramfs \
     --env IRONIC_ENDPOINT=http://localhost:6385/v1/ \
     --env IRONIC_INSPECTOR_ENDPOINT=http://localhost:5050/v1/ \
-    --volume "${PWD}:/go/src/github.com/metal3-io/baremetal-operator:ro,z" \
+    --volume "${PWD}:/go/src/github.com/metal3-io/baremetal-operator:rw,z" \
     --entrypoint sh \
     --workdir /go/src/github.com/metal3-io/baremetal-operator \
-    registry.hub.docker.com/library/golang:1.12 \
+    registry.hub.docker.com/library/golang:1.13.7 \
     /go/src/github.com/metal3-io/baremetal-operator/hack/unit.sh "${@}"
 fi;
diff --git a/pkg/apis/metal3/v1alpha1/baremetalhost_types.go b/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
index a1edfa8d..a4ffc717 100644
--- a/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
+++ b/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
@@ -231,6 +231,11 @@ type Image struct {
 	// 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 *string `json:"format,omitempty"`
 }
 
 // FIXME(dhellmann): We probably want some other module to own these
diff --git a/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
index f3bc79c6..dcd9c3ab 100644
--- a/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
@@ -121,7 +121,7 @@ func (in *BareMetalHostSpec) DeepCopyInto(out *BareMetalHostSpec) {
 	if in.Image != nil {
 		in, out := &in.Image, &out.Image
 		*out = new(Image)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	if in.UserData != nil {
 		in, out := &in.UserData, &out.UserData
@@ -163,7 +163,7 @@ func (in *BareMetalHostStatus) DeepCopyInto(out *BareMetalHostStatus) {
 		*out = new(HardwareDetails)
 		(*in).DeepCopyInto(*out)
 	}
-	out.Provisioning = in.Provisioning
+	in.Provisioning.DeepCopyInto(&out.Provisioning)
 	in.GoodCredentials.DeepCopyInto(&out.GoodCredentials)
 	in.TriedCredentials.DeepCopyInto(&out.TriedCredentials)
 	in.OperationHistory.DeepCopyInto(&out.OperationHistory)
@@ -289,6 +289,11 @@ func (in *HardwareSystemVendor) DeepCopy() *HardwareSystemVendor {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Image) DeepCopyInto(out *Image) {
 	*out = *in
+	if in.DiskFormat != nil {
+		in, out := &in.DiskFormat, &out.DiskFormat
+		*out = new(string)
+		**out = **in
+	}
 	return
 }
 
@@ -364,7 +369,7 @@ func (in *OperationMetric) DeepCopy() *OperationMetric {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ProvisionStatus) DeepCopyInto(out *ProvisionStatus) {
 	*out = *in
-	out.Image = in.Image
+	in.Image.DeepCopyInto(&out.Image)
 	return
 }
 
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index f03d4aa6..e662935c 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -310,6 +310,13 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged bool) (r
 					Value: string(p.host.ObjectMeta.UID),
 				},
 			}
+			if p.host.Spec.Image.DiskFormat != nil {
+				updates = append(updates, nodes.UpdateOperation{
+					Op:    nodes.AddOp,
+					Path:  "/instance_info/image_disk_format",
+					Value: *p.host.Spec.Image.DiskFormat,
+				})
+			}
 			_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
 			switch err.(type) {
 			case nil:
@@ -613,6 +620,14 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (update
 		},
 	)
 
+	if p.host.Spec.Image.DiskFormat != nil {
+		updates = append(updates, nodes.UpdateOperation{
+			Op:    nodes.AddOp,
+			Path:  "/instance_info/image_disk_format",
+			Value: *p.host.Spec.Image.DiskFormat,
+		})
+	}
+
 	// instance_uuid
 	p.log.Info("setting instance_uuid")
 	updates = append(


Test patch:
diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go
index b2975459..d172f9a2 100644
--- a/pkg/provisioner/ironic/ironic_test.go
+++ b/pkg/provisioner/ironic/ironic_test.go
@@ -10,6 +10,7 @@ import (
 
 	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/pkg/apis/metal3/v1alpha1"
 	"github.com/metal3-io/baremetal-operator/pkg/bmc"
+	"k8s.io/utils/pointer"
 )
 
 func init() {
@@ -28,6 +29,7 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 				URL:          "not-empty",
 				Checksum:     "checksum",
 				ChecksumType: metal3v1alpha1.MD5,
+				DiskFormat:   pointer.StringPtr("raw"),
 			},
 			Online: true,
 		},
@@ -70,6 +72,10 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 			Path:  "/instance_info/image_os_hash_value",
 			Value: "checksum",
 		},
+		{
+			Path:  "/instance_info/image_disk_format",
+			Value: "raw",
+		},
 		{
 			Path:  "/instance_uuid",
 			Value: "27720611-e5d1-45d3-ba3a-222dcfaa4ca2",
@@ -120,6 +126,7 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 				URL:          "not-empty",
 				Checksum:     "checksum",
 				ChecksumType: metal3v1alpha1.MD5,
+				//DiskFormat not given to verify it is not added in instance_info
 			},
 			Online: true,
 		},