Use OpenShift Route in Alertmanager tests #3889
Conversation
kubevirt-bot
added
release-note-none
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The test now hard-depends on the
alertmanager-mainRoute inopenshift-monitoring; consider keeping a fallback (e.g., direct service access or the previous port-forward) or at least failing with a more descriptive error when the Route is missing or not yet created to avoid flakes in partially configured clusters. - Instead of hardcoding the Route name and namespace inside
getAlertmanagerRouteHost, consider pulling these values from shared constants or config used elsewhere in the observability tests to reduce duplication and make future changes easier. - Before using
route.Status.Ingress[0].Host, you may want to verify the route is admitted (e.g., checkroute.Status.Ingress[0].Conditions) to avoid tests failing on routes that exist but are not yet ready.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test now hard-depends on the `alertmanager-main` Route in `openshift-monitoring`; consider keeping a fallback (e.g., direct service access or the previous port-forward) or at least failing with a more descriptive error when the Route is missing or not yet created to avoid flakes in partially configured clusters.
- Instead of hardcoding the Route name and namespace inside `getAlertmanagerRouteHost`, consider pulling these values from shared constants or config used elsewhere in the observability tests to reduce duplication and make future changes easier.
- Before using `route.Status.Ingress[0].Host`, you may want to verify the route is admitted (e.g., check `route.Status.Ingress[0].Conditions`) to avoid tests failing on routes that exist but are not yet ready.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: machadovilaca <[email protected]>
machadovilaca
force-pushed
the
use-alertmanager-route-instead
branch
from
01fb51a to
0a54a42
Compare
|
Pull Request Test Coverage Report for Build 19640841063Details
💛 - Coveralls |
|
hco-e2e-operator-sdk-aws, hco-e2e-operator-sdk-gcp, hco-e2e-operator-sdk-gcp, hco-e2e-operator-sdk-gcp lanes succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-azure, ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure In response to this:
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-sigs/prow repository. |
|
hco-e2e-upgrade-operator-sdk-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-aws, ci/prow/hco-e2e-upgrade-operator-sdk-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws In response to this:
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-sigs/prow repository. |
|
@machadovilaca: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nunnatsa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
|
hco-e2e-upgrade-prev-operator-sdk-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws In response to this:
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-sigs/prow repository. |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure In response to this:
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-sigs/prow repository. |
|
/cherry-pick release-1.16 |
|
@nunnatsa: new pull request created: #3894 In response to this:
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-sigs/prow repository. |
|
/cherry-pick release-1.15 release-1.14 |
|
@machadovilaca: new pull request created: #3897 In response to this:
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-sigs/prow repository. |
|
/cherry-pick release-1.14 |
|
@machadovilaca: new pull request created: #3898 In response to this:
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-sigs/prow repository. |
5 participants
What this PR does / why we need it:
Reviewer Checklist
Jira Ticket:
Release note: