Skip to content

Introduce customisation mode option#308

Merged
dirkmueller merged 3 commits intoSUSE:mainfrom
atanasdinov:customisation-mode
Dec 12, 2025
Merged

Introduce customisation mode option#308
dirkmueller merged 3 commits intoSUSE:mainfrom
atanasdinov:customisation-mode

Conversation

@atanasdinov
Copy link
Copy Markdown
Contributor

@atanasdinov atanasdinov commented Dec 11, 2025

Introduces a new --mode CLI parameter to customize command with the following options:

  • embedded (default): Customising an image will embed the Ignition partition within the resulting image
  • split: Customising an image will provide the ready-to-install image and Ignition configuration directory separately

The use cases that are going to utilise this feature are:

  • CAPI: Base image only without any configuration embedded
  • Any environment where having the configuration separate is desired
  • Baseline image testing: Customise an image only once and generate various Ignition configurations only on demand (once we add in --mode config-only)

In order to utilise the configuration directory in a QEMU VM, you can simply:

$ mkisofs -o ignition.iso -V ignition <config-dir>
$ qemu-kvm ... -cdrom ignition.iso

Note: Currently the networking, Ignition data, and custom scripts are going to be extracted to the configuration directory. All files which are part of the overlay (incl. systemd extensions, Kubernetes manifests, Helm charts) will proceed to be part of the image. We will have to properly distribute those to the configuration directory instead later.

@atanasdinov atanasdinov marked this pull request as ready for review December 11, 2025 16:22
@atanasdinov atanasdinov requested a review from a team as a code owner December 11, 2025 16:22
Comment on lines +91 to +92
func resolveOutputPaths(args *cmd.CustomizeFlags) (imagePath, configPath string) {
imagePath = args.OutputPath
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would a nil check be worth it here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't seem to understand -- the args variable cannot be nil no matter what we do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was reviewing the code when I should have been sleeping instead.

return nil
}

func resolveOutputPaths(args *cmd.CustomizeFlags) (imagePath, configPath string) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All the others functions doesn't declare output variables but only the type, maybe good to be consistent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's the other way around. It's good to name those where the caller would instead have to guess.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I understand correctly, configPath is not supposed to be set when mode is embedded? can we capture that in a comment perhaps?

Comment thread internal/cli/cmd/customize.go Outdated
},
&cli.StringFlag{
Name: "mode",
Usage: "Customization mode, 'embedded' or 'split'",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is quite hard to understand for me. maybe

   Usage:  "configuration output mode, 'embedded' (config within image, default) or 'split' (config separate to image)" 

baseName := strings.TrimSuffix(filename, filepath.Ext(filename))

configPath = filepath.Join(outputDir, baseName+"-config")
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we at least warn if args.Mode is neither "split" nor "combined" ? typos galore otherwise.

DefaultText: "image-<timestamp>.<image-type>",
},
&cli.StringFlag{
Name: "mode",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe we could just make it a boolean, --split-config or something like that? "--mode" is such a generic word, we might ourselves need to use it in some other context in two years from now.

@dirkmueller
Copy link
Copy Markdown
Member

dirkmueller commented Dec 11, 2025

@atanasdinov my apologies, I merged the other patch and now this conflicts. let me know if I should rebase.

atanasdinov and others added 3 commits December 11, 2025 23:08
Signed-off-by: Atanas Dinov <atanas.dinov@suse.com>
Signed-off-by: Atanas Dinov <atanas.dinov@suse.com>
@dirkmueller dirkmueller merged commit 7fb28b6 into SUSE:main Dec 12, 2025
4 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

Development

Successfully merging this pull request may close these issues.

5 participants