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

WIP: 30ignition: remove initramfs networking #77

Closed

Conversation

yuqi-zhang
Copy link

Add coreos-remove-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
Copy link
Author

I have not tested operation on FCOS, only RHCOS so far, see: #78 .

Note that the only case this is currently handling is interfaces but not e.g. bridges/bonds etc. for the RHCOS use case it is enough to unblock base use cases, and allow at least e.g. a static IP, so throwing this here for review.

cc @jlebon @bgilbert

@jlebon jlebon added the WIP PR still being worked on label Apr 30, 2019
@yuqi-zhang yuqi-zhang changed the title 30ignition: remove initramfs networking WIP: 30ignition: remove initramfs networking Apr 30, 2019

[Service]
Type=oneshot
ExecStart=/usr/sbin/coreos-remove-initramfs-network
Copy link
Member

Choose a reason for hiding this comment

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

Nit, let's make this /usr/libexec so it's not in default $PATH as a bash completion hazard.

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't exist in the real root so that's not an issue right? We also do this for e.g.https://github.com/coreos/ignition-dracut/blob/master/dracut/30ignition/coreos-populate-var.service#L14

@@ -0,0 +1,11 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Shell script always should have
set -euo pipefail

Copy link
Author

Choose a reason for hiding this comment

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

Done, have been testing in #78 since its more time-sensitive in RHCOS, will update this right after

# Clean up the interfaces set up in the initramfs
for f in /sys/class/net/*; do
interface=$(basename "$f")
ip link set $interface down
Copy link
Member

Choose a reason for hiding this comment

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

Is this enough to kill dhclient? I think we also want to loop over /run/dhclient*.pid or so, actually before doing the interfaces.

Copy link
Author

Choose a reason for hiding this comment

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

dhclient isn't actually running AFAICT in the initramfs (nothing from ps aux). It doesn't try to re-lease if I down the interfaces at least.

@yuqi-zhang yuqi-zhang closed this May 1, 2019
@yuqi-zhang yuqi-zhang force-pushed the fcos-clean-initramfs-networking branch from e87c38c to 85f2e65 Compare May 1, 2019 15:24
Add coreos-remove-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 reopened this May 1, 2019
@yuqi-zhang
Copy link
Author

Whoops, bad force-push

@ajeddeloh
Copy link
Contributor

After jlebon's PRs merge, we'll want to move this to the fcos-config repo instead of here

@jlebon
Copy link
Member

jlebon commented May 6, 2019

@yuqi-zhang Can you reopen this against https://github.com/coreos/fedora-coreos-config ? Would be good to get this working in FCOS as well.

@yuqi-zhang
Copy link
Author

Yep, sorry, will close and PR there

@yuqi-zhang yuqi-zhang closed this May 6, 2019
@yuqi-zhang
Copy link
Author

New PR here: coreos/fedora-coreos-config#89

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
WIP PR still being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants