Skip to content
This repository was archived by the owner on Aug 25, 2021. It is now read-only.

Add ignition-complete.target#55

Merged
dustymabe merged 2 commits intocoreos:masterfrom
jlebon:pr/ignition-target
Mar 14, 2019
Merged

Add ignition-complete.target#55
dustymabe merged 2 commits intocoreos:masterfrom
jlebon:pr/ignition-target

Conversation

@jlebon
Copy link
Member

@jlebon jlebon commented Mar 6, 2019

From the comment block of the unit:

This target is reached when Ignition finishes running. Note that it
gets activated *only* on first boot (or if ignition.firstboot=1 is
provided).  Thus, it is also an API for units to use so that they
are activated only on first boot. Simply add a link under
ignition-complete.target.requires in the initrd.

Thus, units themselves don't need to add explicit Requires=; the
canonical way to get pulled into the transaction is through
ignition-complete.target.

This will also be used by the upcoming CoreOS units that are required
for separate /var support.

jlebon added 2 commits March 7, 2019 11:36
There's no need to generate it dynamically, so let's just split it out
to be more explicit and easier to hack on.
From the comment block of the unit:

    This target is reached when Ignition finishes running. Note that it
    gets activated *only* on first boot (or if ignition.firstboot=1 is
    provided).  Thus, it is also an API for units to use so that they
    are activated only on first boot. Simply add a link under
    ignition-complete.target.requires in the initrd.

Thus, units themselves don't need to add explicit `Requires=`; the
canonical way to get pulled into the transaction is through
`ignition-complete.target`.

This will also be used by the upcoming CoreOS units that are required
for separate `/var` support.
@jlebon jlebon force-pushed the pr/ignition-target branch from 53bc163 to f545dad Compare March 7, 2019 17:01
@jlebon jlebon changed the title WIP: Add ignition.target Add ignition-complete.target Mar 7, 2019
@jlebon
Copy link
Member Author

jlebon commented Mar 7, 2019

OK, renamed to ignition-complete.target, updated the PR description with the commit message, and dropped WIP!

Copy link
Contributor

@ajeddeloh ajeddeloh left a comment

Choose a reason for hiding this comment

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

LGTM, though I haven't tested it.

@dustymabe
Copy link
Member

LGTM

@bgilbert
Copy link
Contributor

Second commit LGTM. Re the first commit, how do you plan to differentiate ignition-setup behavior on live PXE systems?

@jlebon
Copy link
Member Author

jlebon commented Mar 13, 2019

Re the first commit, how do you plan to differentiate ignition-setup behavior on live PXE systems?

Can we use unit overrides for this? My main goal here was keeping things clean and easy to follow by making ignition-setup.service its own file.

@bgilbert
Copy link
Contributor

Could do. It might be cleaner to get an ignition-setup cmdline arg from an env var. I'm fine moving the service to its own file; just wanted to check.

@dustymabe
Copy link
Member

dustymabe commented Mar 13, 2019

It's possible, depending on how we implement things, that we won't need a separate behaving ignition-setup.service in the future. Maybe we just cross that bridge when we add live PXE support?

@jlebon
Copy link
Member Author

jlebon commented Mar 14, 2019

Are we good to merge this then?

@ajeddeloh
Copy link
Contributor

👍 Fine by me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants