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

v1.12 Backports 2023-12-05 #29639

Merged
merged 4 commits into from
Dec 6, 2023
Merged

v1.12 Backports 2023-12-05 #29639

merged 4 commits into from
Dec 6, 2023

Conversation

nbusseneau
Copy link
Member

@nbusseneau nbusseneau commented Dec 5, 2023

Once this PR is merged, a GitHub action will update the labels of these PRs:

 29182 29566

[ upstream commit da354d9 ]

CountUniqueIPsecKeys function fixed to count non-Aead keys and
catch unsupported XfrmStateAlgo combinations.

Fixes #29181.

Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau nbusseneau added kind/backports This PR provides functionality previously merged into master. backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. labels Dec 5, 2023
@nbusseneau
Copy link
Member Author

@viktor-kurchenko @christarazi Some minor conflicts were hit, please check commit notes for details.

viktor-kurchenko and others added 3 commits December 5, 2023 17:57
…lement error handling.

[ upstream commit 6f227fb ]

[ backporter's notes: conflicts due to `cilium/cmd` having been renamed
  to `cilium-dbg/cmd`. ]

Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 4787f8e ]

[ backporter's notes: had to resolve rename conflicts. ]

Similar to how useful log msgs are when endpoints created and deleted,
this log is useful for understanding when nodes are added and deleted in
production clusters.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
[ upstream commit 7c7b723 ]

Given the order of operations in prober.OnIdle, it is possible for the
health probe to have a stale references to a deleted nodes. When that
occurs, node connectivity metrics which were previously deleted [1]
would be brought back, causing confusion. If users defined alerts for
node connectivity health checks metrics (see example below), then this
would erroneously trigger because the old nodes would appear in the
metric labels as a failing health check.

Example given deletion of "kind-worker2" node:

```
cilium_node_connectivity_status                          source_cluster="kind-kind" source_node_name="kind-worker" target_cluster="kind-kind" target_node_name="kind-control-plane" target_nod
e_type="remote_intra_cluster" type="endpoint"                                                                        1.000000
cilium_node_connectivity_status                          source_cluster="kind-kind" source_node_name="kind-worker" target_cluster="kind-kind" target_node_name="kind-control-plane" target_nod
e_type="remote_intra_cluster" type="node"                                                                            1.000000
cilium_node_connectivity_status                          source_cluster="kind-kind" source_node_name="kind-worker" target_cluster="kind-kind" target_node_name="kind-worker" target_node_type=
"local_node" type="endpoint"                                                                                         1.000000
cilium_node_connectivity_status                          source_cluster="kind-kind" source_node_name="kind-worker" target_cluster="kind-kind" target_node_name="kind-worker" target_node_type=
"local_node" type="node"                                                                                             1.000000

cilium_node_connectivity_status                          source_cluster="kind-kind" source_node_name="kind-worker" target_cluster="kind-kind" target_node_name="kind-worker2" target_node_type
="remote_intra_cluster" type="endpoint"                                                                              0.000000
```

Fixes: d9e1ff8 ("cilium-health: Remove unnecessary goroutine")

[1]: e9f97cd ("Ensures prometheus metrics associated with a deleted
node are no longer reported.")

Signed-off-by: Chris Tarazi <chris@isovalent.com>
Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau
Copy link
Member Author

nbusseneau commented Dec 5, 2023

/test-backport-1.12

@nbusseneau nbusseneau marked this pull request as ready for review December 5, 2023 17:25
@nbusseneau nbusseneau requested review from a team as code owners December 5, 2023 17:25
Copy link
Contributor

@chancez chancez left a comment

Choose a reason for hiding this comment

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

Metrics changes LGTM.

Copy link
Contributor

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

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

My commit looks good. Thanks!

@nbusseneau
Copy link
Member Author

nbusseneau commented Dec 6, 2023

/test-1.17-4.9

@nbusseneau
Copy link
Member Author

nbusseneau commented Dec 6, 2023

/test-1.23-4.19

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 6, 2023
@nbusseneau nbusseneau merged commit 46d5b5a into v1.12 Dec 6, 2023
109 checks passed
@nbusseneau nbusseneau deleted the pr/v1.12-backport-2023-12-05 branch December 6, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.12 This PR represents a backport for Cilium 1.12.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

5 participants