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

Ignition Kernel Argument Support #1168

Closed
arithx opened this issue Feb 23, 2021 · 9 comments · Fixed by #1178
Closed

Ignition Kernel Argument Support #1168

arithx opened this issue Feb 23, 2021 · 9 comments · Fixed by #1178

Comments

@arithx
Copy link
Contributor

arithx commented Feb 23, 2021

Overview

Ignition will add fields to support the adding or removing of kernel arguments. Additionally, a new stage will be introduced which by default is NOT enabled in the current stage chaining. During this new stage, Ignition will take the fields and directly pass them into a binary (the location of which is defined in internal/distro to allow overrides) provided by the distribution. Ignition will NOT verify that the kernel arguments have been set, only that the binary returned successfully. Additionally, the binary / wrapping systemd units (provided by the distribution) are responsible for performing a reboot. Ignition will not persistently cache the fetched config, so it will be re-fetched after a reboot.

Example Spec Changes

A new top level field kargs will be added to the Ignition spec which contains two fields, shouldExist & shouldNotExist which both accept lists of strings.

NOTE: The individual field names are subject to changes

{
  "ignition": { "version": "3.3.0-experimental" },
  "kargs": {
      "shouldExist": [
          "hugepagesz=1M hugepages=8 hugepagesz=2G hugepages=2",
          "foo=bar",
          "baz"
      ],
      "shouldNotExist": [
          "nosmt"
      ]
  }
}

Validation

Ignition will fail validation if duplicate entries are found in both the shouldExist & shouldNotExist arrays. It will compare both entire string and space-delimited splits of each string and reject matches. This is done to not require an add/remove specific ordering & to preserve the declarativity of the config.

Distributor Documentation

New distribution documentation will be added to Ignition to provide a sample script for how the binary could look, along with making recommendations about stage ordering (the recommended ordering will be sometime after the fetch stage and before the disks stage).

Binary Constraints

Ignition expects that the distribution provided binaries follow these constraints:

  1. Adds should be done in an append if missing manner
  2. Deletes should NOT fail if they are not present
  3. Rebooting the system if necessary
@cgwalters
Copy link
Member

This seems to support both whitespace-separated strings as well as distinct entries in an array. What's the rationale for that? Is it saying that e.g. the arguments in a single string must be provided in exactly that order, but otherwise the arguments may be reordered?

@cgwalters
Copy link
Member

cgwalters commented Feb 23, 2021

Also I think the problem in openshift/os#503 :

At minimum we'll need to coordinate the two so that there is a maximum of one reboot in the initrd (if any of FIPS or Ignition kernel arguments are set).

is a general one. A FCOS user might also want to reboot for some other reason - perhaps, similarly to the MCO/OpenShift4 they want to apply OS updates before some workloads can land on a machine.

So I think probably what we want is a declarative way to tell Ignition to not reboot in the initramfs. Perhaps to start it's just:

{
  "ignition": { "version": "3.3.0-experimental" },
  "kargs": {
      "reboot": false,
      "shouldExist": [
          "hugepagesz=1M hugepages=8 hugepagesz=2G hugepages=2",
          "foo=bar",
          "baz"
      ],
      "shouldNotExist": [
          "nosmt"
      ]
  }
}

@bgilbert
Copy link
Contributor

bgilbert commented Feb 23, 2021

This seems to support both whitespace-separated strings as well as distinct entries in an array. What's the rationale for that?

In general, users should specify each argument as a separate array entry. The space-separated case is to support specific parameters such as hugepages, where an individual argument might be repeated in combination with another argument. An argument-by-argument shouldExist wouldn't do the right thing in that case.

A FCOS user might also want to reboot for some other reason - perhaps, similarly to the MCO/OpenShift4 they want to apply OS updates before some workloads can land on a machine.

Hmm, do you have a specific use case in mind? In general, we go out of our way to make the first boot behave identically to the other boots, and we encourage FCOS users to directly launch the version of the OS they want to run. (Which is to say, I think we should continue to discourage an OpenShift-style multiple-reboot model in FCOS.)

@jlebon
Copy link
Member

jlebon commented Feb 25, 2021

This isn't describing the API that the distribution binary needs to implement.

I think initially we were discussing about just using switches for this. But I'm thinking it'd be cleaner really if Ignition just reserializes the kargs object to JSON and passes it as its stdin. Then, the API is the spec and has the same compatibility constraints etc...

(It also has the effect of encouraging implementations to be written in !shell, which is not a bad idea. :) )

@bgilbert
Copy link
Contributor

I think we should keep the API for the helper program as simple as possible, to reduce the integration effort needed by a distro. The initrd typically won't have jq or a language interpreter other than shell, and a bespoke binary complicates creation of the distro-specific Dracut module. So I'd argue for sticking with command-line arguments, and keeping the example program in shell.

@jlebon
Copy link
Member

jlebon commented Mar 9, 2021

Starting to look at adding the necessary support for this in rdcore. In the interest of using wording that's implementation-agnostic and a 1:1 mapping to the spec, let's just do something like (using the same kargs as above):

$ <distro.kargCmd> \
    --should-exist "hugepagesz=1M hugepages=8 hugepagesz=2G hugepages=2" \
    --should-exist "foo=bar" \
    --should-exist "baz" \
    --should-not-exist "nosmt"

?

@jlebon
Copy link
Member

jlebon commented Mar 16, 2021

rdcore kargs support: coreos/coreos-installer#498

@lucab
Copy link
Contributor

lucab commented Mar 19, 2021

A bunch of design questions on this proposal.

It looks like the execution flow is not explicitely defined/linearized. How is the system expected to behave if there is a stub configuration with something like kargs.shouldExist = [ "ip=..." ] and ignition.config.replace.source = "https://..."?

Both shouldExist and shouldNotExist take a list of strings. Does this mean that they enforce ordering too? That is, would shouldExist: ["a", "b"] be ok with a b foo a cmdline?

Is this new stage expected to be aware of dracut injected args?

@jlebon
Copy link
Member

jlebon commented Mar 22, 2021

A bunch of design questions on this proposal.

It looks like the execution flow is not explicitely defined/linearized. How is the system expected to behave if there is a stub configuration with something like kargs.shouldExist = [ "ip=..." ] and ignition.config.replace.source = "https://..."?

Do you mean whether this is something that could be used to break the network bootstrapping problem? I'm not sure if this is something we should pursue, because it's inconsistent with how Ignition processes its config otherwise (i.e. fetching everything first and merging/replacing as needed). So in the scenario you described, ignition-fetch would still as usual try to fetch the HTTPS config and replace it in before continuing. I'd like us to keep trying to move away from network kargs towards NM keyfiles and the --copy-network workflow instead (and keep polishing that UX, like openshift/enhancements#492).

Both shouldExist and shouldNotExist take a list of strings. Does this mean that they enforce ordering too? That is, would shouldExist: ["a", "b"] be ok with a b foo a cmdline?

Ordering is not enforced, but you can specify multiple kargs in a single element using spaces if you want specific kargs to appear in a certain order (see #1168 (comment) above).

Is this new stage expected to be aware of dracut injected args?

Ignition itself doesn't really care about dracut args, so this is mostly down to the distro implementation. For FCOS/RHCOS, I don't think we should be aware of dracut args at all. Although we do make use of it at runtime for the conditional networking bits, we should strongly encourage components to not bake drop-ins there (see e.g. latchset/clevis@c52caeb).

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

Successfully merging a pull request may close this issue.

5 participants