-
Notifications
You must be signed in to change notification settings - Fork 464
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
baremetal: enhancement for ironic config passthrough #401
Conversation
This enhancement describes a mechanism to pass arbitrary config option through to ironic in baremetal ipi.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: derekhiggins The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The motivation for this enhancement is to allow people to deal with their individual corner | ||
cases without the need to insert special cases into the ironic containers. The ironic | ||
container can stay opinionated and targeted for a specific set of environments while also | ||
being usable where uniq envirnments require adjustments. |
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.
Do you have some examples of configuration changes that users have needed to change?
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.
Yup, there are 3 examples ongoing that I can think of
Operators trying to deploy kni on openstack(for demos) have been looking for a way to customise IPMI timouts and retries, see here
metal3-io/ironic-image#167
For CI, we've been trying to find a way to add certain kernel args for CI only, one of this merged which I believe means IPA is no longer being logged to the console, making debugging very difficult
metal3-io/ironic-image#172
metal3-io/ironic-image#160
For debugging IPA users need to add a SSH key to kernel args (this one could probbaly be a top level config option)
metal3-io/ironic-image#150
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.
Yup, there are 3 examples ongoing that I can think of
Operators trying to deploy kni on openstack(for demos) have been looking for a way to customise IPMI timouts and retries, see here
metal3-io/ironic-image#167
That seems like a reasonable thing to put into an API. Is it configurable on a per-host basis, or do we have to put a value into the ironic config file?
For CI, we've been trying to find a way to add certain kernel args for CI only, one of this merged which I believe means IPA is no longer being logged to the console, making debugging very difficult
metal3-io/ironic-image#172
metal3-io/ironic-image#160
Is there any reason we wouldn't want serial console logging all the time? Can we re-enable it and write a test to ensure it can't be disabled accidentally?
If it needs to be configurable, is it a per-host option or global?
For debugging IPA users need to add a SSH key to kernel args (this one could probbaly be a top level config option)
metal3-io/ironic-image#150
Could we use the same ssh key that is given in the install-config for access to the control plane nodes?
```yaml | ||
platform: | ||
baremetal: | ||
ironicExtraConf: {"OS_PXE__PXE_APPEND_PARAMS":'nofb nomodeset vga=normal sshkey="ssh-rsa AAAA..."', "OS_API__API_WORKERS":"8"} |
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.
Ironic (and IPA) was always intended to be an implementation detail for IPI, so I'm pretty uncomfortable with adding a top-level ironicFoo option tbh.
In particular if you check the discussion on coreos/ignition#979 you'll see there are a bunch of unsolved issues wrt the OpenShift coreos-install based ramdisk vs IPA, and the options in either case are completely different.
I wonder if we could perhaps make this more generic and have some pxeExtra interface or something, or even better some interface which describes the required behavior e.g which console to use?
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, upstream we want to use the oslo.config environment variable interface instead of customizing individual settings in the ironic config to cut down on the number of changes to the image. The whole point of adding environment variable handling to oslo.config was to make it easier to pass settings to OpenStack services running in containers.
Downstream in OpenShift, however, I would rather that we add explicit APIs for the things we need to allow users to change. That allows us to take advantage of the API server for some basic validation, and makes it harder for users to put the system into an unsupportable state or one where metal3 is crash-looping due to invalid settings.
```yaml | ||
platform: | ||
baremetal: | ||
ironicExtraConf: {"OS_PXE__PXE_APPEND_PARAMS":'nofb nomodeset vga=normal sshkey="ssh-rsa AAAA..."', "OS_API__API_WORKERS":"8"} |
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.
For readability, can you stick to canonical YAML:
platform:
baremetal:
ironicExtraConf:
OS_API__API_WORKERS: "8"
OS_PXE__PXE_APPEND_PARAMS: "nofb nomodeset vga=normal sshkey=\"ssh-rsa AAAA...\""
OK will close this, looks like we'd rather add things when needed. |
This enhancement describes a mechanism to pass arbitrary
config option through to ironic in baremetal ipi.