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

mantle/platform: rename IgnitionNetworkKargs to AppendFirstbootKernelArgs and expose to ext tests #2683

Merged
merged 3 commits into from
Feb 3, 2022

Conversation

dustymabe
Copy link
Member

commit 97249401af27d1596a3189e6a1dee03d1369a32f
Author: Dusty Mabe <dusty@dustymabe.com>
Date:   Tue Feb 1 20:54:33 2022 -0500

    kola/external-tests: add appendFirstbootKernelArgs support in kola.json
    
    Add the ability to specify firstboot kernel arguments to transiently
    append to the default kernel arguments for the first boot of the machine.
    This will be useful when testing workflows that use kernel arguments
    for network configuration.
    
    For now, this only supports the qemu platform.

commit 87eda63f8cee17626d1a1a048e8ea073cf7f2ea6
Author: Dusty Mabe <dusty@dustymabe.com>
Date:   Tue Feb 1 20:34:32 2022 -0500

    mantle/platform: qemuexec: rename --knetargs to --firstbootkargs
    
    This is a little more inline with what's being done, which is to
    apply the given arguments to the first boot of the machine only.

commit b4d9c99d69eeb786c8c1359ce78b799d72dcf858
Author: Dusty Mabe <dusty@dustymabe.com>
Date:   Tue Feb 1 20:29:26 2022 -0500

    mantle/platform: rename IgnitionNetworkKargs to AppendFirstbootKernelArgs
    
    We want to expose this to external tests and AppendFirstbootKernelArgs
    is a bit more explicit about what it's doing, which is applying kernel
    arguments that will apply to only the first boot of the machine.

…Args

We want to expose this to external tests and AppendFirstbootKernelArgs
is a bit more explicit about what it's doing, which is applying kernel
arguments that will apply to only the first boot of the machine.
This is a little more inline with what's being done, which is to
apply the given arguments to the first boot of the machine only.
Add the ability to specify firstboot kernel arguments to transiently
append to the default kernel arguments for the first boot of the machine.
This will be useful when testing workflows that use kernel arguments
for network configuration.

For now, this only supports the qemu platform.
@dustymabe dustymabe changed the title mantle/platform: rename IgnitionNetworkKargs to AppendFirstbootKernelArgs mantle/platform: rename IgnitionNetworkKargs to AppendFirstbootKernelArgs and expose to ext tests Feb 2, 2022
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

One comment, but LGTM as is!

@@ -75,7 +75,7 @@ const maxAdditionalNics = 16

func init() {
root.AddCommand(cmdQemuExec)
cmdQemuExec.Flags().StringVarP(&knetargs, "knetargs", "", "", "Arguments for Ignition networking on kernel commandline")
cmdQemuExec.Flags().StringVarP(&firstbootkargs, "firstbootkargs", "", "", "Additional first boot kernel arguments")
Copy link
Member

Choose a reason for hiding this comment

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

Minor: firstboot-kargs?

Copy link
Member Author

Choose a reason for hiding this comment

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

ehh - i'm not in love with either way. I kept no dashes since knetargs didn't have one either.

Copy link
Member Author

Choose a reason for hiding this comment

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

If this passes CI I'll merge it.

@dustymabe dustymabe enabled auto-merge (rebase) February 2, 2022 22:01
@dustymabe dustymabe merged commit f690451 into coreos:main Feb 3, 2022
@dustymabe dustymabe deleted the dusty-appendfirstbootkernelargs branch February 3, 2022 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants