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

hybrid-overlay: fix host route to hybrid overlay subnets #1385

Merged
merged 1 commit into from
Jun 8, 2020

Conversation

dcbw
Copy link
Contributor

@dcbw dcbw commented Jun 5, 2020

Don't use direct byte manipulation of the node subnet, otherwise
it's changed for all subsequent uses and that causes the host route
via the management port for the hybrid overlay cluster subnet to
get a .6 address instead of the expected .3.

@trozet @rcarrillocruz @dave-tucker

@coveralls
Copy link

coveralls commented Jun 5, 2020

Coverage Status

Coverage increased (+0.1%) to 53.066% when pulling d17f517 on dcbw:fix-hybrid-overlay-host-route into eeb8d87 on ovn-org:master.

@nerdalert
Copy link
Collaborator

First time I have seen that e2e test fail. I will address the retry timer for getting the destination pod IP with a PR. Ty.

@trozet
Copy link
Contributor

trozet commented Jun 5, 2020

/retest

@dcbw
Copy link
Contributor Author

dcbw commented Jun 5, 2020

/hold
need to figure out some network namespace issues with the testcase

@dcbw dcbw changed the title hybrid-overlay: fix host route to hybrid overlay subnets [wip] hybrid-overlay: fix host route to hybrid overlay subnets Jun 5, 2020
@dcbw dcbw force-pushed the fix-hybrid-overlay-host-route branch from 3ba375b to c52ec34 Compare June 8, 2020 02:33
@dcbw dcbw changed the title [wip] hybrid-overlay: fix host route to hybrid overlay subnets hybrid-overlay: fix host route to hybrid overlay subnets Jun 8, 2020
@dcbw
Copy link
Contributor Author

dcbw commented Jun 8, 2020

@trozet @nerdalert fixed now, PTAL thanks

@@ -290,7 +308,6 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() {

//FIXME
//Expect(fexec.CalledMatchesExpected()).To(BeTrue(), fexec.ErrorDesc)
validateNetlinkState(node1Subnet)
Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm

but does the rewriting of the test fix the scattered CalledMatchesExpected FIXMEs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danwinship no, it doesn't touch the ovs resync logic that broke the CalledMatchesExpected() stuff.

Don't use direct byte manipulation of the node subnet, otherwise
it's changed for all subsequent uses and that causes the host route
via the management port for the hybrid overlay cluster subnet to
get a .6 address instead of the expected .3.

In addition the testcases didn't have the right hybrid overlay
CLI options to enable the right code checks.

Signed-off-by: Dan Williams <dcbw@redhat.com>
@dcbw dcbw force-pushed the fix-hybrid-overlay-host-route branch from c52ec34 to d17f517 Compare June 8, 2020 14:27
@dcbw
Copy link
Contributor Author

dcbw commented Jun 8, 2020

Repushed to fix conflict.

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.

None yet

5 participants