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

feat: validate stable metrics via static analysis, which adapts codes from k/k stable metrics framework #2294

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: validate stable metrics via static analysis, which adapts codes from k/k stable metrics framework #2294

wants to merge 5 commits into from

Conversation

CatherineF-dev
Copy link
Contributor

@CatherineF-dev CatherineF-dev commented Jan 2, 2024 •
edited

What this PR does / why we need it: https://github.com/kubernetes/kube-state-metrics/blob/cee19c38d054a0dac4891d98c43d85949b5498b9/docs/design/validate-stable-metrics.md

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality) N/A

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1833

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 2, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: CatherineF-dev
Once this PR has been reviewed and has the lgtm label, please assign dgrisonnet for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 2, 2024
@CatherineF-dev CatherineF-dev changed the title Add CI job on validating stable metrics feat: Add CI job on validating stable metrics Jan 2, 2024
@CatherineF-dev CatherineF-dev mentioned this pull request Jan 2, 2024
Closed
@CatherineF-dev CatherineF-dev changed the title feat: Add CI job on validating stable metrics feat: validate stable metrics via static analysis, which adapts codes from k/k stable metrics framework Jan 2, 2024
@CatherineF-dev CatherineF-dev force-pushed the stable-v2 branch 2 times, most recently from 7958e99 to a9fd176 Compare January 2, 2024 19:18
CatherineF-dev
CatherineF-dev commented Jan 2, 2024
View reviewed changes
tests/stablemetrics/testdata/test-stable-metrics-list.yaml Outdated
type: Counter
stabilityLevel: STABLE
labels:
- container2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use container2 to let stability test fail. So it proves this CI job is working

@CatherineF-dev
Copy link
Contributor Author

cc @mrueg, I added static analysis example code in this PR.

@CatherineF-dev CatherineF-dev force-pushed the stable-v2 branch 4 times, most recently from 60e951f to b3fa244 Compare January 3, 2024 14:21
@dashpole
Copy link

/assign @rexagod
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 11, 2024
@CatherineF-dev
Copy link
Contributor Author

Ping~

@rexagod
Copy link
Member

rexagod commented Jan 21, 2024

Ah! I missed this, thank you for all the great work here! I'll review this ASAP! Sorry for the delay here! :/

@CatherineF-dev
Copy link
Contributor Author

Ping~

1 similar comment
@CatherineF-dev
Copy link
Contributor Author

Ping~

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2024
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgrisonnet dgrisonnet Awaiting requested review from dgrisonnet

@mrueg mrueg Awaiting requested review from mrueg

Assignees

@rexagod rexagod

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Presubmit checks for stable metrics
4 participants
@CatherineF-dev @k8s-ci-robot @dashpole @rexagod