Skip to content

Commit

Permalink
Use virtio disk instead of config drive for injecting ignition for s3…
Browse files Browse the repository at this point in the history 
…90x/ppc64le

Similar to dmacvicar/terraform-provider-libvirt#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
  • Loading branch information
@Prashanth684
Prashanth684 committed Mar 24, 2020
1 parent 2336783 commit fcb260a
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 217 deletions.
52 changes: 21 additions & 31 deletions pkg/cloud/libvirt/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package client
import (
"encoding/xml"
"fmt"
"strings"

"github.com/golang/glog"
libvirt "github.com/libvirt/libvirt-go"
Expand Down Expand Up @@ -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")
Expand Down
168 changes: 0 additions & 168 deletions pkg/cloud/libvirt/client/configdrive_ignition.go
View file

This file was deleted.

46 changes: 37 additions & 9 deletions pkg/cloud/libvirt/client/domain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
47 changes: 38 additions & 9 deletions pkg/cloud/libvirt/client/ignition.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"io/ioutil"
"os"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -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()

Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit fcb260a

Please sign in to comment.