From 889c3a17e550e7fe598380927026925d3da5c369 Mon Sep 17 00:00:00 2001 From: Prashanth Sundararaman Date: Wed, 18 Mar 2020 18:21:27 -0400 Subject: [PATCH] Use virtio-blk instead of disk for injecting ignition config for s390x/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 (https://github.com/coreos/ignition/issues/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. --- examples/v0.12/coreos/README.md | 2 +- libvirt/coreos_ignition_def.go | 58 +++---------------- libvirt/domain.go | 60 ++++++++++---------- libvirt/resource_libvirt_domain.go | 9 +-- website/docs/r/coreos_ignition.html.markdown | 7 +-- 5 files changed, 46 insertions(+), 90 deletions(-) diff --git a/examples/v0.12/coreos/README.md b/examples/v0.12/coreos/README.md index 65260f378..5b79890e0 100644 --- 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 index 34dedd3ed..87ab39946 100644 --- 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 index e2aacf770..cb309d0d7 100644 --- 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 index fb15c5749..ffeac872a 100644 --- 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 index ba3ff3825..ed9a12c71 100644 --- 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