Silence spurious clustermesh-related warnings #35867
Merged
+36
−3
Conversation
This file contains hidden or 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
Don't emit a spurious warning in case remote cluster configuration retrieval gets aborted due to the parent context being canceled (e.g., due to reconnecting to etcd), as already logged elsewhere, and potentially misleading to the users. Reported-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Currently, a warning log gets always emitted in case the list operation to start a watcher fails, even though it will automatically retried. Given that we are starting to flag warning logs in CI, and this can briefly occur in the clustermesh context during the initialization phase (especially when the authorization mode is configured to cluster, until permissions are granted), let's lower the severity for the first few occurrences, and emit a warning only if the situation does not improve. Reported-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Explicitly configure the clustermesh-sync-timeout parameter to a higher value to prevent emitting spurious warning logs during the initial connection phase when the authorization mode is configured to cluster. Indeed, Cilium agents get restarted at that point (due to the configuration of host aliases), while the clustermesh-apiserver does not (assuming that KVStoreMesh is disabled), potentially requiring up to one minute to react to the configmap change and create the associated user in etcd. Still, the warning in this case would have been benign, because no cross-cluster connections could be present, as we are meshing the two clusters for the first time right now. Reported-by: Paul Chaignon <paul.chaignon@gmail.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
|
/test |
aanm
approved these changes
Nov 8, 2024
christarazi
approved these changes
Nov 8, 2024
marseel
approved these changes
Nov 12, 2024
pchaigno
reviewed
Nov 20, 2024
| @@ -158,7 +158,12 @@ func (rc *remoteCluster) restartRemoteConnection() { | |||
| if backend != nil { | |||
| backend.Close() | |||
| } | |||
| rc.logger.WithError(err).Warning("Unable to establish etcd connection to remote cluster") | |||
There was a problem hiding this comment.
@giorio94 I see you did not backport this change to v1.16. Was that intentional? I'm seeing it on downgrades to v1.16: https://github.com/cilium/cilium/actions/runs/11935307422/job/33266363458. Should I just allowlist it until v1.17 is out?
There was a problem hiding this comment.
Thanks for noticing. It seems I mistakenly linked a different backport to this one, and marked this one as completed (rather than the other one). Marking again for backport, so that it gets included in the next round.
There was a problem hiding this comment.
Aah, that's why the commits looked so different 😆
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.
Please refer to the individual commits for additional details.
Fixes: #35865
Marked for backport to v1.16 to prevent issues on downgrade when warning logs start being flagged in CI.