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

overlay: add 40coreos-network dracut module #89

Merged

Conversation

yuqi-zhang
Copy link
Contributor

Add coreos-teardown-initramfs-network.service to run after ignition
has finished using initramfs networking, so NetworkManager properly
brings up ignition-configured networking in the real root. Otherwise
the initramfs network persists into the real root.

Note: I'm not sure what the correct priority is, so I made it 40 to match other services.
It shouldn't really matter since this service actually runs during ExecStop and
before Before=initrd-switch-root.target.

@dustymabe
Copy link
Member

so for RHCOS we have this in the ignition-dracut repo and for FCOS we have it here? should we merge those two strategies in the future?

@jlebon
Copy link
Member

jlebon commented May 8, 2019

should we merge those two strategies in the future?

Once RHCOS rebases to spec3, this will have to happen (i.e. it'll have to be part of the RHCOS overlay/). So it'll converge eventually. But yeah, could definitely do this now too.

Conflicts=initrd-switch-root.target umount.target
Before=initrd-switch-root.target

# Make sure if ExecStart= fails, the boot fails
Copy link
Member

Choose a reason for hiding this comment

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

This should read ExecStop=.

@yuqi-zhang yuqi-zhang force-pushed the add-remove-initramfs-network-script branch from afa6203 to 7b19356 Compare May 9, 2019 19:49
@yuqi-zhang
Copy link
Contributor Author

Fixed up, thanks!

should we merge those two strategies in the future?

And I would vote to do this after GA at the earliest, but I agree we should match the two eventually.

set -euo pipefail

# Clean up the interfaces set up in the initramfs
# This mimics the behaviour of dracut's ifdown() in net-lib.sh
Copy link
Member

Choose a reason for hiding this comment

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

hmm - if it mimics that code then why not source it and run it directly rather than implement our own?

Copy link
Member

Choose a reason for hiding this comment

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

bump

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, this is the original function: https://github.com/dracutdevs/dracut/blob/master/modules.d/40network/net-lib.sh#L97
It does some things we don't necessarily need/want, and I recall having issues running it directly, so I opted to take a snippet of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want I can look into making that work instead? I didn't think it was necessary. We are likely going to try to move for NM managed dracut networking. See: https://github.com/dracutdevs/dracut/tree/master/modules.d/35network-manager

Copy link
Member

Choose a reason for hiding this comment

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

ehh - if it's temporary then it's fine IMHO - should we mention that NM thing in a comment?

@bgilbert
Copy link
Contributor

For FCOS, at least, this is a temporary fix until we can move away from dracut's networking scripts. In the meantime, I'm fine with merging it.

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

Add coreos-teardown-initramfs-network.service to run after ignition
has finished using initramfs networking, so NetworkManager properly
brings up ignition-configured networking in the real root. Otherwise
the initramfs network persists into the real root.

Signed-off-by: Yu Qi Zhang <[email protected]>
@yuqi-zhang yuqi-zhang force-pushed the add-remove-initramfs-network-script branch from 7b19356 to 474e6e3 Compare May 23, 2019 15:12
@yuqi-zhang
Copy link
Contributor Author

Rebased and added comment, feel free to merge

@bgilbert bgilbert merged commit 24e952e into coreos:master May 23, 2019
c4rt0 pushed a commit to c4rt0/fedora-coreos-config that referenced this pull request Mar 27, 2023
Jenkinsfile.cloud: Write AMI json to images directory
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.

4 participants