-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[MULTIARCH-3164] add NBDE encryption for IBM Z #58373
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done with the review.
==== | ||
endif::ibm-z-kvm[] | ||
|
||
. Create a parameter file that includes `ignition.platform.id=metal` and `ignition.firstboot=true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kindly replace ignition.firstboot=true
to only ignition.firstboot
.
+ | ||
[source,terminal] | ||
---- | ||
$ coreos-installer pxe customize rhcos-<version>-live-initramfs.x86_64.img \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rhcos-<version>-live-initramfs.x86_64.img
is not required. as we already providing the path to the initramfs in the next line 68.
+ | ||
[source,terminal] | ||
---- | ||
$ coreos-installer pxe customize rhcos-<version>-live-initramfs.x86_64.img \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rhcos--live-initramfs.x86_64.img need removal as we already providing the initramfs file location on line number 68.
|
||
. Create a parameter file that includes `ignition.platform.id=metal` and `ignition.firstboot=true`. | ||
+ | ||
The following example kernel parameter utane configuration for a control plane node creates a file named `master-storage.bu` for disk encryption:ecify the matching rootfs artifact for the kernel and initramfs you are booting. Only HTTP and HTTPS protocols are supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spell error replace utane to butane and encryption:ecify
🤖 Updated build preview is available at: Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/16081 |
tang: | ||
- url: http://12.23.21.58:7500 | ||
thumbprint: QcPr_NHFJammnRCA3fFMVdNBwjs | ||
threshold: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold defaults to 1, so this could be omitted.
+ | ||
[NOTE] | ||
==== | ||
To encrypt DASD disks you must add `device: /dev/disk/by-label/root` to the Ignition file that is generated by Butane. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one line isn't enough detail; we should give step-by-step instructions.
But, stepping back a bit: please don't ask users to manually modify Butane output. It's not how Butane is supposed to be used. Also, there's no guarantee that Butane's x86_64
template will meet the needs of s390x in the future, since s390x considerations are out of scope there. Instead, please use a Butane config that describes the necessary configuration in the storage
section, avoiding boot_device
altogether. See coreos/butane#453 for more info.
---- | ||
rd.neednet=1 \ | ||
console=ttysclp0 \ | ||
coreos.inst.install_dev=dasda \ <1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omitting /dev/
is legacy syntax.
coreos.inst.install_dev=dasda \ <1> | |
coreos.inst.install_dev=/dev/dasda \ <1> |
rd.neednet=1 \ | ||
console=ttysclp0 \ | ||
coreos.inst.install_dev=dasda \ <1> | ||
ignition.firstboot=true ignition.platform.id=metal \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignition.firstboot=true ignition.platform.id=metal \ | |
ignition.firstboot ignition.platform.id=metal \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the configuration as suggested by Benjamin.
+ | ||
[source,yaml] | ||
---- | ||
variant: openshift |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shortcut method works with s390x zfcp, but not works with dasd. So editing the ignition is not recommended. Define the storage configuration directly in the butane. Kindly remove the line from 41 to 52 and add the following config as example.
variant: openshift
version: 4.13.0
metadata:
name: master-storage
labels:
machineconfiguration.openshift.io/role: master
storage:
filesystems:
- device: /dev/mapper/root
format: xfs
label: root
wipe_filesystem: true
luks:
- clevis:
tang:
- thumbprint: QcPr_NHFJammnRCA3fFMVdNBwjs
url: http://12.23.21.58:7500
device: /dev/disk/by-partlabel/root
label: luks-root
name: root
wipe_volume: true
+ | ||
[NOTE] | ||
==== | ||
To encrypt DASD disks you must add `device: /dev/disk/by-label/root` to the Ignition file that is generated by Butane. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace line 60 To encrypt DASD disks you must add device: /dev/disk/by-label/root to the Ignition file that is generated by Butane.
<- To -> To encrypt DASD disks you must add device: /dev/disk/by-label/root to the butane file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems clearer to make this a footnote on the relevant Butane config line.
rd.znet=qeth,0.0.bdd0,0.0.bdd1,0.0.bdd2,layer2=1 \ | ||
rd.zfcp=0.0.5677,0x600606680g7f0056,0x034F000000000000 | ||
---- | ||
<1> For installations on DASD-type disks, add `coreos.inst.install_dev=dasda`. Omit this value for FCP-type disks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kindly replace coreos.inst.install_dev=dasda
to coreos.inst.install_dev=/dev/dasda
---- | ||
rd.neednet=1 \ | ||
console=ttysclp0 \ | ||
ignition.firstboot=true ignition.platform.id=metal \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignition.firstboot=true
to ignition.firstboot
9c0d889
to
1200491
Compare
@@ -55,7 +55,7 @@ $ virt-install \ | |||
--network network={virt_network_parm} \ | |||
--boot hd \ | |||
--location {media_location},kernel={rhcos_kernel},initrd={rhcos_initrd} \ | |||
--extra-args "rd.neednet=1 coreos.inst=yes coreos.inst.install_dev=vda coreos.live.rootfs_url={rhcos_liveos} ip={ip}::{default_gateway}:{subnet_mask_length}:{vn_name}:enc1:none:{MTU} nameserver={dns} coreos.inst.ignition_url={rhcos_ign}" \ | |||
--extra-args "rd.neednet=1 coreos.inst=yes coreos.inst.install_dev=/dev/vda coreos.live.rootfs_url={rhcos_liveos} ip={ip}::{default_gateway}:{subnet_mask_length}:{vn_name}:enc1:none:{MTU} nameserver={dns} coreos.inst.ignition_url={rhcos_ign}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coreos.inst=yes
is obsolete.
--extra-args "rd.neednet=1 coreos.inst=yes coreos.inst.install_dev=/dev/vda coreos.live.rootfs_url={rhcos_liveos} ip={ip}::{default_gateway}:{subnet_mask_length}:{vn_name}:enc1:none:{MTU} nameserver={dns} coreos.inst.ignition_url={rhcos_ign}" \ | |
--extra-args "rd.neednet=1 coreos.inst.install_dev=/dev/vda coreos.live.rootfs_url={rhcos_liveos} ip={ip}::{default_gateway}:{subnet_mask_length}:{vn_name}:enc1:none:{MTU} nameserver={dns} coreos.inst.ignition_url={rhcos_ign}" \ |
ifndef::ibm-z-kvm[] | ||
<1> For installations on DASD-type disks, replace with `device: /dev/disk/by-label/root`. | ||
endif::ibm-z-kvm[] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instructions don't say what to do with the Butane config once created. I assume we want to render it to a MachineConfig and add that to the install templates? We usually give explicit steps for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a link to Creating machine configs with Butane in the additional resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but I think there's a risk that users will follow the steps verbatim and won't realize that additional actions are implied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just below the line no 71 a Note section will remind the user to follow the Adding day 1 kernel arguments. Like below .
Generate the MachineConfig object from master-storage.bu by using butane and place it in the Openshift installation directory as described in "Adding day-1 kernel arguments"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets tackle this issue after 4.13 GA.
name: root | ||
wipe_volume: true | ||
openshift: | ||
fips: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we requiring FIPS mode? If not, we should footnote that this is optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not required will remove the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this has been reinstated, would it make sense to footnote it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it makes sense. I added the callout text we use in the sample install-config.yaml for FIPS
label: root | ||
wipe_filesystem: true | ||
luks: | ||
- clevis: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the FIPS case, the luks
entry should also include:
options:
- --cipher
- aes-cbc-essiv:sha256
We need this to ensure that we're using a FIPS 140-2 compatible cipher mode. The default cipher mode is too new for 140-2 (but is supported in 140-3). Normally Butane handles this if it sees boot_device.luks
and also openshift.fips: true
, but since we're not using boot_device
we need to do this ourselves.
I'd be inclined to footnote it and say that it's only needed in FIPS mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed because I removed FIPS entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SNiemann15 the comment from @bgilbert catched my eyes.
in case a customer want to have fips mode & a change for LUKS is required to adjust the cipher, we might want to document that. Because encryption is kind of base for compliance rules which also includes FIPS.
---- | ||
$ coreos-installer pxe customize \ | ||
/root/rhcos-bootfiles/rhcos-<release>-live-initramfs.s390x.img \ | ||
--dest-device /dev/sda --dest-karg-append \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--dest-device
does the same thing as coreos.inst.install_dev
in the next step. I think it's better to only use --dest-device
, and footnote the different values that should be used for virt/ECKD/FCP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not require --dest-device in initramfs file for Dasda configuration. It has been mentioned in the call out section just below the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting that the instructions either consistently use only --dest-device
or consistently use only coreos.inst.install_dev
, rather than sometimes using both and having the latter override the former.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We used --dest-device
in coreos-installer
command, and the coreos.inst.install_dev
we used in the kernel param file which we punch during the installation. Are you suggesting we use --dest-device in kernel param file too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I'm suggesting that we either use --dest-device
in the coreos-installer pxe customize
command or coreos.inst.install_dev
in the kernel param file. It's unnecessary and confusing to use both.
- clevis: | ||
tang: | ||
- thumbprint: QcPr_NHFJammnRCA3fFMVdNBwjs | ||
url: http://12.23.21.58:7500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be clearer to use e.g. http://clevis.example.com/
to show that this shouldn't be used as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with http://clevis.example.com:7500
labels: | ||
machineconfiguration.openshift.io/role: master | ||
storage: | ||
filesystems: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Butane doesn't care either way, but I'd be inclined to put this section after the luks
section to make the execution order clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variant: openshift
version: 4.13.0
metadata:
name: master-storage
labels:
machineconfiguration.openshift.io/role: master
storage:
luks:
- clevis:
tang:
- thumbprint: QcPr_NHFJammnRCA3fFMVdNBwjs
url: http://clevis.example.com:7500
device: /dev/disk/by-partlabel/root
label: luks-root
name: root
wipe_volume: true
filesystems:
- device: /dev/mapper/root
format: xfs
label: root
wipe_filesystem: true
fips: true | ||
---- | ||
ifndef::ibm-z-kvm[] | ||
<1> For installations on DASD-type disks, replace with `device: /dev/disk/by-label/root`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using /dev/disk/by-label/root
rather than e.g. /dev/dasda3
(where the user has to fill in the correct disk instead of /dev/dasda
) is a bit of a hack, but it does simplify the docs, and I think I'm okay with it. If we're using this approach, maybe it's worth simplifying by using /dev/disk/by-label/root
for the virt case too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bgilbert , /dev/disk/by-partlabel/root
works with KVM too. During the testing on KVM and zVM the configuration are similar. I did not done explicit test using /dev/disk/by-label/root
in virtual (kvm) and zVM case because on the first shot it worked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting that, if we're going to use the /dev/disk/by-label/root
hack, we might as well use it in all cases to avoid the footnote and additional explanation. It should work equally well everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, this is a cleanup, not a blocker.
+ | ||
[NOTE] | ||
==== | ||
Before first boot, you must customize the initramfs for each node in the cluster and add PXE kernel parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this note confuses me. Why is it only shown in the non-KVM case, and what does it add that the rest of the doc hasn't already said?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need it KVM case as well.
<1> For installations on DASD-type disks, replace with `device: /dev/disk/by-label/root`. | ||
endif::ibm-z-kvm[] | ||
|
||
. Create a customized initramfs file, by running the following command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't explicitly say that the customized initramfs should be used instead of the default initramfs when booting the machine. Should we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need to call that out explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that leaves users to fill in the gaps, and there's little harm in calling it out explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think we are already providing the Note just below the command. So explicitly calling "customised" may not be relevant.
- clevis: | ||
tang: | ||
- thumbprint: QcPr_NHFJammnRCA3fFMVdNBwjs | ||
url: http://<clevis.example.com>:7500 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the angle brackets read as slightly confusing to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to remove angle bracket..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will remove. Usually variables that get replaced have those brackets. But I also thought looks strange in this case.
fips: true | ||
---- | ||
ifndef::ibm-z-kvm[] | ||
<1> For installations on DASD-type disks, replace with `device: /dev/disk/by-label/root`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on doing this?
<1> For installations on DASD-type disks, replace with `device: /dev/disk/by-label/root`. | ||
endif::ibm-z-kvm[] | ||
|
||
. Create a customized initramfs file, by running the following command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that leaves users to fill in the gaps, and there's little harm in calling it out explicitly.
840311f
to
1d9cfb3
Compare
/label peer-review-needed QE review is still in progress. |
@ocpdocs-previewbot: user ocpdocs-previewbot is not trusted for pull request #58373 |
Looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! Few suggestions and nits.
|
||
.Prerequisites | ||
|
||
* You set up the External Tang Server. See link:https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption_security-hardening#network-bound-disk-encryption_configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption[Network-bound disk encryption] for instructions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* You set up the External Tang Server. See link:https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption_security-hardening#network-bound-disk-encryption_configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption[Network-bound disk encryption] for instructions. | |
* You have set up the External Tang Server. See link:https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption_security-hardening#network-bound-disk-encryption_configuring-automated-unlocking-of-encrypted-volumes-using-policy-based-decryption[Network-bound disk encryption] for instructions. |
|
||
. Create Butane config files for the control plane and compute nodes. | ||
+ | ||
The following example Butane configuration for a control plane node creates a file named `master-storage.bu` for disk encryption: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following example Butane configuration for a control plane node creates a file named `master-storage.bu` for disk encryption: | |
The following example of Butane configuration for a control plane node creates a file named `master-storage.bu` for disk encryption: |
|
||
.Procedure | ||
|
||
. Create Butane config files for the control plane and compute nodes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. Create Butane config files for the control plane and compute nodes. | |
. Create Butane configuration files for the control plane and compute nodes. |
+ | ||
[NOTE] | ||
==== | ||
Before first boot, you must customize the initramfs for each node in the cluster and add PXE kernel parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before first boot, you must customize the initramfs for each node in the cluster and add PXE kernel parameters. | |
Before first boot, you must customize the initramfs for each node in the cluster, and add PXE kernel parameters. |
rd.neednet=1 \ | ||
console=ttysclp0 \ | ||
ignition.firstboot ignition.platform.id=metal \ | ||
coreos.live.rootfs_url=http://10.19.17.25/redhat/ocp/rhcos-413.86.202302201445-0/rhcos-413.86.202302201445-0-live-rootfs.s390x.img \ | ||
coreos.inst.ignition_url=http://bastion.ocp-cluster1.example.com:8080/ignition/master.ign \ | ||
ip=10.19.17.2::10.19.17.1:255.255.255.0::enbdd0:none nameserver=10.19.17.1 \ | ||
zfcp.allow_lun_scan=0 \ | ||
rd.znet=qeth,0.0.bdd0,0.0.bdd1,0.0.bdd2,layer2=1 \ | ||
rd.zfcp=0.0.5677,0x600606680g7f0056,0x034F000000000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just mentioning that this block doesn't have callouts unlike the previous ifndef::ibm-z-kvm[]
block, in case you think it's a miss. If it is intentional, then great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional :-)
|
||
. Create a parameter file that includes `ignition.platform.id=metal` and `ignition.firstboot`. | ||
+ | ||
Example kernel parameter file for the control plane machine: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example kernel parameter file for the control plane machine: | |
.Example kernel parameter file for the control plane machine: |
|
||
. Create a parameter file that includes `ignition.platform.id=metal` and `ignition.firstboot`. | ||
+ | ||
Example kernel parameter file for the control plane machine: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example kernel parameter file for the control plane machine: | |
.Example kernel parameter file for the control plane machine: |
---- | ||
endif::ibm-z-kvm[] | ||
+ | ||
Write all options in the parameter file as a single line and make sure you have no newline characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a note or added to the step above. I would prefer a note consideirng the structure and reowrding it to be something like:
Ensure that all options in the parameter file are in a single line, and have no newline characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to keep this active voice. But having it as a note is a good suggestion.
/label merge-review-needed |
@SNiemann15 Please squash to 1 commit and re-queue for merge review. Also, I couldn't tell per the earlier comment whether QE review was completed or not at this point? Thank you! |
@adellape Really strange I squashed yesterday I assume the squash got stuck in the github issues. |
/label merge-review-needed |
$ coreos-installer pxe customize \ | ||
/root/rhcos-bootfiles/rhcos-<release>-live-initramfs.s390x.img \ | ||
--dest-device /dev/sda --dest-karg-append \ | ||
ip=<ip-address>::<gateway-ip>:<subnet-mask>::<network-device>:none \ | ||
--dest-karg-append nameserver=<nameserver-ip> \ | ||
--dest-karg-append rd.neednet=1 -o \ | ||
/root/rhcos-bootfiles/<Node-name>-initramfs.s390x.img | ||
---- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not going to hold up the PR for this, but I would strongly recommend coming back later and adding annotations/callouts to describe the user-replaced values in this block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll pick that up in the follow-up PR we are planning post GA.
[id="additional-resources_configure-nbde-ibm-z-kvm"] | ||
.Additional resources | ||
|
||
* xref:../../installing/install_config/installing-customizing.adoc#installing-customizing[Creating machine configs with Butane]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* xref:../../installing/install_config/installing-customizing.adoc#installing-customizing[Creating machine configs with Butane]. | |
* xref:../../installing/install_config/installing-customizing.adoc#installation-special-config-butane_installing-customizing[Creating machine configs with Butane] |
Please remove the period from end of the additional resources xrefs in these 4 assemblies as they are not complete sentences. I also updated the xref anchor to go directly to the "Creating machine configs with Butane" section which could be updated in all 4 assemblies. Thank you!
/cherrypick enterprise-4.13 |
@bscott-rh: new pull request created: #60007 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Merged and cherrypicked to enterprise-4.13. |
Version(s): 4.13 +
Issue: https://issues.redhat.com/browse/MULTIARCH-3164
Link to docs preview:
QE review:
Additional information: