install: Fix firstboot-args being lost when using --config-file

[yasminvalim]

BUGS:

[coreos/coreos-installer] Args passed via --firstboot-args are lost if also using --config-file
When you use both a config file and --firstboot-args, the firstboot arguments disappear.

Cannot use auto-forward kargs (like ip=) with coreos-installer (iso|pxe) customize
When customizing PXE/ISO images, networking arguments that should be automatically forwarded don't work.

FIX:

Checks if firstboot_args is defined and manually adds --firstboot-args to the arguments list.

[gemini-code-assist]

Code Review

This pull request fixes an issue where --firstboot-args were lost when a --config-file was also used. The change from #[serde(skip)] to #[serde(skip_serializing_if = "is_default")] correctly ensures the firstboot_args field is serialized when it has a value. However, as noted in the specific comment, this change is redundant. Simply removing the #[serde(skip)] attribute would be a cleaner solution. Additionally, the unit tests need to be updated to reflect this change in behavior, as the serialize_full_install_config test will now fail. It would also be beneficial to add a new test case that specifically verifies the fix for the reported bug.

[yasminvalim]

I still thinking what is the best approach. There is other more complex solution such changing the firstboot_args field type to Vec<String> like the other fields, but I think it could be a breaking change. Other option would be simple as fix the bug without handle multiple sources but I guess it would not be the best approach either since it would create another problems.

Any insights?

[jlebon]

Hmm, yeah this seems more complex than I expected. I think it was by design that --firstboot-args was not exposed via the configs; it's an implementation detail of coreos-installer-service and we in general don't expect users to really use it directly.

I think probably the simplest thing is to just have expand_config_files() check if firstboot args are defined (i.e. self.firstboot_args is not None), and if so, then just add --firstboot-args args manually to the args list before calling from_args()?

[yasminvalim]

Hey @jlebon, thanks for your review! I was not aware about how the args are used and if the user had to handle it directly. I fixed it following your comment and also add a unit test to be sure that it will work. Can you take a look and tell me what you think about it now?

Thanks :)

[dustymabe]

LGTM. I did a local test and it seems to fix the problem.