OTA-1817 Adding unit tests for the extend recommended alerts#2213
Open
JianLi-RH wants to merge 1 commit intoopenshift:mainfrom
Open
OTA-1817 Adding unit tests for the extend recommended alerts#2213JianLi-RH wants to merge 1 commit intoopenshift:mainfrom
JianLi-RH wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded multiple static example files (JSON alerts, ClusterVersion YAML, and textual outputs) for both standard and critical upgrade-alert scenarios, and a small test guard in TestExamples to skip the "specific version" variant when no version mapping exists. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JianLi-RH The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output (1)
21-21: Verify whitespace consistency in the runbook annotation.This line has extra leading whitespace (3 spaces) before
<alert does not have a runbook_url annotation>, while the same annotation on lines 8 and 13 has no such leading whitespace.If the actual output includes this extra spacing, the test fixture is correct. However, if this is unintentional, consider normalizing the whitespace for consistency across alerts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output` at line 21, The test fixture contains an inconsistent leading whitespace for the annotation "<alert does not have a runbook_url annotation>" (it has three leading spaces on the shown line while identical annotations earlier do not); locate that annotation string in the example content and remove the extra leading spaces so all occurrences are normalized to the same no-leading-space format to ensure consistency across alerts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cli/admin/upgrade/recommend/examples_test.go`:
- Around line 81-85: The test loop incorrectly assumes variant.versions is
non-nil and indexes it unguarded, causing nil-map variants to always hit the
else branch and skip tests; fix by first checking whether variant.versions !=
nil before reading variant.versions[cv], and only apply the continue/setting of
variant.outputSuffix when the map exists: if variant.versions != nil { if
version = variant.versions[cv]; version != "" { variant.outputSuffix =
fmt.Sprintf(variant.outputSuffixPattern, version) } else { continue } } (leave
behavior unchanged for variants with a nil versions map).
---
Nitpick comments:
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output`:
- Line 21: The test fixture contains an inconsistent leading whitespace for the
annotation "<alert does not have a runbook_url annotation>" (it has three
leading spaces on the shown line while identical annotations earlier do not);
locate that annotation string in the example content and remove the extra
leading spaces so all occurrences are normalized to the same no-leading-space
format to ensure consistency across alerts.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples_test.go
Author
|
/uncc @ardaguclu @atiratree |
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
0c36357 to
261f317
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json (1)
29-33: Consider sanitizing environment-specific identifiers in fixtures.Line 29 and Line 51 include concrete cluster/node identifiers. Replacing them with clearly synthetic placeholders would make snapshots less brittle and safer to share broadly.
Also applies to: 51-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json` around lines 29 - 33, The fixture contains environment-specific identifiers (e.g., values for the JSON keys "instance" and "pod" in pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json) that should be sanitized; replace concrete cluster/node IDs like "10.130.0.31:8443" and "insights-operator-b8b5f97fc-kl8rr" (and the similar values around lines 51-58) with clear synthetic placeholders such as "INSTANCE_PLACEHOLDER" and "POD_PLACEHOLDER" (or similarly named tokens) so snapshots are stable and shareable, and ensure any other environment-specific fields (namespace, job, info_link query params containing IDs) are likewise replaced or parameterized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output`:
- Around line 18-21: The wrapped "Message:" block has inconsistent indentation
(an extra leading space on the third wrapped line); edit the golden output so
that the third line of the wrapped message matches the indentation of the
previous wrapped lines (remove the extra leading space before "it might have
issues..." in the Message block) to normalize formatting and ensure all wrapped
lines under "Message:" align consistently.
---
Nitpick comments:
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json`:
- Around line 29-33: The fixture contains environment-specific identifiers
(e.g., values for the JSON keys "instance" and "pod" in
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json)
that should be sanitized; replace concrete cluster/node IDs like
"10.130.0.31:8443" and "insights-operator-b8b5f97fc-kl8rr" (and the similar
values around lines 51-58) with clear synthetic placeholders such as
"INSTANCE_PLACEHOLDER" and "POD_PLACEHOLDER" (or similarly named tokens) so
snapshots are stable and shareable, and ensure any other environment-specific
fields (namespace, job, info_link query params containing IDs) are likewise
replaced or parameterized.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.output
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/cli/admin/upgrade/recommend/examples_test.go
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yaml
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.output
Comment on lines
+18
to
+21
| Message: warning alert TestAlert firing, suggesting issues worth investigating before updating the cluster. machine ci-ln-plnvytb-76ef8-42h8v-worker-us-east-1a-84pg2 is in phase: . The alert description is: The machine has been without a Running or Deleting phase for more than 60 minutes. | ||
| The machine may not have been provisioned properly from the infrastructure provider, or | ||
| it might have issues with CertificateSigningRequests being approved. | ||
| <alert does not have a runbook_url annotation> |
There was a problem hiding this comment.
Normalize wrapped-message indentation in the golden output.
Line 21 has an extra leading space versus Line 19 and Line 20, making this block inconsistently formatted.
Suggested fix
- <alert does not have a runbook_url annotation>
+ <alert does not have a runbook_url annotation>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Message: warning alert TestAlert firing, suggesting issues worth investigating before updating the cluster. machine ci-ln-plnvytb-76ef8-42h8v-worker-us-east-1a-84pg2 is in phase: . The alert description is: The machine has been without a Running or Deleting phase for more than 60 minutes. | |
| The machine may not have been provisioned properly from the infrastructure provider, or | |
| it might have issues with CertificateSigningRequests being approved. | |
| <alert does not have a runbook_url annotation> | |
| Message: warning alert TestAlert firing, suggesting issues worth investigating before updating the cluster. machine ci-ln-plnvytb-76ef8-42h8v-worker-us-east-1a-84pg2 is in phase: . The alert description is: The machine has been without a Running or Deleting phase for more than 60 minutes. | |
| The machine may not have been provisioned properly from the infrastructure provider, or | |
| it might have issues with CertificateSigningRequests being approved. | |
| <alert does not have a runbook_url annotation> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output`
around lines 18 - 21, The wrapped "Message:" block has inconsistent indentation
(an extra leading space on the third wrapped line); edit the golden output so
that the third line of the wrapped message matches the indentation of the
previous wrapped lines (remove the extra leading space before "it might have
issues..." in the Message block) to normalize formatting and ensure all wrapped
lines under "Message:" align consistently.
Author
|
@wking Could you please help take a look at below tests? Tested it locally: Make test: |
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
261f317 to
626e1c5
Compare
Author
|
/cc @wking |
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
626e1c5 to
f6277e1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/cli/admin/upgrade/recommend/examples_test.go (1)
81-87: Consider checkingvariant.versions != nilinstead of string comparison.The current fix correctly addresses the bug, but comparing against the literal string
"specific version"is fragile—if the variant name changes, this logic silently breaks. A more robust approach is to check whether the variant uses a versions map at all.♻️ Suggested refactor for robustness
var version string - if version = variant.versions[cv]; version != "" { - variant.outputSuffix = fmt.Sprintf(variant.outputSuffixPattern, version) - } else { - if variant.name == "specific version" { + if variant.versions != nil { + if version = variant.versions[cv]; version == "" { continue } + variant.outputSuffix = fmt.Sprintf(variant.outputSuffixPattern, version) }This approach ties the skip logic directly to the presence of a versions map, making the code self-documenting and resilient to variant name changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/recommend/examples_test.go` around lines 81 - 87, The branch that currently checks variant.name == "specific version" should instead test whether the variant has a versions map; change the logic around variant.versions and the assignment to variant.outputSuffix so that you first check if variant.versions != nil (or len(variant.versions) == 0) and if it is nil/empty then continue, otherwise look up version := variant.versions[cv] and, if non-empty, set variant.outputSuffix = fmt.Sprintf(variant.outputSuffixPattern, version); this removes the fragile string comparison and ties behavior to the presence of the versions map on the variant struct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/cli/admin/upgrade/recommend/examples_test.go`:
- Around line 81-87: The branch that currently checks variant.name == "specific
version" should instead test whether the variant has a versions map; change the
logic around variant.versions and the assignment to variant.outputSuffix so that
you first check if variant.versions != nil (or len(variant.versions) == 0) and
if it is nil/empty then continue, otherwise look up version :=
variant.versions[cv] and, if non-empty, set variant.outputSuffix =
fmt.Sprintf(variant.outputSuffixPattern, version); this removes the fragile
string comparison and ties behavior to the presence of the versions map on the
variant struct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5b07e20-35d1-49c7-be70-ca5143a1d700
📒 Files selected for processing (9)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.json
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-output
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output
Author
|
/test security |
JianLi-RH
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
JianLi-RH
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
f6277e1 to
edb8d85
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json (1)
25-39: Use a scenario-specific name for the synthetic custom alert.
TestAlertworks, but a name likeCustomPrecheckAlertwould make the fixture and any golden-output diffs more self-explanatory when this case fails.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json` around lines 25 - 39, Replace the generic synthetic alert name by changing the JSON key "alertname" value currently set to "TestAlert" to a scenario-specific name like "CustomPrecheckAlert" so the fixture and golden-output diffs are self-explanatory; locate the object containing the "alertname" key (alongside keys "openShiftUpdatePrecheck", "reason", "severity") and update its value accordingly.pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yaml (1)
4-10: Trim volatile runtime metadata from this example fixture.
creationTimestamp,resourceVersion,uid,clusterID, the transition timestamps, andversionHashall look incidental to the recommend-alert behavior under test. Keeping only the fields the parser actually reads will make future fixture refreshes much smaller and easier to review.Also applies to: 57-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yaml` around lines 4 - 10, Remove volatile runtime metadata from the example YAML: delete top-level keys creationTimestamp, resourceVersion, uid, and any incidental spec.clusterID, transition timestamps, and versionHash so the fixture only contains the fields the recommend-alert parser actually reads (keep the minimal spec and version fields used by the test). Locate these keys in the top-level metadata/spec blocks (creationTimestamp, resourceVersion, uid, clusterID, versionHash, and any "transition" timestamp fields) and strip them out across the example (also apply the same pruning to the other similar examples mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json`:
- Around line 25-39: Replace the generic synthetic alert name by changing the
JSON key "alertname" value currently set to "TestAlert" to a scenario-specific
name like "CustomPrecheckAlert" so the fixture and golden-output diffs are
self-explanatory; locate the object containing the "alertname" key (alongside
keys "openShiftUpdatePrecheck", "reason", "severity") and update its value
accordingly.
In
`@pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yaml`:
- Around line 4-10: Remove volatile runtime metadata from the example YAML:
delete top-level keys creationTimestamp, resourceVersion, uid, and any
incidental spec.clusterID, transition timestamps, and versionHash so the fixture
only contains the fields the recommend-alert parser actually reads (keep the
minimal spec and version fields used by the test). Locate these keys in the
top-level metadata/spec blocks (creationTimestamp, resourceVersion, uid,
clusterID, versionHash, and any "transition" timestamp fields) and strip them
out across the example (also apply the same pruning to the other similar
examples mentioned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 163e81a4-b78c-4ea9-967b-ca9bc3c15485
📒 Files selected for processing (9)
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.jsonpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yamlpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.outputpkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-outputpkg/cli/admin/upgrade/recommend/examples_test.go
✅ Files skipped from review due to trivial changes (2)
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.show-outdated-releases-output
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.output
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/cli/admin/upgrade/recommend/examples_test.go
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert.show-outdated-releases-output
- pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert.output
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
edb8d85 to
447a13b
Compare
JianLi-RH
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert:
Add Additional Alerts to the OCP Update Precheck
Component teams want to raise special alerts to the `oc adm upgrade recommend ...`
command for consideration by cluster-admins considering a future cluster update.
And we also give `label-maintainers` the ability to add or remove alerts from
the recommend-precheck by a new label `openShiftUpdatePrecheck`[1]
We can follow below steps to generate these test files:
Launch a cluster by cluster-bot:
$ launch 4.22 aws
Cluster Bot gives the default (X.509 certificate based) kubeconfig
returned by the installer, but Thanos alert-retrieval requires a
token. So I created one using the cluster-version operator's
namespace:
$ TOKEN="$(oc -n openshift-cluster-version create token default)"
And Cluster Bot clusters use self-signed certificate, so I collected
all possible Kube API server and ingress TLS certificates in a file:
$ oc -n openshift-kube-apiserver get -o json secrets | jq -r '.items[] | select(.type == "kubernetes.io/tls").data["tls.crt"] | @base64d' >ca.crt
$ oc -n openshift-ingress get -o json secrets | jq -r '.items[] | select(.type == "kubernetes.io/tls").data["tls.crt"] | @base64d' >>ca.crt
Prepare test data for warning alerts:
$ cat <<EOF | oc -n openshift-cluster-version create -f -
apiVersion: monitoring.coreos.com/v1
kind: PrometheusRule
metadata:
name: warning-testing
namespace: openshift-cluster-version
annotations:
kubernetes.io/description: Alerting rules for testing.
spec:
groups:
- name: VirtWarningPrecheck
rules:
- alert: VirtHandlerDaemonSetRolloutFailing
annotations:
summary: Test summary.
description: Test description.
expr: |
up{job="cluster-version-operator"} == 1
labels:
severity: warning
namespace: openshift-cluster-version
reason: UpdatingPrometheusFailed
openShiftUpdatePrecheck: "true"
- name: VMCannotBeEvictedWarningNormal
rules:
- alert: VMCannotBeEvicted
annotations:
summary: Test summary.
description: Test description.
expr: |
up{job="cluster-version-operator"} == 1
labels:
severity: warning
namespace: openshift-cluster-version
reason: UpdatingPrometheusFailed
openShiftUpdatePrecheck: "false"
- name: CustomWarningPrecheck
rules:
- alert: TestAlert
annotations:
summary: Test summary.
description: Test description.
expr: |
up{job="cluster-version-operator"} == 1
labels:
severity: warning
namespace: openshift-cluster-version
reason: UpdatingPrometheusFailed
openShiftUpdatePrecheck: "true"
EOF
retrieved ClusterVersion YAML with:
$ oc get -o yaml clusterversion version >pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-cv.yaml
retrieve the cluster's alerts:
$ OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts --token "${TOKEN}" --certificate-authority ca.crt | jq . > pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-alert-alerts.json
then I generated the output fixtures with:
$ UPDATE=yes go test -v ./pkg/cli/admin/upgrade/recommend/...
delete above alerts:
$ oc delete PrometheusRule/warning-testing -n openshift-cluster-version
Wait some minutes, then repeat above step to generate new test data for critical alearts:
Create critical alerts:
cat <<EOF | oc -n openshift-cluster-version create -f -
apiVersion: monitoring.coreos.com/v1
kind: PrometheusRule
metadata:
name: critical-testing
namespace: openshift-cluster-version
annotations:
kubernetes.io/description: Alerting rules for testing.
spec:
groups:
- name: VirtCriticalPrecheck
rules:
- alert: VirtHandlerDaemonSetRolloutFailing
annotations:
summary: Test summary.
description: Test description.
expr: |
up{job="cluster-version-operator"} == 1
labels:
severity: critical
namespace: openshift-cluster-version
reason: UpdatingPrometheusFailed
openShiftUpdatePrecheck: "true"
- name: VMCannotBeEvictedCriticalPrecheck
rules:
- alert: VMCannotBeEvicted
for: 1h
annotations:
summary: Test summary.
description: Test description.
expr: |
up{job="cluster-version-operator"} == 1
labels:
severity: critical
namespace: openshift-cluster-version
reason: UpdatingPrometheusFailed
openShiftUpdatePrecheck: "true"
EOF
Generate test data:
$ oc get -o yaml clusterversion version >pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-cv.yaml
$ OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts --token "${TOKEN}" --certificate-authority ca.crt | jq . >pkg/cli/admin/upgrade/recommend/examples/4.22.0-extend-recommended-critical-alert-alerts.json
$ UPDATE=yes go test -v ./pkg/cli/admin/upgrade/recommend/...
Then delate critical alerts:
$ oc delete PrometheusRule/critical-testing -n openshift-cluster-version
[1]: https://issues.redhat.com/browse/OTA-1864
JianLi-RH
force-pushed
the
test_for_extend-update-recommend-precheck-alerts
branch
from
447a13b to
030a4e7
Compare
Author
|
/test e2e-aws-ovn-serial-1of2 |
Author
|
/test e2e-aws-ovn-serial-1of2 |
Author
|
@wking could you please help review it? thanks |
Author
|
/test e2e-aws-ovn-serial-1of2 |
Contributor
|
@JianLi-RH: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the follow up to #2210
This is the JIRA card: https://issues.redhat.com/browse/OTA-1817
In this PR I added two test file:
This file covers:
This file covers:
These tests do not need
specific version, so I updatedexamples_test.gothen these cases will be skipped when the test files do not exist in variant versions.Summary by CodeRabbit
Tests
Chores