Skip to content

NO-JIRA: Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch"#2982

Open
machine424 wants to merge 1 commit into
openshift:mainfrom
redhat-chai-bot:revert-pr-2251-maximumStartupDurationSeconds
Open

NO-JIRA: Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch"#2982
machine424 wants to merge 1 commit into
openshift:mainfrom
redhat-chai-bot:revert-pr-2251-maximumStartupDurationSeconds

Conversation

@machine424

@machine424 machine424 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This reverts the changes from PR #2251.

With maximumStartupDurationSeconds=3600, prometheus-operator sets PeriodSeconds=60s (instead of the previous 15s), which significantly slows down Prometheus pod restarts, especially in e2e tests where restarts are numerous.

Additionally, UWM Prometheus still uses the container patch approach, so reverting makes the two consistent.


e2e-agnostic-operator runs, revert #2982 vs last 5 PRs merged to main

PR Title Merged Total vs Revert
#2981 OU-1389: config path fix Jul 08 82m55s -5m02s
#2979 OCPBUGS-93756: update prometheus Jul 02 82m11s -4m18s
#2972 OCPBUGS-92085: set Prom shards Jun 26 83m07s -5m14s
#2919 MON-4527: NodeExporter config Jun 25 78m42s -0m49s
#2971 NO-JIRA: sync component versions Jun 25 86m26s -8m33s
AVG BASELINE (5 runs) 82m40s
REVERT #2982 77m53s -4m47s (5.8%)

Revert is faster than every single baseline run.

Per-test breakdown

Test #2981 #2979 #2972 #2919 #2971 AVG REVERT Δ avg
TestAlertmanagerUWMSecrets 349s 201s 350s 211s 201s 262s 191s -71s
TestClusterMonitoringStatus 164s 163s 161s 151s 164s 161s 111s -50s
TestClusterMonitorAlertMgrCfg 84s 101s 99s 30s 98s 83s 31s -52s
TestAlertingRule 46s 38s 21s 51s 62s 44s 7s -37s
TestUserWorkloadPromOperCfg 149s 139s 141s 110s 140s 136s 108s -28s
TestAlertmanagerDisabling 160s 145s 136s 140s 155s 147s 129s -18s

Telemetry report

…ner patch"

This reverts the changes from PR openshift#2251.

With maximumStartupDurationSeconds=3600, prometheus-operator sets
PeriodSeconds=60s (instead of the previous 15s), which significantly
slows down Prometheus pod restarts — especially in e2e tests where
restarts are numerous.

Additionally, UWM (User Workload Monitoring) Prometheus still uses the
container patch approach, so reverting makes the two consistent.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 8, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@machine424: This pull request references MON-3697 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

This reverts the changes from PR #2251.

With maximumStartupDurationSeconds=3600, prometheus-operator sets PeriodSeconds=60s (instead of the previous 15s), which significantly slows down Prometheus pod restarts, especially in e2e tests where restarts are numerous.

Additionally, UWM Prometheus still uses the container patch approach, so reverting makes the two consistent.

Telemetry report

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 openshift-eng/jira-lifecycle-plugin repository.

@machine424 machine424 changed the title Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch" WIP: Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch" Jul 8, 2026
@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 Jul 8, 2026
@openshift-ci openshift-ci Bot requested review from jan--f and marioferh July 8, 2026 11:13
@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: machine424

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This change removes the maximumStartupDurationSeconds: 3600 setting from the Prometheus jsonnet component and the corresponding generated prometheus.yaml asset. It replaces this behavior with an explicit StartupProbe configuration added in manifests.go for the prometheus container, setting PeriodSeconds to 15 and FailureThreshold to 240, intended as a workaround pending upstream operator support for startup probe configuration.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed HEAD changes only YAML/jsonnet/manifests files; no *_test.go or Ginkgo It/Describe/Context/When titles were modified.
Test Structure And Quality ✅ Passed The diff touches only YAML/Jsonnet/manifests code; no Ginkgo test files or test blocks were changed, so this quality check is not applicable.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added in this PR; only Prometheus config/manifests changed, and the diff contains no test declarations or MicroShift-sensitive test code.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No Ginkgo e2e tests were added or modified; the PR only changes Prometheus config/manifests, so SNO assumptions are not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed No new topology-sensitive scheduling rules were added; the PR only removes maxStartupDurationSeconds and tweaks Prometheus startupProbe behavior.
Ote Binary Stdout Contract ✅ Passed Touched sources add/remove Prometheus startup-probe settings only; no stdout/logging writes appear in process-level code paths.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only changes manifest/config generation files.
No-Weak-Crypto ✅ Passed Diff only reverts startup-probe timing; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret/token comparisons were added.
Container-Privileges ✅ Passed No privileged settings were added; manifests keep allowPrivilegeEscalation:false and runAsNonRoot:true, and the diff only changes startupProbe/maxStartupDurationSeconds.
No-Sensitive-Data-In-Logs ✅ Passed The patch only removes config and adds a startup probe; no new log statements or sensitive data exposure appears in the changed files.
Title check ✅ Passed The title accurately describes reverting the maximumStartupDurationSeconds change back to the container-patch approach.
Description check ✅ Passed The description explains the revert and includes the required telemetry report section, with no obvious template omissions.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@machine424

Copy link
Copy Markdown
Contributor Author

/test ci/prow/e2e-agnostic-operator

@machine424

Copy link
Copy Markdown
Contributor Author

/test pull-ci-openshift-cluster-monitoring-operator-main-e2e-agnostic-operator

@machine424

Copy link
Copy Markdown
Contributor Author

/test e2e-agnostic-operator

@machine424

Copy link
Copy Markdown
Contributor Author

/retest-required

@machine424 machine424 changed the title WIP: Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch" NO-JIRA: Revert "MON-3697: use maximumStartupDurationSeconds instead of container patch" Jul 8, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@machine424: This pull request explicitly references no jira issue.

Details

In response to this:

This reverts the changes from PR #2251.

With maximumStartupDurationSeconds=3600, prometheus-operator sets PeriodSeconds=60s (instead of the previous 15s), which significantly slows down Prometheus pod restarts, especially in e2e tests where restarts are numerous.

Additionally, UWM Prometheus still uses the container patch approach, so reverting makes the two consistent.


e2e-agnostic-operator runs, revert #2982 vs last 5 PRs merged to main

PR Title Merged Total vs Revert
#2981 OU-1389: config path fix Jul 08 82m55s -5m02s
#2979 OCPBUGS-93756: update prometheus Jul 02 82m11s -4m18s
#2972 OCPBUGS-92085: set Prom shards Jun 26 83m07s -5m14s
#2919 MON-4527: NodeExporter config Jun 25 78m42s -0m49s
#2971 NO-JIRA: sync component versions Jun 25 86m26s -8m33s
AVG BASELINE (5 runs) 82m40s
REVERT #2982 77m53s -4m47s (5.8%)

Revert is faster than every single baseline run.

Per-test breakdown

Test #2981 #2979 #2972 #2919 #2971 AVG REVERT Δ avg
TestAlertmanagerUWMSecrets 349s 201s 350s 211s 201s 262s 191s -71s
TestClusterMonitoringStatus 164s 163s 161s 151s 164s 161s 111s -50s
TestClusterMonitorAlertMgrCfg 84s 101s 99s 30s 98s 83s 31s -52s
TestAlertingRule 46s 38s 21s 51s 62s 44s 7s -37s
TestUserWorkloadPromOperCfg 149s 139s 141s 110s 140s 136s 108s -28s
TestAlertmanagerDisabling 160s 145s 136s 140s 155s 147s 129s -18s

Telemetry report

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2026
@machine424

Copy link
Copy Markdown
Contributor Author

/verified by existing tests

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 8, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@machine424: This PR has been marked as verified by existing tests.

Details

In response to this:

/verified by existing tests

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@machine424: 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-ovn-techpreview 3311f49 link true /test e2e-aws-ovn-techpreview
ci/prow/e2e-hypershift-conformance 3311f49 link true /test e2e-hypershift-conformance

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-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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants