Fix up "should not auto-assign egress IPs" cases #21122
Conversation
danwinship
added
kind/bug
openshift-ci-robot
added
size/M
|
/retest |
1 similar comment
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, dcbw 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 |
|
/refresh |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
12 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
The new unit test from #20971 was supposed to reject the egress IPs because there were multiple IPs, but in fact it was rejecting them because I made a typo and one of them wasn't actually a valid IP, and the "don't mix auto-assigned-HA with multiple-egress-IP HA" check wasn't actually working at all. So this fixes the test case and the check, and then for good measure also ensures we don't auto-assign egress IPs in two other "broken" cases. (The new "NetID: 49" test case is because I initially thought the problem might have been that we were handling the case of "good namespace goes bad" but not the case of "new namespace is bad from the start". That was a red herring but I left the extra test in anyway.)
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1633574 (which has been bumped from 3.11.0 to 3.11.z because this code is just there to prevent you from screwing up; as long as you don't screw up in the first place, everything works fine).