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/domain_test.go b/pkg/cloud/libvirt/client/domain_test.go index 433da1323..ba0320407 100644 --- a/pkg/cloud/libvirt/client/domain_test.go +++ b/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 { 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 }