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

initial pods are not wired for hybrid overlay #3595

Merged
merged 1 commit into from
May 19, 2023

Conversation

JacobTanenbaum
Copy link
Contributor

When toggling hybrid overlay the linux nodes can miss setting up the initial pods on start-up if allocating the hybrid overlay DRIP and DRMAC takes too long.

Fix this issue by passing a podLister to the node controller and running the AddPods() command on all local pods once on startup after DRIP and DRMAC are set.

- What this PR does and why is it needed

- Special notes for reviewers

- How to verify it

- Description for the changelog

@@ -476,7 +497,7 @@ func (n *NodeController) handleHybridOverlayMACIPChange(node *kapi.Node) error {

// EnsureHybridOverlayBridge sets up the hybrid overlay bridge
func (n *NodeController) EnsureHybridOverlayBridge(node *kapi.Node) error {
if atomic.LoadUint32(&n.initialized) == 1 {
if atomic.LoadUint32(&n.initialized) >= 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change the atomic comparisons to constants? Now you are using a new value of 2, but I'm not sure what that indicates (without referring to your comment in the struct). So maybe const iota with their names suggesting what the event is you are looking for would be clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trozet changed the states to const using iota, how does it look now?

@JacobTanenbaum JacobTanenbaum force-pushed the windows-toggle branch 2 times, most recently from a944392 to 5109dfc Compare May 17, 2023 16:59
@coveralls
Copy link

coveralls commented May 17, 2023

Coverage Status

Changes Unknown when pulling 61541cf on JacobTanenbaum:windows-toggle into ** on ovn-org:master**.

When toggling hybrid overlay the linux nodes can miss wiring the pods on
start-up if allocating the hybrid overlay DRIP and DRMAC takes too long.

Fix this issue by passing a podLister to the node controller and running
the AddPods() command on all local pods once on startup after DRIP and
DRMAC are set.

Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
@dcbw dcbw requested review from dcbw and trozet May 18, 2023 21:06
Copy link
Contributor

@dcbw dcbw left a comment

Choose a reason for hiding this comment

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

LGTM now

@trozet trozet merged commit 9fcc659 into ovn-org:master May 19, 2023
22 checks passed
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

4 participants