Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert the vm.sh script into a go binary #1164

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
11 changes: 10 additions & 1 deletion cluster-provision/centos9/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Build the vmcli Go CLI
FROM docker.io/library/golang:1.22 AS vmcli-builder

FROM quay.io/kubevirtci/fedora@sha256:e3a6087f62f288571db14defb7e0e10ad7fe6f973f567b0488d3aac5e927035a
WORKDIR /app

COPY vmcli/ ./
RUN CGO_ENABLED=0 GOOS=linux go build -o ./_bin/vmcli ./cmd/cli

# Prepare the VM runner
FROM quay.io/kubevirtci/fedora@sha256:e3a6087f62f288571db14defb7e0e10ad7fe6f973f567b0488d3aac5e927035a AS runner

ARG centos_version

Expand Down Expand Up @@ -31,3 +39,4 @@ RUN curl -L -o /initrd.img http://mirror.stream.centos.org/9-stream/BaseOS/x86_6
RUN curl -L -o /vmlinuz http://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/images/pxeboot/vmlinuz

COPY scripts/* /
COPY --from=vmcli-builder /app/_bin/vmcli /
7 changes: 7 additions & 0 deletions cluster-provision/centos9/vmcli/cmd/cli/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package main

import "kubevirt.io/kubevirtci/cluster-provision/centos9/vmcli/cmd"

func main() {
cmd.Execute()
}
54 changes: 54 additions & 0 deletions cluster-provision/centos9/vmcli/cmd/qemu/img.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package qemu

import (
"encoding/json"
"fmt"
"os"
"os/exec"
"strconv"
)

// Wrapper around the output of the "qemu-img info" command
type qemuImgInfo struct {
// The virtual size of the disk image
VirtualSize uint64 `json:"virtual-size"`
}

// Creates a virtual disk image
func CreateDisk(path string, format string, size uint64) error {
cmd := exec.Command("qemu-img", "create", "-f", format, path, strconv.FormatUint(size, 10))
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not also the StdOut?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know if we wanted to show the output of those commands, but in hindsight more verbosity can't hurt.


return cmd.Run()
}

// Creates a virtual disk image that is backed by another disk image
func CreateDiskWithBackingFile(path string, format string, size uint64, backingPath string, backingFormat string) error {
cmd := exec.Command("qemu-img", "create", "-f", format, "-o", fmt.Sprintf("backing_file=%s", backingPath), "-F", backingFormat, path, strconv.FormatUint(size, 10))
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

return cmd.Run()
}

// Parse a JSON qemu-img info object
func ParseDiskInfo(b []byte) (qemuImgInfo, error) {
var info qemuImgInfo
err := json.Unmarshal(b, &info)

return info, err
}

// Get information about the specified disk image on the file system
func GetDiskInfo(diskPath string) (qemuImgInfo, error) {
cmd := exec.Command("qemu-img", "info", "--output", "json", diskPath)
cmd.Stderr = os.Stderr

out, err := cmd.Output()
if err != nil {
return qemuImgInfo{}, err
}

return ParseDiskInfo(out)
}
67 changes: 67 additions & 0 deletions cluster-provision/centos9/vmcli/cmd/qemu/img_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package qemu_test

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"kubevirt.io/kubevirtci/cluster-provision/centos9/vmcli/cmd/qemu"
)

var _ = Describe("Qemu qemu-img wrapper", func() {
Describe("Parsing the disk information", func() {
When("the disk information is valid", func() {
const sampleQemuImgInfoOutput = `
{
"children": [
{
"name": "file",
"info": {
"children": [
],
"virtual-size": 197120,
"filename": "test",
"format": "file",
"actual-size": 200704,
"format-specific": {
"type": "file",
"data": {
}
},
"dirty-flag": false
}
}
],
"virtual-size": 1048576,
"filename": "test",
"cluster-size": 65536,
"format": "qcow2",
"actual-size": 200704,
"format-specific": {
"type": "qcow2",
"data": {
"compat": "1.1",
"compression-type": "zlib",
"lazy-refcounts": false,
"refcount-bits": 16,
"corrupt": false,
"extended-l2": false
}
},
"dirty-flag": false
}
`
It("returns the correct disk virtual size", func() {
diskInfo, err := qemu.ParseDiskInfo([]byte(sampleQemuImgInfoOutput))
Expect(err).NotTo(HaveOccurred())
Expect(diskInfo.VirtualSize).To(BeNumerically("==", 1048576))
})
})

When("the disk information is invalid", func() {
It("fails", func() {
_, err := qemu.ParseDiskInfo([]byte("{"))
Expect(err).To(HaveOccurred())
})
})
})
})
13 changes: 13 additions & 0 deletions cluster-provision/centos9/vmcli/cmd/qemu/qemu_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package qemu_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestQemu(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Qemu Suite")
}
150 changes: 150 additions & 0 deletions cluster-provision/centos9/vmcli/cmd/qemu/system.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package qemu

import (
"fmt"
"regexp"
"strconv"
"strings"

"github.com/google/uuid"
)

// The name of the QEMU main executable
// TODO: On RHEL and similar, the executable is "qemu-kvm". We'll need to check the distribution and use the correct executable.
const qemuExec = "qemu-system-%s"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is correct for all OSes, in Centos Stream it should be qemu-kvm. I'm wondering how this can work with the current script, maybe there is a symlink

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is actually running in a Fedora 39 container, so this is why it currently works.

Do we want to add a check for the current OS and use qemu-kvm if available instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. I think we should get rid of fedora in the future. It is up to you, either you can a put a comment with a TODO and we can address this in a second PR or you can add it here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to address this in a second PR, so this one doesn't get too big

Copy link
Member

@alicefr alicefr Apr 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to get rid of Fedora in this context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to. But we have converted everything to centos stream. It seems a bit inconsistent to me that we still keep fedora. Aynway, this can be a topic for another PR.


// Thin wrapper around the "qemu-system-x" command
type QemuSystem struct {
// The architecture of the "qemu-system" command to start
Arch string
// The memory to allocate to the guest, wraps the "-m" argument
Memory string
// The number of the CPUs in the SMP system, wraps the "-smp" argument
CpuCount uint64
// The number of NUMA nodes in the system, wraps "-object memory-backend-ram" and "-numa node" arguments
Numa uint64
// Whether to use KVM, wraps the "-enable-kvm" argument
KvmEnabled bool
// Select the CPU model, wraps the "-cpu" argument
CpuModel string
// Select the emulated machine, wraps the "-machine" argument
Machine string
// Set the system UUID, wraps the "-uuid" argument
SystemUuid uuid.UUID
// Start a VNC server on the specified display, wraps the "-vnc" argument
VncServer string
// Redirect the virtual serial port to the specified character device, wraps the "-serial" argument
SerialHostDev string
// Path to the initramfs file, wraps the "-initrd" argument
InitrdPath string
// Path to the kernel to boot, wraps the "-kernel" argument
KernelPath string
// The cmdlines to pass to the booted kernel, wraps the "-append" argument
KernelArgs []string
// Drives to attach to the guest, wraps the "-drive" arguments
Drives []string
// Devices to attach to the guest, wraps the "-device" arguments
Devices []string
// The network backend to configure, wraps the "-netdev" argument
Netdev string
}

// Parse the memory argument into a value and possibly a unit
func (q QemuSystem) ParseMemory() (uint64, string, error) {
regex, err := regexp.Compile(`(\d+)(\w*)`)
if err != nil {
return 0, "", err
}

submatch := regex.FindStringSubmatch(q.Memory)
if len(submatch) < 2 {
return 0, "", fmt.Errorf("Unable to parse the QEMU memory argument %q", q.Memory)
}

val, err := strconv.ParseUint(submatch[1], 10, 64)
if err != nil {
return 0, "", err
}

unit := ""
if len(submatch) >= 3 {
unit = submatch[2]
}

return val, unit, nil
}

// Generate the QEMU arguments for creating the NUMA topology
func (q QemuSystem) GenerateNumaArguments() ([]string, error) {
if q.Numa < 2 {
return []string{}, nil
}

result := []string{}

memoryValue, memoryUnit, err := q.ParseMemory()
if err != nil {
return []string{}, err
}

if q.CpuCount%q.Numa > 0 || memoryValue%q.Numa > 0 {
return []string{}, fmt.Errorf("unable to calculate symmetric NUMA topology with vCPUs:%v Memory:%v NUMA:%v", q.CpuCount, q.Memory, q.Numa)
}

memoryPerNodeValue := memoryValue / q.Numa
memoryPerNode := fmt.Sprintf("%v%v", memoryPerNodeValue, memoryUnit)
cpuPerNode := q.CpuCount / q.Numa

for nodeId := uint64(0); nodeId < q.Numa; nodeId++ {
nodeFirstCpu := nodeId * cpuPerNode
nodeLastCpu := nodeFirstCpu + cpuPerNode - 1

memoryBackendRamArg := fmt.Sprintf("-object memory-backend-ram,size=%v,id=m%v", memoryPerNode, nodeId)
numaNodeArg := fmt.Sprintf("-numa node,nodeid=%v,memdev=m%v,cpus=%v-%v", nodeId, nodeId, nodeFirstCpu, nodeLastCpu)

result = append(result, memoryBackendRamArg, numaNodeArg)
}

return result, nil
}

// Generate the command line to use to start QEMU
func (q QemuSystem) GenerateCmdline() (string, error) {
qemuArgs := []string{
fmt.Sprintf(qemuExec, q.Arch),
"-m", q.Memory,
"-smp", strconv.FormatUint(q.CpuCount, 10),
"-cpu", q.CpuModel,
"-M", q.Machine,
"-uuid", q.SystemUuid.String(),
"-vnc", q.VncServer,
"-serial", q.SerialHostDev,
"-initrd", q.InitrdPath,
"-kernel", q.KernelPath,
"-append", fmt.Sprintf("\"%s\"", strings.TrimSpace(strings.Join(q.KernelArgs, " "))),
"-netdev", q.Netdev,
}

if q.KvmEnabled {
qemuArgs = append(qemuArgs, "-enable-kvm")
}

for _, drive := range q.Drives {
qemuArgs = append(qemuArgs, "-drive", drive)
}

for _, device := range q.Devices {
qemuArgs = append(qemuArgs, "-device", device)
}

numaArgs, err := q.GenerateNumaArguments()
if err != nil {
return "", err
}

if len(numaArgs) > 0 {
qemuArgs = append(qemuArgs, numaArgs...)
}

return strings.Join(qemuArgs, " "), nil
}
Loading