Skip to content

Commit

Permalink
kola: add support for primaryDisk key
Browse files Browse the repository at this point in the history
Just like we have `additionalDisks`, add a new `primaryDisk` key which
takes the same "diskspec" format. This immediately unlocks all the same
knobs available to additional disks to the primary disk itself from
external tests.

E.g. with this, we can now migrate multipath tests to external tests
that use `primaryDisk: 20G:mpath`.

My immediate motivation for this however is to be able to use the
new `512e` disk option from an external test as part of fixing
OCPBUGS-35410.
  • Loading branch information
jlebon committed Jun 20, 2024
1 parent 3330a81 commit 4f207de
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 24 deletions.
4 changes: 3 additions & 1 deletion docs/kola/external-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Here's an example `kola.json`:
"platforms": "qemu",
"tags": "sometagname needs-internet skip-base-checks othertag",
"requiredTag": "special",
"primaryDisk": ["20G:mpath"],
"additionalDisks": [ "5G" ],
"minMemory": 4096,
"minDisk": 15,
Expand Down Expand Up @@ -229,7 +230,8 @@ If a test has a `requiredTag`, it is run only if the required tag is specified.
In the example above, the test would only run if `--tag special` was provided.

The `additionalDisks` key has the same semantics as the `--add-disk` argument
to `qemuexec`. It is currently only supported on `qemu`.
to `qemuexec`. It is currently only supported on `qemu`. The `primaryDisk` key
also supports the same syntax and controls the primary boot disk.

The `injectContainer` boolean if set will cause the framework to inject
the ostree base image container into the target system; the path can be
Expand Down
3 changes: 3 additions & 0 deletions mantle/kola/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,7 @@ type externalTestMeta struct {
Tags string `json:"tags,omitempty" yaml:"tags,omitempty"`
RequiredTag string `json:"requiredTag,omitempty" yaml:"requiredTag,omitempty"`
AdditionalDisks []string `json:"additionalDisks,omitempty" yaml:"additionalDisks,omitempty"`
PrimaryDisk string `json:"primaryDisk,omitempty" yaml:"primaryDisk,omitempty"`
InjectContainer bool `json:"injectContainer,omitempty" yaml:"injectContainer,omitempty"`
MinMemory int `json:"minMemory,omitempty" yaml:"minMemory,omitempty"`
MinDiskSize int `json:"minDisk,omitempty" yaml:"minDisk,omitempty"`
Expand Down Expand Up @@ -1233,6 +1234,7 @@ ExecStart=%s
Tags: []string{"external"},

AdditionalDisks: targetMeta.AdditionalDisks,
PrimaryDisk: targetMeta.PrimaryDisk,
InjectContainer: targetMeta.InjectContainer,
MinMemory: targetMeta.MinMemory,
MinDiskSize: targetMeta.MinDiskSize,
Expand Down Expand Up @@ -1752,6 +1754,7 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig

options := platform.MachineOptions{
MultiPathDisk: t.MultiPathDisk,
PrimaryDisk: t.PrimaryDisk,
AdditionalDisks: t.AdditionalDisks,
MinMemory: t.MinMemory,
MinDiskSize: t.MinDiskSize,
Expand Down
9 changes: 7 additions & 2 deletions mantle/kola/register/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,26 @@ type Test struct {
RequiredTag string // if specified, test is filtered by default unless tag is provided -- defaults to none
Description string // test description

// Whether the primary disk is multipathed.
// Whether the primary disk is multipathed. Deprecated in favour of PrimaryDisk.
MultiPathDisk bool

// Sizes of additional empty disks to attach to the node, followed by
// comma-separated list of optional options (e.g. ["1G",
// "5G:mpath,foo,bar"]) -- defaults to none.
AdditionalDisks []string

// Size of primary disk to attach to the node, followed by
// comma-separated list of optional options (e.g. "20G:mpath"]).
PrimaryDisk string

// InjectContainer will cause the ostree base image to be injected into the target
InjectContainer bool

// Minimum amount of memory in MB required for test.
MinMemory int

// Minimum amount of primary disk in GB required for test.
// Minimum amount of primary disk in GB required for test. Deprecated in favour
// of PrimaryDisk.
MinDiskSize int

// Additional amount of NICs required for test.
Expand Down
41 changes: 20 additions & 21 deletions mantle/platform/machine/qemu/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,34 +140,33 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
builder.MemoryMiB = 4096 // SE needs at least 4GB
}

channel := "virtio"
var primaryDisk platform.Disk
if options.PrimaryDisk != "" {
var diskp *platform.Disk
if diskp, err = platform.ParseDisk(options.PrimaryDisk); err != nil {
return nil, errors.Wrapf(err, "parsing primary disk spec '%s'", options.PrimaryDisk)
}
primaryDisk = *diskp
}

if qc.flight.opts.Nvme || options.Nvme {
channel = "nvme"
primaryDisk.Channel = "nvme"
}
sectorSize := 0
logicalSectorSize := 0
if qc.flight.opts.Native4k {
sectorSize = 4096
primaryDisk.SectorSize = 4096
} else if qc.flight.opts.Disk512e {
sectorSize = 4096
logicalSectorSize = 512
primaryDisk.SectorSize = 4096
primaryDisk.LogicalSectorSize = 512
}
multiPathDisk := options.MultiPathDisk || qc.flight.opts.MultiPathDisk
var diskSize string
if options.MinDiskSize > 0 {
diskSize = fmt.Sprintf("%dG", options.MinDiskSize)
} else {
diskSize = qc.flight.opts.DiskSize
if options.MultiPathDisk || qc.flight.opts.MultiPathDisk {
primaryDisk.MultiPathDisk = true
}
primaryDisk := platform.Disk{
BackingFile: qc.flight.opts.DiskImage,
Channel: channel,
Size: diskSize,
SectorSize: sectorSize,
LogicalSectorSize: logicalSectorSize,
MultiPathDisk: multiPathDisk,
if options.MinDiskSize > 0 {
primaryDisk.Size = fmt.Sprintf("%dG", options.MinDiskSize)
} else if qc.flight.opts.DiskSize != "" {
primaryDisk.Size = qc.flight.opts.DiskSize
}

primaryDisk.BackingFile = qc.flight.opts.DiskImage
if options.OverrideBackingFile != "" {
primaryDisk.BackingFile = options.OverrideBackingFile
}
Expand Down
1 change: 1 addition & 0 deletions mantle/platform/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ type Flight interface {

type MachineOptions struct {
MultiPathDisk bool
PrimaryDisk string
AdditionalDisks []string
MinMemory int
MinDiskSize int
Expand Down

0 comments on commit 4f207de

Please sign in to comment.