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

No alerting for certificates in error state #190

Merged
merged 2 commits into from
Aug 31, 2023
Merged

No alerting for certificates in error state #190

merged 2 commits into from
Aug 31, 2023

Conversation

MartinWeindel
Copy link
Member

How to categorize this PR?

/area monitoring
/kind bug

What this PR does / why we need it:
If a cluster contains misconfigured certificates, the metrics cert_management_cert_object_expire reports a value of 0 and the alerting is triggered as the certificate seems to have expired since about 19600 days.

To exclude such false alerts, the expression has been adapted.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

No alerting for certificates in error state

@MartinWeindel MartinWeindel requested review from a team as code owners August 30, 2023 12:39
@gardener-robot gardener-robot added area/monitoring Monitoring (including availability monitoring and alerting) related kind/bug Bug labels Aug 30, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 30, 2023
@MartinWeindel
Copy link
Member Author

/invite @istvanballok @rickardsjp

@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Aug 30, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 30, 2023
ScheererJ
ScheererJ previously approved these changes Aug 30, 2023
View reviewed changes
Copy link
Member

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Aug 30, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 30, 2023
istvanballok
istvanballok reviewed Aug 30, 2023
View reviewed changes
Copy link
Member

@istvanballok istvanballok left a comment

Choose a reason for hiding this comment

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

/hold for a second

@gardener-robot gardener-robot added the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Aug 30, 2023
istvanballok
istvanballok suggested changes Aug 30, 2023
View reviewed changes
Copy link
Member

@istvanballok istvanballok left a comment

Choose a reason for hiding this comment

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

See minor caveat related to using and in PromQL.

charts/internal/shoot-cert-management-seed/templates/configmap-observability.yaml Outdated Show resolved Hide resolved
charts/internal/shoot-cert-management-seed/templates/configmap-observability.yaml Outdated Show resolved Hide resolved
@MartinWeindel @istvanballok
Update charts/internal/shoot-cert-management-seed/templates/configmap…
8c0cb64 
…-observability.yaml

Co-authored-by: Istvan Ballok <istvan.zoltan.ballok@sap.com>
@gardener-robot gardener-robot added needs/changes Needs (more) changes needs/review Needs review and removed reviewed/lgtm Has approval for merging labels Aug 30, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 30, 2023
istvanballok
istvanballok approved these changes Aug 30, 2023
View reviewed changes
Copy link
Member

@istvanballok istvanballok left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes labels Aug 30, 2023
@gardener-robot gardener-robot removed the needs/review Needs review label Aug 30, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 30, 2023
istvanballok
istvanballok reviewed Aug 31, 2023
View reviewed changes
Copy link
Member

@istvanballok istvanballok left a comment

Choose a reason for hiding this comment

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

/unhold

@gardener-robot gardener-robot removed the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Aug 31, 2023
timuthy
timuthy approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@MartinWeindel MartinWeindel merged commit ca9f275 into master Aug 31, 2023
1 check passed
@MartinWeindel MartinWeindel deleted the fix/no-alerting-for-failed-certs branch August 31, 2023 11:01
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@istvanballok istvanballok istvanballok approved these changes

@timuthy timuthy timuthy approved these changes

@rickardsjp rickardsjp Awaiting requested review from rickardsjp

@ScheererJ ScheererJ Awaiting requested review from ScheererJ

Assignees
No one assigned
Labels
area/monitoring Monitoring (including availability monitoring and alerting) related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@MartinWeindel @istvanballok @ScheererJ @timuthy @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot