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

load grub config from ai_data #1536

Merged
merged 3 commits into from
Jul 17, 2023
Merged

Conversation

ITJamie
Copy link
Contributor

@ITJamie ITJamie commented Jan 18, 2023

launchpad bug: 1952926

This change should make it so that user supplied grub config in the autoinstall data will be used even when the layout option is supplied.

This would allow users to supply the simpler storage configs (see launchpad bug) and disable the grub reordering (which currently moves network boot devices to the first boot item making the newly installed os not the default boot option)

@dbungert
Copy link
Collaborator

Thanks for the contribution. Looks correct.

I took the liberty of adding some tests - I was going to ask you to look into it, but tests on the filesystem controller can be kind of nasty and I wasn't sure how to explain it, so I wrote it. Then I realized the same should be done for the swap declaration. Then I realized I was kind of taking over your PR. Sorry!

documentation/autoinstall-reference.md would benefit from an update for the above, if you're interested in helping further a text update would be nice, or I can take a look soon.

@ITJamie
Copy link
Contributor Author

ITJamie commented Feb 10, 2023

Absolutely no problem. I had looked at the tests and instantly realised i was in over my head to figure out initial testing logic for this. Thank you!

@m-shibata
Copy link
Contributor

Could you please update me on the status of this Pull Request?

The code changes seem fine, is it just the documentation that needs to be updated? If you are expecting someone to make the documentation changes, should they issue a Pull Request to ITJamie:layout_grub_config?

@ITJamie
Copy link
Contributor Author

ITJamie commented Jul 8, 2023

I dont think there is any doco to change. There are examples showing its uses already but they didnt work as the config block was indented

@m-shibata
Copy link
Contributor

Is it okay if I don't make any changes to the document at the URL below?

If the "layout" feature is used to configure the disks, the "config" section will not be used.
As well as putting the list of actions under the 'config' key, the [grub](https://curtin.readthedocs.io/en/latest/topics/config.html#grub) and [swap](https://curtin.readthedocs.io/en/latest/topics/config.html#swap) curtin config items can be put here. So a storage section might look like:

From what I understand, the document explains that even with a layout set, you can write "grub" and "swap" under the config block. With the proposed modification, I believe that "grub" and "swap" will be applied regardless of the presence of a layout.

@dbungert dbungert merged commit d1e716f into canonical:main Jul 17, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants