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

test: Add Error Log Exceptions #18117

Conversation

nathanjsweet
Copy link
Member

@nathanjsweet nathanjsweet commented Dec 3, 2021

Occasionally the cilium-operator will run into a transient issue
where it cannot get/update/release the leaselock with K8s that
it uses to adjudicate its leader election. This error message
is part and parcel of this failure and can be ignored.

cf. #16402

Signed-off-by: Nate Sweet nathanjsweet@pm.me

Fixes: #17660.

@nathanjsweet nathanjsweet added area/CI Continuous Integration testing issue or flake release-note/ci This PR makes changes to the CI. labels Dec 3, 2021
@nathanjsweet nathanjsweet requested a review from a team as a code owner December 3, 2021 21:45
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.0 Dec 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Dec 3, 2021
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

My take on this is that this is an error message which resolves itself, and causing CI to fail on this error message causes issues by itself without providing a useful way for developers to understand and attempt to resolve the issue. The log message is very specific to the leader election code in the operator (+ underlying k8s library). So, if we think this is benign then this is the right solution. If we think this error is not benign, then we should probably develop dedicated operator HA tests in order to catch any potential issues with those codepaths. In both cases it doesn't make sense to error out randomly in different tests on arbitrary contributors' PRs.

But I welcome input from the other reviewers as well before merging this.

@joestringer joestringer added this to Needs backport from master in 1.11.1 Dec 5, 2021
@joestringer joestringer removed this from Needs backport from master in 1.11.0 Dec 5, 2021
test/helpers/cilium.go Outdated Show resolved Hide resolved
test/helpers/cons.go Show resolved Hide resolved
Occasionally the cilium-operator will run into a transient issue
where it cannot get/update/release the leaselock with K8s that
it uses to adjudicate its leader election. This error message
is part and parcel of this failure and can be ignored.

cf. #16402

Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
@pchaigno pchaigno force-pushed the pr/nathanjsweet/add-klog-resource-lock-exception-to-error-list-test-execeptions branch from e9a25ec to 2874bf9 Compare December 9, 2021 13:00
@pchaigno
Copy link
Member

pchaigno commented Dec 9, 2021

I don't think we need to run the full end-to-end tests given this is a trivial change. All reviews are in, so marking ready to merge.

The CodeQL failures are for code not touched in this pull request.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 9, 2021
@nbusseneau nbusseneau merged commit 82d4422 into master Dec 9, 2021
@nbusseneau nbusseneau deleted the pr/nathanjsweet/add-klog-resource-lock-exception-to-error-list-test-execeptions branch December 9, 2021 16:00
@joestringer joestringer added this to Needs backport from master in 1.10.7 Dec 10, 2021
@joestringer joestringer removed this from Needs backport from master in 1.10.6 Dec 10, 2021
@tklauser tklauser added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Dec 16, 2021
@tklauser tklauser moved this from Needs backport from master to Backport pending to v1.10 in 1.10.7 Dec 20, 2021
@tklauser tklauser moved this from Needs backport from master to Backport done to v1.11 in 1.11.1 Dec 22, 2021
@joestringer joestringer moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.7 Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.10.7
Backport done to v1.10
1.11.1
Backport done to v1.11
Development

Successfully merging this pull request may close these issues.

CI: Found level=error: Failed to release lock: resource name may not be empty
6 participants