Verify LB ip and address pool annotations compatibility #1920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mlguerrero12
requested review from
fedepaol,
rata,
russellb,
gclawes and
oribon
as code owners
fedepaol
reviewed
May 3, 2023
| // Verify that ip and address pool annotations are compatible. | ||
| if desiredPool != "" && c.ips.Pool(key) != desiredPool { | ||
| c.ips.Unassign(key) | ||
| return nil, fmt.Errorf("requested loadBalancer IP(s) %q is not compatible with requested address pool %s", desiredLbIPs, desiredPool) |
There was a problem hiding this comment.
we might want to return a custom error we handle outside here. Specifically, when this happens it makes sense to clear the service. We might get here with a user adding the annotation after the lb ip. In that case we need to wipe.
There was a problem hiding this comment.
the service is cleared here. your example is handled in line 107
There was a problem hiding this comment.
Makes sense.
I thought assign would set the ips on the svc.
mlguerrero12
force-pushed
the
compatibleannotations
branch
from
41269f0
to
a0a0aa1
Compare
When a service requests a specific LB IP(s) and address pool via annotations, these must be compatible. Fixes metallb#1916 Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12
force-pushed
the
compatibleannotations
branch
from
a0a0aa1
to
fa3184f
Compare
|
lgtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a service requests a specific LB IP(s) and address pool via annotations, these must be compatible.
Fixes #1916