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.

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

RFC: Merge coreos/ignition-dracut into coreos/ignition #511

Closed
darkmuggle opened this issue Jun 1, 2020 · 13 comments
Closed

RFC: Merge coreos/ignition-dracut into coreos/ignition #511

darkmuggle opened this issue Jun 1, 2020 · 13 comments
Assignees
Labels
jira for syncing to jira

Comments

@darkmuggle
Copy link
Contributor

Currently, this repo and Ignition-Dracut are each housed in a separate Github Repository

In the course of several PR's such as coreos/ignition#960 and coreos/ignition#956 it's been referenced that some in-development features would need implementation in Ignition-Dracut.

Merging the two would:

  • reduce the burden of maintenance on both Ignition and the Dracut modules in source repos
  • make the introduction of features easier since the feature could be bound to a single commit.
  • make distro packaging cleaner. In the RPM's used by Fedora CoreOS, for example, we have to source both Ignition and the Dracut code separately, but we use the Ignition branch in the package version. I would argue that this should make back-porting fixes for distros easier.
@darkmuggle
Copy link
Contributor Author

/cc @laenion from SuSE

@jlebon jlebon changed the title RFC: Merge Ignition and Ignition-Dracut togher. RFC: Merge coreos/ignition-dracut into coreos/ignition Jun 1, 2020
@jlebon jlebon transferred this issue from coreos/ignition Jun 1, 2020
@jlebon jlebon added the meeting topics for meetings label Jun 1, 2020
@jlebon
Copy link
Member

jlebon commented Jun 2, 2020

+1 from me. I think we'll probably want to move the following functionality into fedora-coreos-config:

dracut/30ignition/coreos-teardown-initramfs.sh
dracut/30ignition/coreos-gpt-setup.sh
dracut/30ignition/[email protected]
dracut/30ignition/coreos-teardown-initramfs.service
dracut/99journald-conf/module-setup.sh
dracut/99journald-conf/00-journal-log-forwarding.conf

Some debatable ones:

dracut/99emergency-timeout/ignition-virtio-dump-journal.service
dracut/99emergency-timeout/ignition-virtio-dump-journal.sh
dracut/99emergency-timeout/module-setup.sh
dracut/99emergency-timeout/timeout.sh

@darkmuggle
Copy link
Contributor Author

I'm leaning towards putting dracut/99emergency-timeout/* in Fedora CoreOS Config. The ignition-virtio-dump-journal.* came of features in Kola and its use-cases is for testing. I think the better-path is to put that in FCOS Config and then come up a generic console dump that is configurable. But I can go either way on that myself as well.

@dustymabe
Copy link
Member

  • make distro packaging cleaner. In the RPM's used by Fedora CoreOS, for example, we have to source both Ignition and the Dracut code separately, but we use the Ignition branch in the package version. I would argue that this should make back-porting fixes for distros easier.

What we have right now in our spec files where we just bump the git hash for ignition-dracut is pretty easy to do and we often do it to pick up small changes that were made to the dracut module or the systemd units. I think the only downside I can see with doing this is that now it's not quite as easy to pick up just the changes from ignition-dracut (i.e. we'd probably have to do more "patches" rather than just updating the hash for a separate repo) and not all of ignition upstream.

@laenion
Copy link

laenion commented Jun 3, 2020

I don't mind either way, but keeping the files in sync and releasing them together seems like a good idea.

+1 for splitting out the coreos- prefixed files, those don't make any sense in Ignition's core distribution. The other files don't hurt, but I won't miss them either.

@dustymabe
Copy link
Member

We discussed this during the meeting today:

  * AGREED: Merging ignition-dracut into ignition seems to have clear
    advantages and the disadvantages seem to be minor or easily worked
    around. We'll merge them together in the future (when it's safest to
    do so) unless clear blockers come up when attempting the transition.
    (dustymabe, 17:16:33)

@darkmuggle
Copy link
Contributor Author

darkmuggle commented Jul 21, 2020

In light of the planning meeting for the approach tomorrow, I've put together a POC -- with no expectation in regards to fitness or that it will ever merge -- with the following:

At a high level this:

  • Merged Ignition-Dracut into Ingition and preserved the histories
  • Fixed the Makefile to support the new Dracut code
  • Moved the CoreOS specific bits to Fedora CoreOS Config
  • Updated the RPM specs
  • Confirms a build

All this produced a usable FCOS build that passed KOLA tests:

[email protected]:~/src/builder/fcos $ fcosa kola run coreos.ignition.*
⚠️  Skipping kola test pattern "fcos.internet":
⚠️  https://github.com/coreos/coreos-assembler/pull/1478
⚠️  Skipping kola test pattern "podman.workflow":
⚠️  https://github.com/coreos/coreos-assembler/pull/1478
kola -p qemu-unpriv --output-dir tmp/kola run coreos.ignition.* --denylist-test fcos.internet --denylist-test podman.workflow
=== RUN   coreos.ignition.groups
=== RUN   coreos.ignition.sethostname
=== RUN   coreos.ignition.journald-log
=== RUN   coreos.ignition.failure
=== RUN   coreos.ignition.instantiated.enable-service
=== RUN   coreos.ignition.v2.users
=== RUN   coreos.ignition.resource.remote
=== RUN   coreos.ignition.once
=== RUN   coreos.ignition.mount.disks
=== RUN   coreos.ignition.mount.partitions
--- PASS: coreos.ignition.groups (36.30s)
--- PASS: coreos.ignition.resource.remote (38.50s)
--- PASS: coreos.ignition.mount.partitions (70.42s)
--- PASS: coreos.ignition.mount.disks (70.09s)
--- PASS: coreos.ignition.once (70.10s)
2020-07-21T18:51:17Z platform: Error killing qemu instance 117: exec: Wait was already called
--- PASS: coreos.ignition.failure (17.18s)
--- PASS: coreos.ignition.v2.users (37.14s)
--- PASS: coreos.ignition.instantiated.enable-service (36.19s)
--- PASS: coreos.ignition.sethostname (35.46s)
--- PASS: coreos.ignition.journald-log (35.60s)
PASS, output in tmp/kola

@cgwalters
Copy link
Member

What about Flatcar? They're still using the spec2x branch right? Or maybe they've already got things forked. Does someone have a handy contact for them? cc @blixtra

@bgilbert
Copy link
Contributor

Container Linux had its dracut modules in bootengine, not here, and the change won't affect the Ignition spec2x branch. So this shouldn't affect Flatcar.

@darkmuggle
Copy link
Contributor Author

After discussing this with @jlebon @cgwalters @bgilbert @miabbott and @arithx we have agreed to the following:

  1. Move CoreOS specific modules into Fedora Core Config. Add remove directives for the conflicting files and temporarily host the needed files from Ignition in FCOS's configuration until Ignition has it.
  2. Merge in Ignition-Dracut into Ignition and cut release 2.5.0
  3. Update Ignition spec in FCOS to 2.5.0 and build
  4. Bake the changes FCOS
  5. Archive Ignition-Dracut
  6. Update Ignition spec in RHCOS to 2.5.0 and build

Acceptance Criteria:

  • FCOS must past KOLA tests
  • RHCOS must past KOLA tests

In general, the consensus is that this a low-risk operation since we are not introducing new features; rather we are moving code around.

Timeline:

  1. By EOD Friday: merge the changes needed for the transitional changes to FCOS by Friday
  2. Cut a new Ignition release Tuesday

@vbatts
Copy link

vbatts commented Jul 23, 2020

like @bgilbert said. That is handled in bootengine currently. Flatcar is still on the 2x spec. Planning the reconcile of this currently. /cc @pothos

@darkmuggle
Copy link
Contributor Author

Changes for both Ignition and ignition-dracut have landed. FCOS and RHCOS 4.6 has Ignition 2.5.0.

Closing this as complete.

@dustymabe dustymabe added the status/pending-testing-release Fixed upstream. Waiting on a testing release. label Jul 30, 2020
@dustymabe
Copy link
Member

The fix for this went into testing stream release 32.20200809.2.1. Please try out the new release and report issues.

@dustymabe dustymabe removed the status/pending-testing-release Fixed upstream. Waiting on a testing release. label Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira for syncing to jira
Projects
None yet
Development

No branches or pull requests

7 participants