Skip to content

WIP Use single instances of Prometheus and Loki for all tests #5779

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
Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

WIP Use single instances of Prometheus and Loki for all tests #5779

wants to merge 12 commits into from

Conversation

@pmtk
Copy link
Member

@pmtk pmtk commented Nov 20, 2025

No description provided.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 20, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@pmtk
Copy link
Member Author

pmtk commented Nov 20, 2025

/test all

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pmtk

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 20, 2025
agullon
agullon reviewed Nov 21, 2025
View reviewed changes
Copy link
Contributor

@agullon agullon left a comment

Choose a reason for hiding this comment

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

The resourcedetection processor is a great idea, but I do have 2 concerns going forward with this approach:

  • It'll be more complicated to run scenarios in local env, because the Prometheus/Loki server must be started before running the test instead of within the scenario.
  • Prometheus and Loki servers are always started, but in some cases we don't need them because there are no scenarios using Prometheus or Loki, for example: test/scenarios/presubmits

test/suites/telemetry/telemetry.robot
Comment on lines 98 to +100
MicroShift Reports Metrics To Prometheus Server
[Documentation] Check the expected metrics are sent to the local server.
[Setup] Run Keywords
... Start Prometheus Server ${PROMETHEUS_PORT}
... AND
... Setup Telemetry Configuration ${ENABLE_TELEMETRY_TO_PROMETHUS} ${PULL_SECRET_METRICS}
[Setup] Setup Telemetry Configuration ${ENABLE_TELEMETRY_TO_PROMETHUS} ${PULL_SECRET_METRICS}

Should Find Metrics In Journal Log Success Metrics sent successfully

@{metrics_to_check}= Get List Prometheus Metrics To Check
FOR ${metric} IN @{metrics_to_check}
Check Prometheus Query ${PROMETHEUS_HOST} ${PROMETHEUS_PORT} ${metric}
Check Prometheus Query ${PROMETHEUS_HOST} ${PROMETHEUS_PORT} ${metric} add_hostname_filter=${False}
END

[Teardown] Run Keywords
... Remove Telemetry Configuration
... AND
... Stop Prometheus Server ${PROMETHEUS_PORT}
[Teardown] Remove Telemetry Configuration
Copy link
Contributor

@agullon agullon Nov 21, 2025

Choose a reason for hiding this comment

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

This MicroShift Reports Metrics To Prometheus Server Test Case must be executed always in the last place in this Test Suite. If not, the other 2 Prometheus Test Cases will fail because they'll find metrics in the Prometheus server.
This is the reason why, in the original implementation, the Prometheus server starts and stops for every Test Case.

Copy link
Member Author

@pmtk pmtk Nov 21, 2025

Choose a reason for hiding this comment

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

That would explain weird failure I've seen locally

@pmtk
Copy link
Member Author

pmtk commented Nov 27, 2025

/test all

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2025
@pmtk
Copy link
Member Author

pmtk commented Nov 27, 2025

/test all

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2025
@pmtk
Copy link
Member Author

pmtk commented Nov 27, 2025

/test all

1 similar comment
@pmtk
Copy link
Member Author

pmtk commented Nov 27, 2025

/test all

@pmtk pmtk force-pushed the single-prom-loki branch from 6a61de6 to 2786b58 Compare December 2, 2025 15:17
@pmtk
Copy link
Member Author

pmtk commented Dec 2, 2025

/test all

@pmtk pmtk force-pushed the single-prom-loki branch from 2786b58 to 96ba780 Compare December 2, 2025 17:04
@pmtk
Copy link
Member Author

pmtk commented Dec 2, 2025

/test all

@pmtk pmtk marked this pull request as ready for review December 3, 2025 09:07
@pmtk
Copy link
Member Author

pmtk commented Dec 3, 2025

/retest

@pmtk
Copy link
Member Author

pmtk commented Dec 4, 2025

/retst

@pmtk
Copy link
Member Author

pmtk commented Dec 4, 2025

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 4, 2025

@pmtk: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-tests-release-arm 96ba780 link true /test e2e-aws-tests-release-arm
ci/prow/e2e-aws-tests-periodic-arm 96ba780 link true /test e2e-aws-tests-periodic-arm
ci/prow/e2e-aws-tests-bootc-upstream-arm 96ba780 link true /test e2e-aws-tests-bootc-upstream-arm
ci/prow/e2e-aws-tests-bootc-periodic 96ba780 link true /test e2e-aws-tests-bootc-periodic
ci/prow/e2e-aws-tests-bootc-periodic-arm 96ba780 link true /test e2e-aws-tests-bootc-periodic-arm
ci/prow/e2e-aws-tests-periodic 96ba780 link true /test e2e-aws-tests-periodic

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.

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

Reviewers

@copejon copejon Awaiting requested review from copejon

@kasturinarra kasturinarra Awaiting requested review from kasturinarra

1 more reviewer

@agullon agullon agullon left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pmtk @agullon @openshift-merge-robot @pacevedom