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

rpmostree: Use --merge for kargs #6084

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Conversation

cgwalters
Copy link
Contributor

Without --merge, all kernel arguments will be replaced, which is
not what is desired in general. Especially with bootc karg support
which we definitely want to work with Anaconda.

@github-actions github-actions bot added the f42 Fedora 42 label Jan 7, 2025
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me.
I wonder if it couldn't be a fix for https://issues.redhat.com/browse/RHEL-66155 ?

@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

/kickstart-test --testtype ostree

@cgwalters
Copy link
Contributor Author

I wonder if it couldn't be a fix for https://issues.redhat.com/browse/RHEL-66155 ?

I have no idea what's going on in that bug, kargs work elsewhere. I doubt this fixes it, but maybe.

@jkonecny12
Copy link
Member

There are broken tests with this patch. Could you please fix them or I'm happy to fix your commit if you want me to do that.

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Tests needs to be fixed first.

Without `--merge`, all kernel arguments will be replaced, which is
not what is desired in general. Especially with bootc karg support
which we definitely want to work with Anaconda.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Contributor Author

I took a stab at that

@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@jkonecny12
Copy link
Member

/kickstart-test --testtype ostree

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

I guess we want to get this to RHEL too?

@jkonecny12
Copy link
Member

@cgwalters I'm thinking about creation of kickstart test for this functionality. How can we test this?
If I understand it correctly it will replace existing arguments but I guess there are non during the installation am I correct?

@cgwalters
Copy link
Contributor Author

@cgwalters I'm thinking about creation of kickstart test for this functionality. How can we test this?

Build a container adding https://containers.github.io/bootc/building/kernel-arguments.html#usrlibbootckargsd

@jkonecny12
Copy link
Member

We discussed today how to approach this from the CI perspective. The issue is that we need a custom ISO to be able to test this with kickstart tests and these are not prepared for such an approach. To enable https://github.com/rhinstaller/kickstart-tests/ on a custom ISO we would need to change the ISO centric framework to be able to set a custom ISO for a specific test which will need some effort to do that.

Anyway, we agreed that we don't want to block this simple patch on that but we would like to have this covered for RHEL backports if possible.

Merging.

@jkonecny12 jkonecny12 merged commit 2c34a65 into rhinstaller:main Jan 15, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f42 Fedora 42
Development

Successfully merging this pull request may close these issues.

2 participants