Use virtio-blk instead of disk for injecting ignition config for s390…

…x/ppc64le The method of mimicing what Openstack does for injecting ignition config works for images which have the provider as Openstack because ignition recognizes the platform and knows it has to get the ignition config from the config drive. For QEMU images, ignition supports getting the config from the firmware config device which is not supported by ppc64 and s390x. The workaround we have used thus far is to use the Openstack image on the QEMU platform but have this provider create the iso containing the ignition config. There was a discussion in ignition (coreos/ignition#928) to have a more QEMU based method of injecting ignition config and it was decided to use a virtio-blk device with a serial of `ignition` which ignition can recognize. This was mainly because with external devices, it is hard to tell if there is an issue with the device or if the kernel has not detected it yet if it has a long discovery phase. This PR reverts the ISO method used by Openstack and just creates a virtio-blk device through the QEMU command line options.
dmacvicar · Mar 23, 2020 · 889c3a1 · 889c3a1
1 parent c2b6ef5
commit 889c3a1
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 90 deletions.
diff --git a/examples/v0.12/coreos/README.md b/examples/v0.12/coreos/README.md
@@ -11,7 +11,7 @@ with their own Ignition configuration
 
 
 ### Supported architectures
-This example will work on i686 (x86), x86\_64 (AMD64), aarch64 (ARM64), s390x (IBM Z) and IBM PowerPC. On i686, x86\_64, and aarch64, the Terraform provider uses QEMU's firmware configuration (fw\_cfg) device to pass the Ignition config through to the Ignition instance running in the virtual machine. In order for this to work, Ignition needs to be run with `--provider=qemu`. Unfortunately, QEMU doesn't support the firmware configuration device for s390x and PowerPC guests, so the Terraform provider falls back to using an OpenStack config-drive instead. The config-drive will automatically be created by the provider, provided that `mkisofs` is installed in the path, so no changes are necessary to the Terraform templates. Ignition needs to be run with `--provider=openstack` when inside of an s390x or PowerPc guest in order to find the config-drive.
+This example will work on i686 (x86), x86\_64 (AMD64), aarch64 (ARM64), s390x (IBM Z) and IBM PowerPC. On i686, x86\_64, and aarch64, the Terraform provider uses QEMU's firmware configuration (fw\_cfg) device to pass the Ignition config through to the Ignition instance running in the virtual machine. In order for this to work, Ignition needs to be run with `--provider=qemu`. Unfortunately, QEMU doesn't support the firmware configuration device for s390x and PowerPC guests, so alternatively, a virtio-blk device is created with a serial of `ignition` which ignition detects and reads.
 
 
 ### Using the QEMU Guest Agent

diff --git a/libvirt/coreos_ignition_def.go b/libvirt/coreos_ignition_def.go
@@ -8,8 +8,6 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
-	"os/exec"
-	"path/filepath"
 	"strings"
 
 	libvirt "github.com/libvirt/libvirt-go"
@@ -52,55 +50,17 @@ func (ign *defIgnition) CreateAndUpload(client *Client) (string, error) {
 	volumeDef := newDefVolume()
 	volumeDef.Name = ign.Name
 
-	tmpDir, err := ioutil.TempDir("", "ignition")
+	ignFile, err := ign.createFile()
 	if err != nil {
-		return "", fmt.Errorf("Failed to create Ignition directory: %v", err)
+		return "", err
 	}
 	defer func() {
-		if err = os.RemoveAll(tmpDir); err != nil {
-			log.Printf("Error while removing Ignition directory: %v", err)
+		if err = os.Remove(ignFile); err != nil {
+			log.Printf("Error while removing tmp Ignition file: %v", err)
 		}
 	}()
 
-	ignPath, err := ign.createFile(tmpDir)
-	if err != nil {
-		return "", err
-	}
-
-	arch, err := getHostArchitecture(client.libvirt)
-	if err != nil {
-		return "", fmt.Errorf("Error retrieving host architecture: %s", err)
-	}
-
-	var userdataPath string
-	switch arch {
-	case "i686", "x86_64", "aarch64":
-		userdataPath = ignPath
-	case "s390", "s390x", "ppc64", "ppc64le":
-		configDrivePath := filepath.Join(tmpDir, "config.iso")
-		cmd := exec.Command(
-			"mkisofs",
-			"-output",
-			configDrivePath,
-			"-volid",
-			"config-2",
-			"-root",
-			"openstack/latest",
-			"-joliet",
-			"-rock",
-			ignPath)
-
-		log.Printf("Executing command: %+v", cmd)
-		if err = cmd.Run(); err != nil {
-			return "", fmt.Errorf("Failed to create Config Drive ISO image: %v", err)
-		}
-
-		userdataPath = configDrivePath
-	default:
-		return "", fmt.Errorf("Ignition not supported on %q", arch)
-	}
-
-	img, err := newImage(userdataPath)
+	img, err := newImage(ignFile)
 	if err != nil {
 		return "", err
 	}
@@ -156,12 +116,12 @@ func getIgnitionVolumeKeyFromTerraformID(id string) (string, error) {
 }
 
 // Dumps the Ignition object - either generated by Terraform or supplied as a file -
-// to a userdata Ignition file in the provided directory
-func (ign *defIgnition) createFile(dir string) (string, error) {
+// to a temporary ignition file
+func (ign *defIgnition) createFile() (string, error) {
 	log.Print("Creating Ignition temporary file")
-	tempFile, err := os.Create(filepath.Join(dir, "user_data"))
+	tempFile, err := ioutil.TempFile("", ign.Name)
 	if err != nil {
-		return "", fmt.Errorf("Error creating userdata file: %v", err)
+		return "", fmt.Errorf("Error creating tmp file: %v", err)
 	}
 	defer tempFile.Close()
 

diff --git a/libvirt/domain.go b/libvirt/domain.go
@@ -198,26 +198,14 @@ func domainGetIfacesInfo(domain libvirt.Domain, rd *schema.ResourceData) ([]libv
 	return interfaces, nil
 }
 
-func newDiskForCloudInit(virConn *libvirt.Connect, volumeKey string, arch string) (libvirtxml.DomainDisk, error) {
-	var target *libvirtxml.DomainDiskTarget
-	switch arch {
-	case "s390", "s390x", "ppc64", "ppc64le":
-		target = &libvirtxml.DomainDiskTarget{
-			// s390 and ppc64 platforms don't support IDE controllers
-			Dev: "vdb",
-			Bus: "scsi",
-		}
-	default:
-		target = &libvirtxml.DomainDiskTarget{
+func newDiskForCloudInit(virConn *libvirt.Connect, volumeKey string) (libvirtxml.DomainDisk, error) {
+	disk := libvirtxml.DomainDisk{
+		Device: "cdrom",
+		Target: &libvirtxml.DomainDiskTarget{
 			// Last device letter possible with a single IDE controller on i440FX
 			Dev: "hdd",
 			Bus: "ide",
-		}
-	}
-
-	disk := libvirtxml.DomainDisk{
-		Device: "cdrom",
-		Target: target,
+		},
 		Driver: &libvirtxml.DomainDiskDriver{
 			Name: "qemu",
 			Type: "raw",
@@ -270,17 +258,31 @@ func setCoreOSIgnition(d *schema.ResourceData, domainDef *libvirtxml.Domain, vir
 				}
 			}
 		case "s390", "s390x", "ppc64", "ppc64le":
-			// System Z and PowerPC do not support any of the same pass-through
-			// mechanisms as Ignition. As a temporary workaround, the OpenStack
-			// Config Drive can be used instead. The Ignition volume already
-			// contains a Config Drive at this point.
-
-			disk, err := newDiskForCloudInit(virConn, ignitionKey, arch)
-			if err != nil {
-				return err
+			// 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: ignitionKey,
+					},
+				},
+				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, disk)
+			domainDef.Devices.Disks = append(domainDef.Devices.Disks, igndisk)
 		default:
 			return fmt.Errorf("Ignition not supported on %q", arch)
 		}
@@ -655,13 +657,13 @@ func setFilesystems(d *schema.ResourceData, domainDef *libvirtxml.Domain) error
 	return nil
 }
 
-func setCloudinit(d *schema.ResourceData, domainDef *libvirtxml.Domain, virConn *libvirt.Connect, arch string) error {
+func setCloudinit(d *schema.ResourceData, domainDef *libvirtxml.Domain, virConn *libvirt.Connect) error {
 	if cloudinit, ok := d.GetOk("cloudinit"); ok {
 		cloudinitID, err := getCloudInitVolumeKeyFromTerraformID(cloudinit.(string))
 		if err != nil {
 			return err
 		}
-		disk, err := newDiskForCloudInit(virConn, cloudinitID, arch)
+		disk, err := newDiskForCloudInit(virConn, cloudinitID)
 		if err != nil {
 			return err
 		}

diff --git a/libvirt/resource_libvirt_domain.go b/libvirt/resource_libvirt_domain.go
@@ -497,7 +497,7 @@ func resourceLibvirtDomainCreate(d *schema.ResourceData, meta interface{}) error
 		return err
 	}
 
-	if err := setCloudinit(d, &domainDef, virConn, arch); err != nil {
+	if err := setCloudinit(d, &domainDef, virConn); err != nil {
 		return err
 	}
 
@@ -652,12 +652,7 @@ func resourceLibvirtDomainUpdate(d *schema.ResourceData, meta interface{}) error
 			return err
 		}
 
-		arch, err := getHostArchitecture(virConn)
-		if err != nil {
-			return fmt.Errorf("Error retrieving host architecture: %s", err)
-		}
-
-		disk, err := newDiskForCloudInit(virConn, cloudinitID, arch)
+		disk, err := newDiskForCloudInit(virConn, cloudinitID)
 		if err != nil {
 			return err
 		}

diff --git a/website/docs/r/coreos_ignition.html.markdown b/website/docs/r/coreos_ignition.html.markdown
@@ -16,10 +16,9 @@ a CoreOS Domain during first boot.
 
 ~> **Note:** On the i686 (x86), x86\_64 (AMD64), and aarch64 (ARM64) platforms,
 the QEMU firmware config device (`-fw_cfg`) is used to pass Ignition configs
-into the guests. On the s390x (IBM Z) platform however, the QEMU firmware
-config device is not supported. As alternative, the OpenStack config-drive
-mechanism is instead used. The `mkisofs` utility must be installed into the
-path and Ignition must be run with the `--provider=openstack` flag.
+into the guests. On the s390x (IBM Z) and PowerPC platforms however, the QEMU firmware
+config device is not supported. As an alternative, a virtio-blk device is created with a serial
+of `ignition` which ignition recognizes and reads the config from the device
 
 ## Example Usage