From fcb260a04e9711dccad4429e3d94dabbafe916b4 Mon Sep 17 00:00:00 2001 From: Prashanth Sundararaman Date: Tue, 24 Mar 2020 10:54:22 -0400 Subject: [PATCH] Use virtio disk instead of config drive for injecting ignition for s390x/ppc64le Similar to https://github.com/dmacvicar/terraform-provider-libvirt/pull/718 The method of mimicking 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. Reference PR which supports ignition fetching through virtio-blk for QEMU: coreos/ignition#936 --- pkg/cloud/libvirt/client/client.go | 52 +++--- .../libvirt/client/configdrive_ignition.go | 168 ------------------ pkg/cloud/libvirt/client/domain.go | 46 ++++- pkg/cloud/libvirt/client/ignition.go | 47 ++++- 4 files changed, 96 insertions(+), 217 deletions(-) delete mode 100644 pkg/cloud/libvirt/client/configdrive_ignition.go diff --git a/pkg/cloud/libvirt/client/client.go b/pkg/cloud/libvirt/client/client.go index 314a7a9a7..f64ffc487 100644 --- a/pkg/cloud/libvirt/client/client.go +++ b/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") diff --git a/pkg/cloud/libvirt/client/configdrive_ignition.go b/pkg/cloud/libvirt/client/configdrive_ignition.go deleted file mode 100644 index e5a0b301a..000000000 --- a/pkg/cloud/libvirt/client/configdrive_ignition.go +++ /dev/null @@ -1,168 +0,0 @@ -package client - -import ( - "fmt" - "github.com/golang/glog" - libvirt "github.com/libvirt/libvirt-go" - libvirtxml "github.com/libvirt/libvirt-go-xml" - providerconfigv1 "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1beta1" - "io" - "io/ioutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - "os" - "os/exec" - "path/filepath" -) - -var execCommand = exec.Command - -// Currently used for s390 and PowerPC arches -func setIgnitionWithConfigDrive(domainDef *libvirtxml.Domain, client *libvirtClient, ignition *providerconfigv1.Ignition, kubeClient kubernetes.Interface, machineNamespace, volumeName string) error { - glog.Infof("Creating ignition file with config drive") - ignitionDef := newIgnitionDef() - - if ignition.UserDataSecret == "" { - return fmt.Errorf("ignition.userDataSecret not set") - } - - secret, err := kubeClient.CoreV1().Secrets(machineNamespace).Get(ignition.UserDataSecret, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("can not retrieve user data secret '%v/%v' when constructing cloud init volume: %v", machineNamespace, ignition.UserDataSecret, err) - } - userDataSecret, ok := secret.Data["userData"] - if !ok { - return fmt.Errorf("can not retrieve user data secret '%v/%v' when constructing cloud init volume: key 'userData' not found in the secret", machineNamespace, ignition.UserDataSecret) - } - - ignitionDef.Name = volumeName - ignitionDef.PoolName = client.poolName - ignitionDef.Content = string(userDataSecret) - - glog.Infof("Ignition: %+v", ignitionDef) - - tmpDir, err := ioutil.TempDir("", "config-drive") - if err != nil { - return fmt.Errorf("Failed to create config-drive directory: %v", err) - } - defer func() { - if err = os.RemoveAll(tmpDir); err != nil { - glog.Errorf("Error while removing config-drive directory: %v", err) - } - }() - - ignitionVolumeName, err := ignitionDef.createAndUploadIso(tmpDir, client) - if err != nil { - return fmt.Errorf("Error create and upload iso file: %s", err) - } - - glog.Infof("Calling newDiskForConfigDrive for coreos_ignition ") - disk, err := newDiskForConfigDrive(client.connection, ignitionVolumeName) - if err != nil { - return err - } - - domainDef.Devices.Disks = append(domainDef.Devices.Disks, disk) - - return nil -} - -func (ign *defIgnition) createAndUploadIso(tmpDir string, client *libvirtClient) (string, error) { - ignFile, err := ign.createFile() - if err != nil { - return "", err - } - defer func() { - // Remove the tmp ignition file - if err = os.Remove(ignFile); err != nil { - glog.Infof("Error while removing tmp Ignition file: %s", err) - } - }() - - isoVolumeFile, err := createIgnitionISO(tmpDir, ignFile) - if err != nil { - return "", fmt.Errorf("Error generate iso file: %s", err) - } - - img, err := newImage(isoVolumeFile) - if err != nil { - return "", err - } - - size, err := img.size() - if err != nil { - return "", err - } - - volumeDef := newDefVolume(ign.Name) - volumeDef.Capacity.Unit = "B" - volumeDef.Capacity.Value = size - volumeDef.Target.Format.Type = "raw" - - return uploadVolume(ign.PoolName, client, volumeDef, img) -} - -func newDiskForConfigDrive(virConn *libvirt.Connect, volumeKey string) (libvirtxml.DomainDisk, error) { - disk := libvirtxml.DomainDisk{ - Device: "cdrom", - Target: &libvirtxml.DomainDiskTarget{ - // s390 and ppc platforms doesn't support IDE controller, it shoule be virtio controller - Dev: "vdb", - Bus: "scsi", - }, - Driver: &libvirtxml.DomainDiskDriver{ - Name: "qemu", - Type: "raw", - }, - } - diskVolume, err := virConn.LookupStorageVolByKey(volumeKey) - if err != nil { - return disk, fmt.Errorf("Can't retrieve volume %s: %v", volumeKey, err) - } - diskVolumeFile, err := diskVolume.GetPath() - if err != nil { - return disk, fmt.Errorf("Error retrieving volume file: %s", err) - } - - disk.Source = &libvirtxml.DomainDiskSource{ - File: &libvirtxml.DomainDiskSourceFile{ - File: diskVolumeFile, - }, - } - - return disk, nil -} - -// createIgnitionISO create config drive iso with ignition-config file -func createIgnitionISO(tmpDir string, ignPath string) (string, error) { - glog.Infof("The ignPath %s", ignPath) - //get the ignition contentt - userData, err := os.Open(ignPath) - if err != nil { - return "", fmt.Errorf("Error get the ignition content : %s", err) - } - defer userData.Close() - newDestinationPath, err := os.Create(filepath.Join(tmpDir, "user_data")) - if _, err := io.Copy(newDestinationPath, userData); err != nil { - return "", fmt.Errorf("Error copy the ignitio content to newDestinationPath : %s", err) - } - configDrivePath := filepath.Join(tmpDir, "config.iso") - cmd := exec.Command( - "mkisofs", - "-output", - configDrivePath, - "-volid", - "config-2", - "-root", - "openstack/latest", - "-joliet", - "-rock", - newDestinationPath.Name()) - glog.Infof("Executing command: %+v", cmd) - - if err := cmd.Run(); err != nil { - return "", fmt.Errorf("Error while starting the creation of ignition's ISO image: %s", err) - } - glog.Infof("ISO created at %s", configDrivePath) - return configDrivePath, nil -} diff --git a/pkg/cloud/libvirt/client/domain.go b/pkg/cloud/libvirt/client/domain.go index 39eda699a..4c7d65618 100644 --- a/pkg/cloud/libvirt/client/domain.go +++ b/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 } diff --git a/pkg/cloud/libvirt/client/ignition.go b/pkg/cloud/libvirt/client/ignition.go index 5cd6f8ba2..6c71c9efc 100644 --- a/pkg/cloud/libvirt/client/ignition.go +++ b/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 }