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

Test-Layout s390x #483

Closed
wants to merge 8 commits into from
Closed
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion config/common/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ var (
ErrMountUnitNoFormat = errors.New("format is required if with_mount_unit is true")

// boot device
ErrUnknownBootDeviceLayout = errors.New("layout must be one of: aarch64, ppc64le, x86_64")
ErrUnknownBootDeviceLayout = errors.New("layout must be one of: aarch64, ppc64le, x86_64, s390x-zfcp, s390x-eckd, s390x-virt")
ErrTooFewMirrorDevices = errors.New("mirroring requires at least two devices")
//ErrNoLuksBootDevice = errors.New("device field support layout: s390x-eckd /dev/dasd[a-z] or s390x-zfcp /dev/sd[a-z]")
ErrMirrorNotSupport = errors.New("mirror is not support with luks.device for layout: s390x-zfcp, s390x-eckd")

// partition
ErrWrongPartitionNumber = errors.New("incorrect partition number; a new partition will be created using reserved label")
Expand Down
1 change: 1 addition & 0 deletions config/fcos/v1_6_exp/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type BootDevice struct {

type BootDeviceLuks struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commit 561ce19 should be amended with a724048

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll create a new PR with less clutter on spaces and new line. This PR was not supposed to raised, As i need some assistance is some of the logic in coding which i am not able to meet. I'll work upon.

Discard *bool `yaml:"discard"`
Device string `yaml:"device"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:This change should have been part of the originating commit.

Tang []base.Tang `yaml:"tang"`
Threshold *int `yaml:"threshold"`
Tpm2 *bool `yaml:"tpm2"`
Expand Down
40 changes: 35 additions & 5 deletions config/fcos/v1_6_exp/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package v1_6_exp
import (
"fmt"
"strings"
"regexp"
"strconv"

baseutil "github.com/coreos/butane/base/util"
"github.com/coreos/butane/config/common"
Expand All @@ -29,6 +31,11 @@ import (
"github.com/coreos/vcontext/report"
)

var (
dasdRe = regexp.MustCompile("(/dev/dasd[a-z]$)")
sdRe = regexp.MustCompile("(/dev/sd[a-z]$)")
)

const (
reservedTypeGuid = "8DA63339-0007-60C0-C436-083AC8230908"
biosTypeGuid = "21686148-6449-6E6F-744E-656564454649"
Expand Down Expand Up @@ -109,8 +116,9 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
var r report.Report

// check for high-level features
wantLuks := util.IsTrue(c.BootDevice.Luks.Tpm2) || len(c.BootDevice.Luks.Tang) > 0
wantLuks := util.IsTrue(c.BootDevice.Luks.Tpm2) || len(c.BootDevice.Luks.Tang) > 0
wantMirror := len(c.BootDevice.Mirror.Devices) > 0

if !wantLuks && !wantMirror {
return r
}
Expand All @@ -119,6 +127,8 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
var wantBIOSPart bool
var wantEFIPart bool
var wantPRePPart bool
var wantMBR bool
var wantDasd bool
layout := c.BootDevice.Layout
switch {
case layout == nil || *layout == "x86_64":
Expand All @@ -128,6 +138,13 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
wantEFIPart = true
case *layout == "ppc64le":
wantPRePPart = true
case *layout == "s390x-zfcp":
wantMBR = true
case *layout == "s390x-eckd":
wantDasd = true
case *layout == "s390x-virt":
wantBIOSPart = true
wantEFIPart = true
default:
// should have failed validation
panic("unknown layout")
Expand Down Expand Up @@ -232,12 +249,25 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
rendered.Storage.Filesystems = append(rendered.Storage.Filesystems, bootFilesystem)
}

// encrypted root partition
//encrypted root partition
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: why did we remove a space in the comment it makes it less readable.

if wantLuks {
luksDevice := "/dev/disk/by-partlabel/root"
if wantMirror {
var luksDevice string
dasd := dasdRe.FindString(c.BootDevice.Luks.Device)
sd := sdRe.FindString(c.BootDevice.Luks.Device)
switch {
case wantMBR && len(sd) > 0:
luksDevice = sd + strconv.Itoa(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get a comment explaining the luks device value?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also why strconv.Itoa()? and not "2" and is there a better way so there is not string magic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add the comment explaining the luks device value..

Also why strconv.Itoa()? and not "2" and is there a better way so there is not string magic?
Yes will add, sd + "2"

case wantDasd && len(dasd) > 0:
luksDevice = dasd + strconv.Itoa(2)
case wantMirror:
luksDevice = "/dev/md/md-root"
default:
luksDevice = "/dev/disk/by-partlabel/root"
}
// luksDevice := "/dev/disk/by-partlabel/root"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commented out code is a no no.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove the commented out code.

// if wantMirror {
// luksDevice = "/dev/md/md-root"
// }
clevis, ts2, r2 := translateBootDeviceLuks(c.BootDevice.Luks, options)
rendered.Storage.Luks = []types.Luks{{
Clevis: clevis,
Expand All @@ -258,7 +288,6 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
renderedTranslations.AddTranslation(lpath, path.New("json", "storage", "luks"))
r.Merge(r2)
}

// create root filesystem
Copy link
Collaborator

Choose a reason for hiding this comment

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

see above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove the extra line.

var rootDevice string
switch {
Expand All @@ -271,6 +300,7 @@ func (c Config) processBootDevice(config *types.Config, ts *translate.Translatio
default:
panic("can't happen")
}

rootFilesystem := types.Filesystem{
Device: rootDevice,
Format: util.StrToPtr("xfs"),
Expand Down
69 changes: 69 additions & 0 deletions config/fcos/v1_6_exp/translate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,75 @@ func TestTranslateBootDevice(t *testing.T) {
},
report.Report{},
},
// boot_device s390x
{
Config{
BootDevice: BootDevice{
Layout: util.StrToPtr("s390x-zfcp"),
Luks: BootDeviceLuks{
Device: "/dev/sda",
Tang: []base.Tang{{
URL: "https://example.com/",
Thumbprint: util.StrToPtr("z"),
Advertisement: util.StrToPtr("{\"payload\": \"xyzzy\"}"),
}},
},
},
},
types.Config{
Ignition: types.Ignition{
Version: "3.5.0-experimental",
},
Storage: types.Storage{
Luks: []types.Luks{
{
Clevis: types.Clevis{
Tang: []types.Tang{{
URL: "https://example.com/",
Thumbprint: util.StrToPtr("z"),
Advertisement: util.StrToPtr("{\"payload\": \"xyzzy\"}"),
}},
},
Device: util.StrToPtr("/dev/sda2"),
Label: util.StrToPtr("luks-root"),
Name: "root",
WipeVolume: util.BoolToPtr(true),
},
},
Filesystems: []types.Filesystem{
{
Device: "/dev/mapper/root",
Format: util.StrToPtr("xfs"),
Label: util.StrToPtr("root"),
WipeFilesystem: util.BoolToPtr(true),
},
},
},
},
[]translate.Translation{
{From: path.New("yaml", "version"), To: path.New("json", "ignition", "version")},
{From: path.New("yaml", "boot_device", "luks", "tang", 0, "url"), To: path.New("json", "storage", "luks", 0, "clevis", "tang", 0, "url")},
{From: path.New("yaml", "boot_device", "luks", "tang", 0, "thumbprint"), To: path.New("json", "storage", "luks", 0, "clevis", "tang", 0, "thumbprint")},
{From: path.New("yaml", "boot_device", "luks", "tang", 0, "advertisement"), To: path.New("json", "storage", "luks", 0, "clevis", "tang", 0, "advertisement")},
{From: path.New("yaml", "boot_device", "luks", "tang", 0), To: path.New("json", "storage", "luks", 0, "clevis", "tang", 0)},
{From: path.New("yaml", "boot_device", "luks", "tang"), To: path.New("json", "storage", "luks", 0, "clevis", "tang")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0, "clevis")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0, "device")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0, "label")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0, "name")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0, "wipeVolume")},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks", 0)},
{From: path.New("yaml", "boot_device", "luks"), To: path.New("json", "storage", "luks")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems", 0, "device")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems", 0, "format")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems", 0, "label")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems", 0, "wipeFilesystem")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems", 0)},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage", "filesystems")},
{From: path.New("yaml", "boot_device"), To: path.New("json", "storage")},
},
report.Report{},
},
}

// The partition sizes of existing layouts must never change, but
Expand Down
12 changes: 11 additions & 1 deletion config/fcos/v1_6_exp/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,25 @@ import (
func (d BootDevice) Validate(c path.ContextPath) (r report.Report) {
if d.Layout != nil {
switch *d.Layout {
case "aarch64", "ppc64le", "x86_64":
case "aarch64", "ppc64le", "x86_64", "s390x-virt", "s390x-zfcp", "s390x-eckd":
default:
r.AddOnError(c.Append("layout"), common.ErrUnknownBootDeviceLayout)
}
}
if len(d.Luks.Device) != 0 && len(d.Mirror.Devices) != 0 {
r.AddOnError(c.Append("mirror"), common.ErrMirrorNotSupport)
}
r.Merge(d.Mirror.Validate(c.Append("mirror")))
return
}

// func (l BootDeviceLuks) Validate(c path.ContextPath) (r report.Report) {
// if len(l.Device) == 0 {
// r.AddOnWarn(c.Append("device"), common.ErrNoLuksBootDevice)
// }
// return
// }

func (m BootDeviceMirror) Validate(c path.ContextPath) (r report.Report) {
if len(m.Devices) == 1 {
r.AddOnError(c.Append("devices"), common.ErrTooFewMirrorDevices)
Expand Down
2 changes: 1 addition & 1 deletion docs/config-fcos-v1_3.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s
* **_should_exist_** (boolean): whether or not the group with the specified `name` should exist. If omitted, it defaults to true. If false, then Ignition will delete the specified group.
* **_system_** (boolean): whether or not the group should be a system group. This only has an effect if the group doesn't exist yet.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"aarch64, ppc64le, and x86_64"

ie : "aarch64, ppc64le,s390x-eckd, s390x-virt, s390x-zfcp, and x86_64. Defaults to x86_64".

I think these were alphabetical lets keep it that way :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update that.

* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-fcos-v1_4.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s
* **_should_exist_** (list of strings): the list of kernel arguments that should exist.
* **_should_not_exist_** (list of strings): the list of kernel arguments that should not exist.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-fcos-v1_5.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s
* **_should_exist_** (list of strings): the list of kernel arguments that should exist.
* **_should_not_exist_** (list of strings): the list of kernel arguments that should not exist.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
3 changes: 2 additions & 1 deletion docs/config-fcos-v1_6-exp.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,14 @@ The Fedora CoreOS configuration is a YAML document conforming to the following s
* **_should_exist_** (list of strings): the list of kernel arguments that should exist.
* **_should_not_exist_** (list of strings): the list of kernel arguments that should not exist.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
* **thumbprint** (string): thumbprint of a trusted signing key.
* **_advertisement_** (string): the advertisement JSON. If not specified, the advertisement is fetched from the tang server during provisioning.
* **_tpm2_** (boolean): whether or not to use a tpm2 device.
* **device** (string): Specifically for s390x `eckd` and `zfcp` disk without `mirror`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If "device" is specifically for s390x maybe we can name it more explicitly rather then "device"? wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about luks.device-s390x ?

* **_threshold_** (integer): sets the minimum number of pieces required to decrypt the device. Default is 1.
* **_discard_** (boolean): whether to issue discard commands to the underlying block device when blocks are freed. Enabling this improves performance and device longevity on SSDs and space utilization on thinly provisioned SAN devices, but leaks information about which disk blocks contain data. If omitted, it defaults to false.
* **_mirror_** (object): describes mirroring of the boot disk for fault tolerance.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-openshift-v4_10.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ The OpenShift configuration is a YAML document conforming to the following speci
* **name** (string): the username for the account. Must be `core`.
* **_ssh_authorized_keys_** (list of strings): a list of SSH keys to be added to `.ssh/authorized_keys` (OpenShift < 4.13) or `.ssh/authorized_keys.d/ignition` (OpenShift ≥ 4.13) in the user's home directory. All SSH keys must be unique.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-openshift-v4_11.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ The OpenShift configuration is a YAML document conforming to the following speci
* **name** (string): the username for the account. Must be `core`.
* **_ssh_authorized_keys_** (list of strings): a list of SSH keys to be added to `.ssh/authorized_keys` (OpenShift < 4.13) or `.ssh/authorized_keys.d/ignition` (OpenShift ≥ 4.13) in the user's home directory. All SSH keys must be unique.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-openshift-v4_12.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ The OpenShift configuration is a YAML document conforming to the following speci
* **name** (string): the username for the account. Must be `core`.
* **_ssh_authorized_keys_** (list of strings): a list of SSH keys to be added to `.ssh/authorized_keys` (OpenShift < 4.13) or `.ssh/authorized_keys.d/ignition` (OpenShift ≥ 4.13) in the user's home directory. All SSH keys must be unique.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
2 changes: 1 addition & 1 deletion docs/config-openshift-v4_13.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ The OpenShift configuration is a YAML document conforming to the following speci
* **_password_hash_** (string): the hashed password for the account.
* **_ssh_authorized_keys_** (list of strings): a list of SSH keys to be added as an SSH key fragment at `.ssh/authorized_keys.d/ignition` in the user's home directory. All SSH keys must be unique.
* **_boot_device_** (object): describes the desired boot device configuration. At least one of `luks` or `mirror` must be specified.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, and `x86_64`. Defaults to `x86_64`.
* **_layout_** (string): the disk layout of the target OS image. Supported values are `aarch64`, `ppc64le`, `s390x-zfcp`, `s390x-eckd`, `s390x-virt` and `x86_64`. Defaults to `x86_64`.
* **_luks_** (object): describes the clevis configuration for encrypting the root filesystem.
* **_tang_** (list of objects): describes a tang server. Every server must have a unique `url`.
* **url** (string): url of the tang server.
Expand Down
Loading