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

Support kargs.d directories for default kargs #1836

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rfairley
Copy link
Member

@rfairley rfairley commented Apr 8, 2019

Add the following config directories for setting default kernel
arguments:

/etc/ostree/kargs.d (host config)
/usr/lib/ostree-boot/kargs.d (base config)

These directories contain files whose contents consist of a karg
snippet, which is a collection kernel parameter keys and values.
Example of a snippet:

KEY=FOO KEYFLAG ANOTHERKEY=VALUE1 ANOTHERKEY=VALUE2

Snippets are read in alphanumeric order of the karg snippet filename
when generating the final kernel options from the host and base config
directories. Ordering may be specified by prefixing a number, e.g.
/etc/ostree/kargs.d/4000_localhost.

The bootconfig key ostree-kargs-generated-from-config indicates
whether the kargs were generated by ostree from the kargs.d directories
(true), or copied from the previous deployment (false). Editing the
kargs through the command line (e.g. ostree admin deploy --karg=) will
set the flag to false, and kargs will be copied from the previous
deployment for subsequent deployments.

Also add support for ostree admin instutil set-kargs so that installer
programs using this command (such as Anaconda) remain managed by the
kargs.d directories from the first deployment.

Closes: #479


Self notes (to fix in this PR):

  • Typo in commit message which is a collection kernel parameter keys and values
  • Need to use glnx_autofd where appropriate

To do (as follow-ups to this PR):

  • Update ostree package ownership of new kargs config files /etc/ostree/kargs and /usr/lib/ostree-boot/kargs (in Fedora RPM, etc).
  • Add unit tests for reading kargs with space-separated values (i.e. to the next_arg() function).

@rfairley rfairley changed the title Add support for default kargs WIP: Add support for default kargs Apr 8, 2019
@rfairley rfairley force-pushed the rfairley-default-kargs branch 2 times, most recently from 679bc5b to c8ab08f Compare April 10, 2019 15:05
rfairley pushed a commit to rfairley/ostree that referenced this pull request Apr 10, 2019
Add an ostree-kargs-override flag which is set to true if the
kargs of the current deployment were overridden. The flag is
written to the bootconfig, and persists in subsequent deployments.

This is in preparation for ostreedev#1836 which needs a way to determine
whether the previous deployment's kargs were modified by the user,
so that user-modified kargs can be carried forward to the next
deployment.
rfairley pushed a commit to rfairley/ostree that referenced this pull request Apr 10, 2019
Add an ostree-kargs-override flag which is set to true if the
kargs of the current deployment were overridden. The flag is
written to the bootconfig, and persists in subsequent deployments.

This is in preparation for ostreedev#1836 which needs a way to determine
whether the previous deployment's kargs were modified by the user,
so that user-modified kargs can be carried forward to the next
deployment.
@rfairley rfairley force-pushed the rfairley-default-kargs branch 2 times, most recently from cc3abc3 to 256226e Compare April 11, 2019 15:53
rfairley pushed a commit to rfairley/ostree that referenced this pull request Apr 11, 2019
Add an ostree-kargs-override flag which is set to true if the
kargs of the current deployment were overridden. The flag is
written to the bootconfig, and persists in subsequent deployments.

This is in preparation for ostreedev#1836 which needs a way to determine
whether the previous deployment's kargs were modified by the user,
so that user-modified kargs can be carried forward to the next
deployment.
@rfairley rfairley force-pushed the rfairley-default-kargs branch 8 times, most recently from 5a24fe1 to bb91d98 Compare April 12, 2019 14:45
@rfairley rfairley changed the title WIP: Add support for default kargs WIP: Add support for default kargs files Apr 12, 2019
@rfairley
Copy link
Member Author

rfairley commented Apr 12, 2019

Commands to test this on Fedora Atomic Host 29 (also works with FCOS setting branch=fedora/29/x86_64/coreos).

# Checkout the system repo, write kargs to `/usr/lib/ostree-boot/kargs` and `/usr/etc/ostree/kargs`, and commit back to the repo
branch=fedora/29/x86_64/atomic-host
sudo su && cd /var && mkdir -p test/repo && cd test/repo && rev=$(ostree rev-parse $branch) && ostree checkout --repo=/sysroot/ostree/repo $rev && cd $rev && echo "FOO=USR_1 no_timer_check console=tty1 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 rd.lvm.lv=atomicos/root root=/dev/mapper/atomicos-root" > usr/lib/ostree-boot/kargs && echo "HELLO=USR_ETC_1" > usr/etc/ostree/kargs && ostree diff --repo=/sysroot/ostree/repo $rev $(pwd) && ostree commit --repo=/sysroot/ostree/repo --tree=dir=$(pwd) --branch $branch

ostree admin deploy $branch

cat /boot/loader/entries/ostree-* | grep HELLO
# Output: options HELLO=USR_ETC_1 FOO=USR_1 no_timer_check console=tty1 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 rd.lvm.lv=atomicos/root root=/dev/mapper/atomicos-root ostree=/ostree/boot.1/fedora-atomic/bcd1a84e1581948bb08fe52bf09249f8a9fc0fe20968410f90e7f8163227e30e/0

# Configure the host kargs
echo "MYCONFIG=ETC_1" > /etc/ostree/kargs
ostree admin deploy $branch

cat /boot/loader/entries/ostree-* | grep MYCONFIG
# Output: options MYCONFIG=ETC_1 FOO=USR_1 no_timer_check console=tty1 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 rd.lvm.lv=atomicos/root root=/dev/mapper/atomicos-root ostree=/ostree/boot.0/fedora-atomic/bcd1a84e1581948bb08fe52bf09249f8a9fc0fe20968410f90e7f8163227e30e/0

# Using the `--karg*` options will append the specified kargs, and add the `ostree-kargs-override` flag to the BLS config
ostree admin deploy --karg-append="BAR=KARG" $branch

cat /boot/loader/entries/ostree-* | grep ostree-kargs-override
# Output: ostree-kargs-override true

cat /boot/loader/entries/ostree-* | grep BAR
# Output: options MYCONFIG=ETC_1 FOO=USR_1 no_timer_check console=tty1 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 rd.lvm.lv=atomicos/root root=/dev/mapper/atomicos-root ostree=/ostree/boot.1/fedora-atomic/bcd1a84e1581948bb08fe52bf09249f8a9fc0fe20968410f90e7f8163227e30e/0 BAR=KARG

# Rebooting should succeed with the kargs added above
systemctl reboot

@rfairley rfairley marked this pull request as ready for review April 12, 2019 18:00
@rfairley rfairley changed the title WIP: Add support for default kargs files Add support for default kargs files Apr 12, 2019
@rfairley
Copy link
Member Author

rfairley commented Apr 12, 2019

OK - this is ready for a look now!

@rfairley rfairley changed the title Add support for default kargs files WIP: Add support for default kargs files Apr 12, 2019
@rfairley rfairley force-pushed the rfairley-default-kargs branch 6 times, most recently from bb64139 to 483d369 Compare May 10, 2019 22:05
@rfairley rfairley force-pushed the rfairley-default-kargs branch 13 times, most recently from f154467 to 6c2610e Compare May 24, 2019 15:07
Add the following config directories for setting default kernel
arguments:

/etc/ostree/kargs.d (host config)
/usr/lib/ostree-boot/kargs.d (base config)

These directories contain files whose contents consist of a karg
snippet, which is a collection kernel parameter keys and values.
Example of a snippet:

```
KEY=FOO KEYFLAG ANOTHERKEY=VALUE1 ANOTHERKEY=VALUE2
```

Snippets are read in alphanumeric order of the karg snippet filename
when generating the final kernel options from the host and base config
directories. Ordering may be specified by prefixing a number, e.g.
`/etc/ostree/kargs.d/4000_localhost`.

The bootconfig key `ostree-kargs-generated-from-config` indicates
whether the kargs were generated by ostree from the kargs.d directories
(`true`), or copied from the previous deployment (`false`). Editing the
kargs through the command line (e.g. `ostree admin deploy --karg=`) will
set the flag to `false`, and kargs will be copied from the previous
deployment for subsequent deployments.

Also add support for `ostree admin instutil set-kargs` so that installer
programs using this command (such as Anaconda) remain managed by the
kargs.d directories from the first deployment.

Closes: ostreedev#479
@rfairley
Copy link
Member Author

rfairley commented May 24, 2019

Tested this in FCOS with the following commands:

From a coreos-assembler repo clone:

git clone https://github.com/coreos/coreos-assembler
cd coreos-assembler
git remote add rfairley https://github.com/rfairley/coreos-assembler
git fetch rfairley
git checkout rfairley-localostree

(The above coreos-assembler branch has the following patch applied):

diff --git a/src/virt-install b/src/virt-install
index 2ad6578..61761fb 100755
--- a/src/virt-install
+++ b/src/virt-install
@@ -150,6 +150,10 @@ if args.ostree_repo:
     assert args.ostree_ref
     ks_tmp.write(f"""
 ostreesetup --nogpg --osname={args.ostree_stateroot} --remote={args.ostree_remote} --url=http://{ipv4}:{PORT}/ --ref="{args.ostree_ref}"
+%pre
+curl -L -O http://{ipv4}:{PORT}/ostreebuild.tar.gz
+tar -C / -xf ostreebuild.tar.gz
+%end
     """)
 
 if args.fs9p:

From the COSA/FCOS working directory:

cosadir=/path/to/coreos-assembler
export COREOS_ASSEMBLER_GIT=$cosadir
cosa init https://github.com/coreos/fedora-coreos-config # see coreos-assembler README.md (https://github.com/coreos/coreos-assembler#setup) for the `cosa` alias

From an OSTree repo checkout of this PR:

fcosdir=/path/to/fcos
ostreedir=/path/to/ostree
cd $ostreedir/ci
./build-rpm.sh
cd ..
make install DESTDIR=$ostreedir/install/ && rm -rf ./ostreebuild && rm -rf ./ostreebuild.tar.gz && mkdir -p ./ostreebuild && cp -r ./install/* ./ostreebuild && cd ./ostreebuild  && tar -czvf ../ostreebuild.tar.gz ./
cp $ostreedir/ostreebuild.tar.gz $fcosdir/repo/
cp $ostreedir/ci/x86_64/ostree-* $fcosdir/overrides/rpm/

From the FCOS workdir:

cosa build
cosa run

In the booted FCOS VM, see that the /etc/ostree/kargs.d snippet is created and the kargs specified by Anaconda are present in the BLS config:

$ ls /etc/ostree/kargs.d/ -l
total 4
-rw-r--r--. 1 root root 156 May 24 21:03 4000_ostree_instutil
$ cat /boot/loader/entries/ostree-1-fedora-coreos.conf 
...
options console=tty0 console=ttyS0,115200n8 rootflags=defaults,prjquota rw mitigations=auto,nosmt $ignition_firstboot root=UUID=e2b701bb-81f4-48c2-a471-4bcebabddf71 ostree=/ostree/boot.0/fedou
ostree-kargs-generated-from-config true
...

If you use add-files in $fcosdir/src/config/fedora-coreos-base.yaml e.g. like so:

diff --git a/fedora-coreos-base.yaml b/fedora-coreos-base.yaml
index 617d000..17e8993 100644
--- a/fedora-coreos-base.yaml
+++ b/fedora-coreos-base.yaml
@@ -150,3 +150,10 @@ packages:
   - console-login-helper-messages-profile
   # CoreOS Installer
   - coreos-installer coreos-installer-dracut
+
+add-files:
+  - - 2000_hello
+    - /usr/lib/ostree-boot/kargs.d/2000_hello
+  - - 2001_foo
+    - /usr/lib/ostree-boot/kargs.d/2001_foo

then empty files will be written to /etc/ostree/kargs.d/2000_hello and /etc/ostree/kargs.d/2001_foo, which override the kargs specified in add-files - as expected by Anaconda using ostree admin instutil set-kargs.

@rfairley
Copy link
Member Author

Lifting WIP - now in a working/reviewable state.

The commands ostree admin deploy --karg* and rpm-ostree kargs * will need optimizing to stay managed by the configs in a similar way that has been done for ostree admin instutil set-kargs, but this should be safe to introduce as a separate change (given the ostree-kargs-generated-from-config flag to indicate kargs source of truth).

CI tests looks to fail on tests/test-pull-repeated.sh - which is a known flake.

@rfairley rfairley changed the title WIP: Support kargs.d directories for default kargs Support kargs.d directories for default kargs May 25, 2019
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably be2572b) made this pull request unmergeable. Please resolve the merge conflicts.

@AdrianVovk
Copy link

Any updates about this?

@clime
Copy link
Contributor

clime commented Apr 2, 2020

Any updates about this?

The same question here. This would be useful.

@agners
Copy link
Contributor

agners commented Oct 13, 2020

This would be useful for us as well.

@rfairley do you plan on working further on this? Otherwise I might do a rebase and submit a new merge request.

@rfairley
Copy link
Member Author

rfairley commented Oct 15, 2020

Hi @agners, I'm not currently planning to work further on this - feel free to continue to further this.

Note, these changes may still be blocked/dependent on resolving some of the issues brought up in #479 - e.g. support to run OSTree in the initramfs (coreos/fedora-coreos-tracker#34), so that systems where the kargs.d files can change in the initramfs (such as FCOS, through Ignition), can reboot with the new args before the real root switch. Alternative solutions have been suggested in #479 too, like storing the default kargs within the kernel binary. I think some discussion is still needed there before this approach is finalized and this kind of support is accepted into OSTree (and machines start depending on kargs.d).

But as a POC, this has been tested per #1836 (comment). A lot has changed in the FCOS build tooling used since that comment (e.g. Anaconda is no longer used, so no need to inject an OSTree build with these changes into an Anaconda image -- it should be installed in the COSA container image instead), but the general testing method is still valid (apply a build of OSTree with this patch included into the OS build environment/tooling, build an OS image which incorporates this build of OSTree, and add the kargs.d files to the OS image through some means, e.g. the rpm-ostree compose treefile, to trigger this functionality).

@openshift-ci-robot
Copy link
Collaborator

@rfairley: PR needs rebase.

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.

@openshift-ci
Copy link

openshift-ci bot commented Jun 29, 2023

@rfairley: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/sanity e1d2e47 link true /test sanity
ci/prow/images e1d2e47 link true /test images
ci/prow/fcos-e2e e1d2e47 link true /test fcos-e2e

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support default kernel arguments
6 participants