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

Jump to bottom

overlay.d: move CoreOS specific dracut modules here #526

Merged
merged 2 commits into from Jul 27, 2020
Merged

overlay.d: move CoreOS specific dracut modules here #526

merged 2 commits into from Jul 27, 2020

Conversation

darkmuggle
Copy link
Contributor

@darkmuggle darkmuggle commented Jul 21, 2020 •
edited

With Ignition v2.5.0 Ignition-Dracut will be merged into Ignition itself.
Per [1] it was decided to move the CoreOS specific bits over here in this repo.

This includes a transitional hack (yes its an ugly, terrible, no-good hack) but the hack is intended to be short term until the 2.5.0 Ignition lands. Once 2.5.0 lands dracut/31ignition can be burned.

[1] coreos/fedora-coreos-tracker#511

@darkmuggle darkmuggle added WIP PR still being worked on hold labels Jul 21, 2020
@darkmuggle
Copy link
Contributor Author

Blocked on coreos/ignition#1039

Posting this for early discussion.

This was referenced Jul 21, 2020
Closed
Merged
jlebon
jlebon approved these changes Jul 22, 2020
View reviewed changes
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some minor comments, but LGTM overall.
Thanks for tackling this!

overlay.d/05core/usr/lib/dracut/modules.d/31ignition-coreos/coreos-diskful-generator Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/31ignition-coreos/coreos-teardown-initramfs.sh Outdated Show resolved Hide resolved
@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 2 times, most recently from 1ce8a11 to ead5fce Compare July 23, 2020 01:12
@darkmuggle darkmuggle changed the title WIP: dracut/31ignition-coreos: move CoreOS specific dracut modules overlay.d: move CoreOS specific dracut modules here Jul 23, 2020
@darkmuggle darkmuggle removed WIP PR still being worked on hold labels Jul 23, 2020
@darkmuggle darkmuggle marked this pull request as ready for review July 23, 2020 01:17
@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 2 times, most recently from fbda194 to ff2b9e2 Compare July 23, 2020 01:43
@darkmuggle
Copy link
Contributor Author

darkmuggle commented Jul 23, 2020 •
edited

Ugh @jlebon looks like my local testing was flawed. I'm not sure we can remove 30ignition/<stuff> since it breaks the build. The Ignition package puts Ignition in /usr/lib/dracut/modules.d/30ignition/ignition. So we can't just delete the dracut modules from the Ignition package unless we put the ignition binary in the source tree.

I was not able to find documentation about providing in-tree overrides.

@jlebon
Copy link
Member

jlebon commented Jul 23, 2020 •
edited

Ugh @jlebon looks like my local testing was flawed. I'm not sure we can remove 30ignition/ since it breaks the build. The Ignition package puts Ignition in /usr/lib/dracut/modules.d/30ignition/ignition. So we can't just delete the dracut modules from the Ignition package unless we put the ignition binary in the source tree.

So here's an idea that was discussed this morning:

  • in the Ignition PR, rename the dracut module to e.g. ignition-v3
  • in our module-setup.sh here, we do something like:
if dracut_module_included "ignition"; then
	exit 0
fi

so that it's a no-op if we're on the old Ignition.

  • we merge this PR
  • we merge the Ignition PR
  • release new Ignition
  • bump Ignition in FCOS
  • at some point later on, we can drop the module-setup.sh hack, and rename back the module in Ignition to ignition.

The advantage of this is that in the Ignition PR, we get to test the final state in CI before merging and cutting a release.

The PR's CI will have to also sed this line to --add=ignition-v3.

@darkmuggle
Copy link
Contributor Author

As discussed in IRC, the mechanics of getting everything in place before the 2.5.0 release is more risky than just cutting over. Once 2.5.0 lands, this will be ready.

Pending 2.5.0 release an RPM build.

/hold

@bgilbert bgilbert mentioned this pull request Jul 24, 2020
Merged
@bgilbert
Copy link
Contributor

Ignition 2.5.0 is packaged: https://bodhi.fedoraproject.org/updates/FEDORA-2020-3162a47c9f

bgilbert
bgilbert reviewed Jul 24, 2020
View reviewed changes
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

LGTM generally. Some nits, including a couple things that came over verbatim from ignition-dracut.

overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/coreos-diskful-generator Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/coreos-diskful-generator Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/coreos-gpt-setup.service Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/module-setup.sh Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/module-setup.sh Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/30ignition-coreos/module-setup.sh Outdated Show resolved Hide resolved
...ore/usr/lib/dracut/modules.d/99emergency-timeout-coreos/ignition-virtio-dump-journal.service Outdated Show resolved Hide resolved
...ore/usr/lib/dracut/modules.d/99emergency-timeout-coreos/ignition-virtio-dump-journal.service Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/99emergency-timeout-coreos/module-setup.sh Outdated Show resolved Hide resolved
overlay.d/05core/usr/lib/dracut/modules.d/99journald-conf-coreos/module-setup.sh Outdated Show resolved Hide resolved
@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 4 times, most recently from b27518b to d9936cc Compare July 24, 2020 16:03
@darkmuggle
Copy link
Contributor Author

Confirmed against https://bodhi.fedoraproject.org/updates/FEDORA-2020-3162a47c9f

@jlebon
Copy link
Member

jlebon commented Jul 24, 2020

CI is failing because of the file conflicts. Want to include the Ignition override as part of this PR? See #522 for an example.

@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 2 times, most recently from 1044c3b to a9ac7b7 Compare July 27, 2020 16:21
@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 2 times, most recently from 25d7a26 to 53c1339 Compare July 27, 2020 16:49
@darkmuggle darkmuggle force-pushed the pr/dracut-coreos branch 3 times, most recently from d90a385 to c016c0a Compare July 27, 2020 17:53
jlebon
jlebon reviewed Jul 27, 2020
View reviewed changes
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Needs one last fix, and then I think this is good to go!

manifest-lock.x86_64.json Outdated Show resolved Hide resolved
Ben Howard added 2 commits July 27, 2020 13:29
overrides: fast track Ignition 2.5.0
4d9223a 
Signed-off-by: Ben Howard <ben.howard@redhat.com>
dracut: adopt CoreOS specific modules from ignition-dracut
4871828 
With the merging of ignition-dracut into Ignition starting with version
2.5.0, we are moving the 99* and 30ignition CoreOS specific modules here.

Signed-off-by: Ben Howard <ben.howard@redhat.com>
@darkmuggle darkmuggle merged commit d5b6647 into coreos:testing-devel Jul 27, 2020
@darkmuggle darkmuggle deleted the pr/dracut-coreos branch July 27, 2020 19:46
c4rt0 pushed a commit to c4rt0/fedora-coreos-config that referenced this pull request Mar 27, 2023
@openshift-merge-robot
Merge pull request coreos#526 from travier/cleanup
4495083 
Remove now unneeded sync-sources.sh script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlebon jlebon jlebon approved these changes

@bgilbert bgilbert bgilbert approved these changes

Assignees
No one assigned
Labels
hold
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@darkmuggle @jlebon @bgilbert