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
Configure OVS after firstboot is done #2006
Configure OVS after firstboot is done #2006
Conversation
/cc @trozet @LorbusChris |
/approve |
@@ -6,8 +6,8 @@ contents: | | |||
# This service is used to move a physical NIC into OVS and reconfigure OVS to use the host IP | |||
Requires=openvswitch.service | |||
Wants=NetworkManager-wait-online.service | |||
After=NetworkManager-wait-online.service openvswitch.service node-valid-hostname.service | |||
Before=network-online.target kubelet.service crio.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was dropping Before=network-online.target
intentional?
How about adding ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json
to this service instead?
And we should do that for kubelet.service
and crio.service
too probably - basically everything we don't want to run until we've pivoted, rather than implicitly relying on having the firstboot service initiate a reboot before they've started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was dropping Before=network-online.target intentional?
Yes, after network-online
mcd-pull and mcd-firstboot started, so ovs-configuration would always happen after network-online anyway
How about adding ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json to this service instead?
Good idea, added that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it wouldn't hurt to leave it, though? Just in case we shuffle things around again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unnecessary After/Before/Wanted changed for now - this can be re-iterated later
1f535da
to
080a88e
Compare
080a88e
to
adc3603
Compare
I think this is a clear bugfix, and we should avoid starting services before pivoting in general. |
OKD is expected to fail for now - it needs an updated installer. |
It's not just jq, even without jq I don't think NM OVS will work. |
looks OK to me and ovn-step-registry passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, LorbusChris, trozet, vrutkovs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
@LorbusChris: LorbusChris unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@vrutkovs: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@vrutkovs is there a bug for this? |
@ashcrow I didn't file one (as it primarily affects OKD) |
Run OVS configuration only after the machine has pivoted. This is crucial for OKD installs which uses standard FCOS image with no OVS pre-installed.