Skip to content

allow loki target to be down#26771

Merged
deads2k merged 1 commit intoopenshift:masterfrom
deads2k:ignore-promtaildown
Jan 20, 2022
Merged

allow loki target to be down#26771
deads2k merged 1 commit intoopenshift:masterfrom
deads2k:ignore-promtaildown

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Jan 20, 2022

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2022
allowedFiringAlerts := helper.MetricConditions{
{
Selector: map[string]string{"alertname": "TargetDown", "namespace": "openshift-e2e-loki"},
Text: "Loki is nice to have, but we can allow it to be down briefly",
Copy link
Member

Choose a reason for hiding this comment

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

TargetDown is already for: 15m, so not clear to me that we can claim "briefly" when allowing it to fire. Dropping "briefly" and just saying ...but we can allow it to be down. would be internally consistent.

@deads2k deads2k force-pushed the ignore-promtaildown branch from 1f2c3fa to e7e4c05 Compare January 20, 2022 20:18
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

Sounds like pending alerts are non-fatal today, so I don't think we need the allowedPendingAlerts exception, but doesn't matter too much to me, so:

/lgtm
/hold

Pull the hold if you want to keep the allowedPendingAlerts around in the window before we get a real fix and revert this, but looking at the run, there are a whole bunch of TargetDown pending alerts today during various phases of the updates, and we don't have explicit exceptions for those (because we're only concerned about alerts that are still pending when the update completes).

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2022
@deads2k deads2k force-pushed the ignore-promtaildown branch from e7e4c05 to 9480da6 Compare January 20, 2022 20:36
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, wking

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

The pull request process is described here

Details 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

@deads2k deads2k added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jan 20, 2022
@deads2k
Copy link
Contributor Author

deads2k commented Jan 20, 2022

this built. we want to make the next payload. merging.

@deads2k deads2k merged commit 81dc64b into openshift:master Jan 20, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2022

@deads2k: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants