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.

Sign up for GitHub

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

many: move machine id first boot to image config #1258

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+13 −13
Open

many: move machine id first boot to image config #1258

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e3c068
osbuild: fix typo in machine-id constants
supakeen Feb 25, 2025
585d2be
many: move machine id first boot to image config
supakeen Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/distro/fedora/distro.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ var defaultDistroImageConfig = &distro.ImageConfig{
Timezone: common.ToPtr("UTC"),
Locale: common.ToPtr("en_US"),
DefaultOSCAPDatastream: common.ToPtr(oscap.DefaultFedoraDatastream()),
FirstBootStrategy: common.ToPtr(osbuild.MachineIdFirstBootYes),
}

func defaultDistroInstallerConfig(d *distribution) *distro.InstallerConfig {
Expand Down
5 changes: 2 additions & 3 deletions pkg/distro/fedora/images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ func osCustomizations(
osc.CACerts = ca.PEMCerts
}

osc.FirstBootStrategy = imageConfig.FirstBootStrategy

return osc, nil
}

Expand Down Expand Up @@ -345,9 +347,6 @@ func diskImage(workload workload.Workload,

img.Filename = t.Filename()

d := t.arch.distro
img.FirstBoot = common.VersionGreaterThanOrEqual(d.osVersion, VERSION_FIRSTBOOT)

return img, nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/distro/image_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ type ImageConfig struct {
LockRootUser *bool

IgnitionPlatform *string

// How to handle the /etc/machine-id file
FirstBootStrategy *osbuild.MachineIdFirstBoot
Comment on lines +98 to +99
Copy link
Member

Choose a reason for hiding this comment

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

In general, we want to avoid using osbuild stage options or types as types in ImageConfig.

I'm also not too keen on the name. FirstBootStrategy sounds way too general, while all it does is manage machine-id... 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but the status quo right now is to use them. Do I need to introduce another type and then directly convert/map that onto osbuild types?

I don't particularly like it either (everything seems to be called something-FirstBoot). It controls "ConditionFirstBoot" which is where the name comes from :)

Copy link
Member Author

Choose a reason for hiding this comment

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

MachineIdStrategy?

Copy link
Member

Choose a reason for hiding this comment

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

MachineIdFirstBoot? 🙄 I like your suggestion, but the values make it weird.

I'll let others comment since there isn't a good option.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but the status quo right now is to use them. Do I need to introduce another type and then directly convert/map that onto osbuild types?

It is, but we want to move away from it. So, it seems natural not to use it for newly added options. So yes, ideally, you should add a new type to https://github.com/osbuild/images/tree/main/pkg/customizations if it can't be represented by a primitive type.

On the other hand, we do not need to necessarily map the stage options 1:1 to the ImageConfig option. Maybe adding just something like MachineIdUninitialized bool would be good enough? And the OS pipeline would just set the correct stage option if true.

}

// InheritFrom inherits unset values from the provided parent configuration and
Expand Down
3 changes: 0 additions & 3 deletions pkg/image/disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ type DiskImage struct {
// InstallWeakDeps enables installation of weak dependencies for packages
// that are statically defined for the payload pipeline of the image.
InstallWeakDeps *bool

FirstBoot bool
}

func NewDiskImage() *DiskImage {
Expand Down Expand Up @@ -68,7 +66,6 @@ func (img *DiskImage) InstantiateManifest(m *manifest.Manifest,
if img.InstallWeakDeps != nil {
osPipeline.InstallWeakDeps = *img.InstallWeakDeps
}
osPipeline.FirstBoot = img.FirstBoot

rawImagePipeline := manifest.NewRawImage(buildPipeline, osPipeline)
rawImagePipeline.PartTool = img.PartTool
Expand Down
6 changes: 3 additions & 3 deletions pkg/manifest/os.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ type OSCustomizations struct {
// FirstBoot sets if the machine-id should be written with the
// magic value that determines if the machine is being booted for the
// first time.
FirstBoot bool
FirstBootStrategy *osbuild.MachineIdFirstBoot
}

// OS represents the filesystem tree of the target image. This roughly
Expand Down Expand Up @@ -813,9 +813,9 @@ func (p *OS) serialize() osbuild.Pipeline {
pipeline.AddStage(osbuild.NewCAStageStage())
}

if p.FirstBoot {
if p.FirstBootStrategy != nil {
pipeline.AddStage(osbuild.NewMachineIdStage(&osbuild.MachineIdStageOptions{
FirstBoot: osbuild.MachineIdFirstBootYes,
FirstBoot: *p.FirstBootStrategy,
}))
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/manifest/os_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func testTomlPkgsFor(t *testing.T, os *OS) {
func TestFirstBootIncludesMachineIdStage(t *testing.T) {
os := NewTestOS()

os.FirstBoot = true
os.FirstBootStrategy = common.ToPtr(osbuild.MachineIdFirstBootYes)

pipeline := os.serialize()
st := findStage("org.osbuild.machine-id", pipeline.Stages)
Expand Down
Loading