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

Enable network service in RHEL8 #689

Merged
merged 1 commit into from
Jul 25, 2019
Merged

Enable network service in RHEL8 #689

merged 1 commit into from
Jul 25, 2019

Conversation

e-minguez
Copy link
Contributor

@e-minguez e-minguez commented Jul 24, 2019

It is disabled by default in RHEL8
Fixes #688

@@ -111,6 +111,9 @@ if [ "$MANAGE_INT_BRIDGE" == "y" ]; then
else
sudo systemctl restart network
fi
if [ "${RHEL8}" = "True" ] ; then
sudo systemctl enable network
fi
Copy link

Choose a reason for hiding this comment

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

I'm wondering how this ends up disabled? Was it a manual ISO install where no network was activated as part of the install?

Regardless this seems unrelated to the OS version, perhaps we should just unconditionally enable then restart it, to remove the restart duplication above?

Copy link

Choose a reason for hiding this comment

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

Ah I see the restart de-duplication is handled in #690 thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the enable only if the ifcfg-* files are modified

Copy link
Member

Choose a reason for hiding this comment

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

Is the network service only for handling ifcfg scripts? In RHEL8 that's not installed by default. We install it so we can still use ifcfg scripts. That's probably why it has to be enabled in RHEL8 and we didn't see that before.

In RHEL8 it is disabled by default.
Fixes #688
# If there were modifications to the /etc/sysconfig/network-scripts/ifcfg-*
# files, it is required to enable the network service
if [ "$MANAGE_INT_BRIDGE" == "y" ] || [ "$MANAGE_PRO_BRIDGE" == "y" ]; then
sudo systemctl enable network
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this was only needed for RHEL8 since we install network-scripts for ifcfg file support, which isn't there by default. I suppose this is harmless elsewhere though.

Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

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

lgtm, but want to let @hardys look again too

@hardys
Copy link

hardys commented Jul 24, 2019

This is fine, but I can't help wondering if we should just move to nmcli for RHEL8 instead?

Here's an example of configuring an interface with nmcli:

https://github.com/hardys/installer/blob/issue_2060/data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template#L29

Not blocking this as a quick-fix, but wanted to have that discussion and now seems a good time to mention it :)

@russellb
Copy link
Member

russellb commented Jul 24, 2019 via email

@e-minguez
Copy link
Contributor Author

e-minguez commented Jul 25, 2019

Can we have this merged then create an issue to track the NetworkManager modifications? Thanks!

(also, TIL about https://github.com/nmstate/nmstate)

@hardys
Copy link

hardys commented Jul 25, 2019

Can we have this merged then create an issue to track the NetworkManager modifications? Thanks!

(also, TIL about https://github.com/nmstate/nmstate)

Sure, and yes maybe nmstate would be a nice thing to investigate as an alternative to shell hacking :)

@hardys hardys merged commit 647b8a5 into openshift-metal3:master Jul 25, 2019
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.

Rebooting the RHEL8 helper makes it unreachable
3 participants