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.

Sign up for GitHub

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

Jump to bottom

[v1.13] Author Backport of 28382 (Metrics associated with a deleted node should not be reported) #28932

Merged
merged 1 commit into from Nov 8, 2023
Merged

[v1.13] Author Backport of 28382 (Metrics associated with a deleted node should not be reported) #28932

merged 1 commit into from Nov 8, 2023

Conversation

derailed
Copy link
Contributor

@derailed derailed commented Nov 1, 2023 •
edited

Once this PR is merged, you can update the PR labels via:

$ for pr in 28382; do contrib/backporting/set-labels.py $pr done 1.13; done

@derailed derailed requested a review from a team as a code owner November 1, 2023 19:33
@derailed derailed added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Nov 1, 2023
@derailed
Copy link
Contributor Author

derailed commented Nov 1, 2023

/test-backport-1.13

@derailed
Copy link
Contributor Author

derailed commented Nov 2, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.13

Job 'Cilium-PR-K8s-1.24-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathConfig MonitorAggregation Checks that monitor aggregation restricts notifications

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.24-kernel-4.19/src/github.com/cilium/cilium/test/k8s/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-xc2q5 policy revision: cannot get the revision

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-4.19/223/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.24-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

@derailed
Copy link
Contributor Author

derailed commented Nov 3, 2023

/test-backport-1.13

@derailed derailed changed the title v1.13 backports 2023-10-31 [v1.13] Author Backport of 28382 (Metrics associated with a deleted node should not be reported) Nov 3, 2023
@tommyp1ckles
Copy link
Contributor

Think connectivity tests are failing due to image build being cancelled for some reason 😕 Error: The operation was canceled.

@tommyp1ckles
Copy link
Contributor

/test-backport-1.13

@derailed
Copy link
Contributor Author

derailed commented Nov 3, 2023

/test-backport-1.13

@christarazi
Copy link
Member

@derailed I think the commits here got a little messed up and probably just need squashing.

@derailed derailed force-pushed the pr/v1.13-backport-2023-10-31 branch 2 times, most recently from ea01a4d to 8348990 Compare November 7, 2023 14:48
@derailed
Copy link
Contributor Author

derailed commented Nov 7, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.13

Job 'Cilium-PR-K8s-1.21-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathConfig MonitorAggregation Checks that monitor aggregation restricts notifications

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.21-kernel-4.19/src/github.com/cilium/cilium/test/k8s/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-bwpp8 policy revision: cannot get the revision

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.21-kernel-4.19/228/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.21-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

@tommyp1ckles
Copy link
Contributor

tommyp1ckles commented Nov 8, 2023 •
edited by maintainer-s-little-helper bot

/test-backport-1.13

Job 'Cilium-PR-K8s-1.23-kernel-4.19' failed:

Click to show.

Test Name

K8sDatapathConfig MonitorAggregation Checks that monitor aggregation restricts notifications

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.23-kernel-4.19/src/github.com/cilium/cilium/test/k8s/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-tnc7g policy revision: cannot get the revision

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.23-kernel-4.19/408/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.23-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

@derailed
Ensures prometheus metrics associated with a deleted node are no long…
56def24 
…er reported.

[ upstream commit e9f97cd ]

When a node is deleted from a cluster, metrics associated with that node
are still being exported to prometheus. Short of restarting the agent,
we want to dynamically delete these metrics when a node is removed from the cluster.

This PR ensures node_connectivity_status and node_connectivity_latency
no longer report metrics for nodes that are no longer present on the
cluster.

[ Backporter's notes: Original PR was adapted! ]

The original PR depends (mainly!) on 2 other PRs that haven't been
backported and are fairly substential.
Given this, I've opted to adapt the original implementation to surface the
fix while minimizing impact with 2 updates:
1. pkg/metrics/interfaces did not introduce pkg/metrics/metric wrappers
  as of this release. Hence adapted deletableVec to use the current
implementation. (Referring to commit: 84ea383)
2. pkg/node/manager/manager was adapted to provide for metrics deletion when a
   node is deleted. Subsequent PR refactored the manager metrics structure which
   the original PR used. (Referring to commit: c49ef45)

Signed-off-by: Fernand Galiana <fernand.galiana@isovalent.com>
@derailed
Copy link
Contributor Author

derailed commented Nov 8, 2023

/test-backport-1.13

@christarazi christarazi merged commit acb72ff into cilium:v1.13 Nov 8, 2023
61 checks passed
@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 Nov 8, 2023
@nathanjsweet nathanjsweet mentioned this pull request Nov 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@squeed squeed squeed approved these changes

Assignees
No one assigned
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.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
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@derailed @tommyp1ckles @christarazi @squeed