[release-1.28] [occm] Make sure we don't mask LB tests failures and fix what was failing #2537
Conversation
k8s-ci-robot
added
do-not-merge/hold
k8s-ci-robot
added
the
size/L
|
This one should wait for #2536 to make it as clean as possible. |
…ling (kubernetes#2360) * Compare proper LB name for shared LBs With shared LBs we distinguish the elements by tagging them with the proper name of the LB that would be created for a Service if it wasn't created as shared. This commit fixes that comparison for listener deletion as code was always comparing the name of the primary LB. * Fix shared LBs tests PR kubernetes#2190 prohibited sharing an LB that is internal for security reasons. This commit fixes the shared LBs tests to not create internal LBs. * Make sure we don't mask LB tests failures In `test-lb-service.sh` we do `trap "delete_resources" EXIT` to make sure we cleanup resources on a test failure. In there, we only fetched the `$?` after making a check for `${AUTO_CLEAN_UP}`, which itself alters the code to 0, so function always returns success. This means tests can never really fail. This commit fixes it by making sure `$ERROR_CODE` is fetched at the very beginning of the cleanup function.
dulek
force-pushed
the
cherry-pick-2360-to-release-1.28
branch
from
4457dd8 to
d7ffecb
Compare
|
/hold cancel |
k8s-ci-robot
added
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dulek 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 |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
lgtm
|
/retest This is very weird. |
|
You may try with a newer version of golangci-lint? There was a new release just an hour ago. |
|
But locally all works. :/ |
|
It's clearly not caused by your PR as we had a bunch of the same failure yesterday, both in #2542 shows that it can work, however I'm surprised that it suddenly broke as the previous version was working fine until now. Perhaps there's a new dep that it doesn't like? Let's see one last time if it fixed by itself. |
|
The test image is different between a failing job and a working one: I suspect different versions of go that break golangci-lint. |
|
Bingo! And golangci/golangci-lint#4272 This explains why we need the newest version of golangci-lint. |
|
/retest |
This is a cherry-pick of #2360
/hold