Merge pull request #191 from openshift-cherrypick-robot/cherry-pick-1…

…88-to-release-4.4

Bug 1829037: Use virtio disk instead of config drive for injecting ignition for s390x/ppc64le

pkg/cloud/libvirt/client/client.go

@@ -3,7 +3,6 @@ package client
 import (
 	"encoding/xml"
 	"fmt"
-	"strings"
 
 	"github.com/golang/glog"
 	libvirt "github.com/libvirt/libvirt-go"
@@ -198,39 +197,30 @@ func (client *libvirtClient) CreateDomain(input CreateDomainInput) error {
 	if err != nil {
 		return fmt.Errorf("Error retrieving host architecture: %s", err)
 	}
-	// Both "s390" and "s390x" are linux kernel architectures for Linux on IBM z Systems, and they are for 31-bit and 64-bit respectively.
-	// PowerPC architectures require this support as well
-	if strings.HasPrefix(arch, "s390") || strings.HasPrefix(arch, "ppc64") {
-		if input.Ignition != nil {
-			if err := setIgnitionWithConfigDrive(&domainDef, client, input.Ignition, input.KubeClient, input.MachineNamespace, input.IgnitionVolumeName); err != nil {
-				return err
-			}
+
+	if input.Ignition != nil {
+		if err := setIgnition(&domainDef, client, input.Ignition, input.KubeClient, input.MachineNamespace, input.IgnitionVolumeName, arch); err != nil {
+			return err
+		}
+	} else if input.IgnKey != "" {
+		ignVolume, err := client.getVolume(input.IgnKey)
+		if err != nil {
+			return fmt.Errorf("error getting ignition volume: %v", err)
+		}
+		ignVolumePath, err := ignVolume.GetPath()
+		if err != nil {
+			return fmt.Errorf("error getting ignition volume path: %v", err)
 		}
-	} else {
-		if input.Ignition != nil {
-			if err := setIgnition(&domainDef, client, input.Ignition, input.KubeClient, input.MachineNamespace, input.IgnitionVolumeName); err != nil {
-				return err
-			}
-		} else if input.IgnKey != "" {
-			ignVolume, err := client.getVolume(input.IgnKey)
-			if err != nil {
-				return fmt.Errorf("error getting ignition volume: %v", err)
-			}
-			ignVolumePath, err := ignVolume.GetPath()
-			if err != nil {
-				return fmt.Errorf("error getting ignition volume path: %v", err)
-			}
 
-			if err := setCoreOSIgnition(&domainDef, ignVolumePath); err != nil {
-				return err
-			}
-		} else if input.CloudInit != nil {
-			if err := setCloudInit(&domainDef, client, input.CloudInit, input.KubeClient, input.MachineNamespace, input.CloudInitVolumeName, input.DomainName); err != nil {
-				return err
-			}
-		} else {
-			return fmt.Errorf("machine does not has a IgnKey nor CloudInit value")
+		if err := setCoreOSIgnition(&domainDef, ignVolumePath, arch); err != nil {
+			return err
+		}
+	} else if input.CloudInit != nil {
+		if err := setCloudInit(&domainDef, client, input.CloudInit, input.KubeClient, input.MachineNamespace, input.CloudInitVolumeName, input.DomainName); err != nil {
+			return err
 		}
+	} else {
+		return fmt.Errorf("machine does not has a IgnKey nor CloudInit value")
 	}
 
 	glog.Info("Set up network interface")

pkg/cloud/libvirt/client/domain.go

@@ -219,20 +219,48 @@ func newDomainDefForConnection(virConn *libvirt.Connect) (libvirtxml.Domain, err
 	return d, nil
 }
 
-func setCoreOSIgnition(domainDef *libvirtxml.Domain, ignKey string) error {
+func setCoreOSIgnition(domainDef *libvirtxml.Domain, ignKey string, arch string) error {
 	if ignKey == "" {
 		return fmt.Errorf("error setting coreos ignition, ignKey is empty")
 	}
-	domainDef.QEMUCommandline = &libvirtxml.DomainQEMUCommandline{
-		Args: []libvirtxml.DomainQEMUCommandlineArg{
-			{
-				// https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
-				Value: "-fw_cfg",
+	if strings.HasPrefix(arch, "s390") || strings.HasPrefix(arch, "ppc64") {
+		// System Z and PowerPC do not support the Firmware Configuration
+		// device. After a discussion about the best way to support a similar
+		// method for qemu in https://github.com/coreos/ignition/issues/928,
+		// decided on creating a virtio-blk device with a serial of ignition
+		// which contains the ignition config and have ignition support for
+		// reading from the device which landed in https://github.com/coreos/ignition/pull/936
+		igndisk := libvirtxml.DomainDisk{
+			Device: "disk",
+			Source: &libvirtxml.DomainDiskSource{
+				File: &libvirtxml.DomainDiskSourceFile{
+					File: ignKey,
+				},
 			},
-			{
-				Value: fmt.Sprintf("name=opt/com.coreos/config,file=%s", ignKey),
+			Target: &libvirtxml.DomainDiskTarget{
+				Dev: "vdb",
+				Bus: "virtio",
 			},
-		},
+			Driver: &libvirtxml.DomainDiskDriver{
+				Name: "qemu",
+				Type: "raw",
+			},
+			ReadOnly: &libvirtxml.DomainDiskReadOnly{},
+			Serial:   "ignition",
+		}
+		domainDef.Devices.Disks = append(domainDef.Devices.Disks, igndisk)
+	} else {
+		domainDef.QEMUCommandline = &libvirtxml.DomainQEMUCommandline{
+			Args: []libvirtxml.DomainQEMUCommandlineArg{
+				{
+					// https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
+					Value: "-fw_cfg",
+				},
+				{
+					Value: fmt.Sprintf("name=opt/com.coreos/config,file=%s", ignKey),
+				},
+			},
+		}
 	}
 	return nil
 }

pkg/cloud/libvirt/client/domain_test.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"runtime"
 	"testing"
 
 	libvirtxml "github.com/libvirt/libvirt-go-xml"
@@ -28,7 +29,7 @@ TestCases:
 	for i, tc := range testCases {
 		domainDef := libvirtxml.Domain{}
 
-		err := setCoreOSIgnition(&domainDef, tc.ignKey)
+		err := setCoreOSIgnition(&domainDef, tc.ignKey, runtime.GOARCH)
 		// if err, verify it returns expected error
 		if err != nil {
 			if err.Error() != tc.errorMessage {

pkg/cloud/libvirt/client/ignition.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -15,7 +16,7 @@ import (
 	providerconfigv1 "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1beta1"
 )
 
-func setIgnition(domainDef *libvirtxml.Domain, client *libvirtClient, ignition *providerconfigv1.Ignition, kubeClient kubernetes.Interface, machineNamespace, volumeName string) error {
+func setIgnition(domainDef *libvirtxml.Domain, client *libvirtClient, ignition *providerconfigv1.Ignition, kubeClient kubernetes.Interface, machineNamespace, volumeName string, arch string) error {
 	glog.Info("Creating ignition file")
 	ignitionDef := newIgnitionDef()
 
@@ -43,16 +44,44 @@ func setIgnition(domainDef *libvirtxml.Domain, client *libvirtClient, ignition *
 		return err
 	}
 
-	domainDef.QEMUCommandline = &libvirtxml.DomainQEMUCommandline{
-		Args: []libvirtxml.DomainQEMUCommandlineArg{
-			{
-				// https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
-				Value: "-fw_cfg",
+	if strings.HasPrefix(arch, "s390") || strings.HasPrefix(arch, "ppc64") {
+		// System Z and PowerPC do not support the Firmware Configuration
+		// device. After a discussion about the best way to support a similar
+		// method for qemu in https://github.com/coreos/ignition/issues/928,
+		// decided on creating a virtio-blk device with a serial of ignition
+		// which contains the ignition config and have ignition support for
+		// reading from the device which landed in https://github.com/coreos/ignition/pull/936
+		igndisk := libvirtxml.DomainDisk{
+			Device: "disk",
+			Source: &libvirtxml.DomainDiskSource{
+				File: &libvirtxml.DomainDiskSourceFile{
+					File: ignitionVolumeName,
+				},
 			},
-			{
-				Value: fmt.Sprintf("name=opt/com.coreos/config,file=%s", ignitionVolumeName),
+			Target: &libvirtxml.DomainDiskTarget{
+				Dev: "vdb",
+				Bus: "virtio",
 			},
-		},
+			Driver: &libvirtxml.DomainDiskDriver{
+				Name: "qemu",
+				Type: "raw",
+			},
+			ReadOnly: &libvirtxml.DomainDiskReadOnly{},
+			Serial:   "ignition",
+		}
+		domainDef.Devices.Disks = append(domainDef.Devices.Disks, igndisk)
+	} else {
+		domainDef.QEMUCommandline = &libvirtxml.DomainQEMUCommandline{
+			Args: []libvirtxml.DomainQEMUCommandlineArg{
+				{
+					// https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
+					Value: "-fw_cfg",
+				},
+				{
+					Value: fmt.Sprintf("name=opt/com.coreos/config,file=%s", ignitionVolumeName),
+				},
+			},
+		}
 	}
 	return nil
 }