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

Succesor for PR 2331 [fix reserve joinSwitch LRP IPs] #2434

Merged

Conversation

alexanderConstantinescu
Copy link
Collaborator

- What this PR does and why is it needed

I have no clue what is happening with Github and its actions on PR: #2331 , it's cancelling the build during every run. Hence try to get a CI signal on it here.

- Special notes for reviewers

- How to verify it

- Description for the changelog

master.go: Use of ensureJoinLRPIPs, which also checks the running DB.

logical_switch_manager: ensureJoinLRPIPs now also looks into the running DB and fills the cache on a hit.

Move getJoinLRPAddresses from gateway to logical_switch_manager

Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
logical_switch_manager: During startup, getJoinLRPAddresses validates the
active joinLRPAddress against the node's subnet, but because of
the early state, the node's subnets are empty, instead we should
validate against the join switch's subnets that are already initialised.

Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 51.758% when pulling cdf2795 on alexanderConstantinescu:fix_reserve into 2249afa on ovn-org:master.

@Reamer
Copy link
Contributor

Reamer commented Aug 20, 2021

Thank you for pushing for these changes.

@Reamer
Copy link
Contributor

Reamer commented Aug 23, 2021

/retest

@alexanderConstantinescu alexanderConstantinescu changed the title [soak]: PR: 2331 (fix_reserve) Succesor for PR 2331 [fix reserve joinSwitch LRP IPs] Aug 23, 2021
@alexanderConstantinescu
Copy link
Collaborator Author

This PR is a successor to #2331 , since that PR is bonkers...

This PR passes CI and has the exact same commits as #2331 (which has an lgtm)

@alexanderConstantinescu
Copy link
Collaborator Author

Merging this, following agreement: #2331 (comment)

@alexanderConstantinescu alexanderConstantinescu merged commit fc40188 into ovn-org:master Aug 23, 2021
alexanderConstantinescu added a commit to alexanderConstantinescu/ovn-kubernetes that referenced this pull request Aug 31, 2021
After the modifications made to `ensureJoinLRPIPs` by PR: ovn-org#2434 the
function "lost" thread-safety. This meant that we could have one
go-routine - spawned by the network policy watchers - and another by the
node watcher, race and try to `Allocate` the same IP for the same
logical switch. Causing the second to error with `ErrAllocated`. We can
re-work the existing locks and have `ensureJoinLRPIPs` and
`releaseJoinLRPIPs` locks that span the caching functions, ensuring that
that all cache operations are thread-safe.

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
trozet pushed a commit to trozet/ovn-kubernetes that referenced this pull request Sep 14, 2021
After the modifications made to `ensureJoinLRPIPs` by PR: ovn-org#2434 the
function "lost" thread-safety. This meant that we could have one
go-routine - spawned by the network policy watchers - and another by the
node watcher, race and try to `Allocate` the same IP for the same
logical switch. Causing the second to error with `ErrAllocated`. We can
re-work the existing locks and have `ensureJoinLRPIPs` and
`releaseJoinLRPIPs` locks that span the caching functions, ensuring that
that all cache operations are thread-safe.

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
andreaskaris pushed a commit to andreaskaris/ovn-kubernetes that referenced this pull request Oct 6, 2021
After the modifications made to `ensureJoinLRPIPs` by PR: ovn-org#2434 the
function "lost" thread-safety. This meant that we could have one
go-routine - spawned by the network policy watchers - and another by the
node watcher, race and try to `Allocate` the same IP for the same
logical switch. Causing the second to error with `ErrAllocated`. We can
re-work the existing locks and have `ensureJoinLRPIPs` and
`releaseJoinLRPIPs` locks that span the caching functions, ensuring that
that all cache operations are thread-safe.

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
astoycos pushed a commit to astoycos/ovn-kubernetes that referenced this pull request Nov 30, 2021
After the modifications made to `ensureJoinLRPIPs` by PR: ovn-org#2434 the
function "lost" thread-safety. This meant that we could have one
go-routine - spawned by the network policy watchers - and another by the
node watcher, race and try to `Allocate` the same IP for the same
logical switch. Causing the second to error with `ErrAllocated`. We can
re-work the existing locks and have `ensureJoinLRPIPs` and
`releaseJoinLRPIPs` locks that span the caching functions, ensuring that
that all cache operations are thread-safe.

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
kyrtapz pushed a commit to kyrtapz/ovn-kubernetes that referenced this pull request Nov 21, 2022
After the modifications made to `ensureJoinLRPIPs` by PR: ovn-org#2434 the
function "lost" thread-safety. This meant that we could have one
go-routine - spawned by the network policy watchers - and another by the
node watcher, race and try to `Allocate` the same IP for the same
logical switch. Causing the second to error with `ErrAllocated`. We can
re-work the existing locks and have `ensureJoinLRPIPs` and
`releaseJoinLRPIPs` locks that span the caching functions, ensuring that
that all cache operations are thread-safe.

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
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

3 participants