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

Jump to bottom

Add missing IPv6 support for OVN/OVS config #1965

Merged
merged 1 commit into from Aug 1, 2020
Merged

Add missing IPv6 support for OVN/OVS config #1965

merged 1 commit into from Aug 1, 2020

Conversation

trozet
Copy link
Contributor

@trozet trozet commented Jul 31, 2020

Previously we were only looking at IPv4 for default routes. This also
looks for IPv6 if IPv4 is not found. This patch also includes a fix for
jq returning null when it fails to read, which we were not previously
checking for.

Signed-off-by: Tim Rozet trozet@redhat.com

@trozet
Add missing IPv6 support for OVN/OVS config
3af2ff7 
Previously we were only looking at IPv4 for default routes. This also
looks for IPv6 if IPv4 is not found. This patch also includes a fix for
jq returning null when it fails to read, which we were not previously
checking for.

Signed-off-by: Tim Rozet <trozet@redhat.com>
@dcbw
Copy link
Member

dcbw commented Jul 31, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2020
@trozet trozet mentioned this pull request Jul 31, 2020
Merged
@trozet
Copy link
Contributor Author

trozet commented Jul 31, 2020

/assign @runcom

@trozet
Copy link
Contributor Author

trozet commented Jul 31, 2020

/retest

danwinship
danwinship reviewed Jul 31, 2020
View reviewed changes
templates/common/_base/files/configure-ovs-network.yaml
iface=$(ip -j route show default | jq -r '.[0].dev')
if [ -n "$iface" ]; then
echo "Default gateway interface found: ${iface}"
if [[ -n "$iface" && "$iface" != "null" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw you can do jq -r '.[0].dev // empty' to avoid the "null" output
(you can also use jq -e to exit with non-0 status on "null", but I don't think that would help here)

@runcom
Copy link
Member

runcom commented Jul 31, 2020

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, runcom, trozet

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@kikisdeliveryservice
Copy link
Contributor

@trozet do we expect this e2e-ovn-step-registry test to pass?

@trozet
Copy link
Contributor Author

trozet commented Jul 31, 2020

@trozet do we expect this e2e-ovn-step-registry test to pass?

I expect it to only fail 1 case for that job:
openshift/origin#25356

However, I can see this patch worked in the must-gather journal for the node:

Jul 31 15:47:10.998720 ip-10-0-152-53 configure-ovs.sh[1638]: OVS successfully configured
Jul 31 15:47:10.998960 ip-10-0-152-53 configure-ovs.sh[1638]: + ip a show br-ex
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]: 4: br-ex: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9001 qdisc noqueue state UNKNOWN group default qlen 1000
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]:     link/ether 0a:d9:b7:51:75:71 brd ff:ff:ff:ff:ff:ff
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]:     inet 10.0.152.53/18 brd 10.0.191.255 scope global dynamic noprefixroute br-ex
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]:        valid_lft 3595sec preferred_lft 3595sec
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]:     inet6 fe80::319b:c1cf:8fc1:a9bc/64 scope link noprefixroute
Jul 31 15:47:11.002879 ip-10-0-152-53 configure-ovs.sh[1638]:        valid_lft forever preferred_lft forever
Jul 31 15:47:11.012168 ip-10-0-152-53 configure-ovs.sh[1638]: + exit 0
Jul 31 15:47:11.007248 ip-10-0-152-53 systemd[1]: Started Configures OVS with proper host networking configuration

@dcbw
Copy link
Member

dcbw commented Jul 31, 2020

/retest

@kikisdeliveryservice
Copy link
Contributor

/test e2e-gcp-op

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit d8d7ca7 into openshift:master Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danwinship danwinship danwinship left review comments

@kikisdeliveryservice kikisdeliveryservice Awaiting requested review from kikisdeliveryservice

@sinnykumari sinnykumari Awaiting requested review from sinnykumari

Assignees

@runcom runcom

@dcbw dcbw

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@trozet @dcbw @runcom @openshift-ci-robot @openshift-bot @kikisdeliveryservice @danwinship @openshift-merge-robot