OTA-855: Create GET requests to the Prometheus API server #999
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
approved
DavidHurta
force-pushed
the
exchange-prometheus-client
branch
from
d47c148 to
d4f4c9b
Compare
|
/test all |
DavidHurta
changed the title
DavidHurta
force-pushed
the
exchange-prometheus-client
branch
from
870cd9a to
d00ab44
Compare
DavidHurta
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@Davoska: This pull request references OTA-855 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 story to target the "4.15.0" version, but no target version was set. DetailsIn 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/test-infra repository. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/test unit |
DavidHurta
force-pushed
the
exchange-prometheus-client
branch
from
d00ab44 to
2230cd1
Compare
This commit will create GET requests to the Prometheus API server. The used library doesn't allow to specify the type of requests (GET/POST) being made to the Prometheus API server. The tenancy port of the thanos-querier.openshift-monitoring.svc service being utilized in self-managed HyperShift requires different permissions depending on the type of requests. This commit will make sure that the CVO uses GET requests to the PromQL service. The used Prometheus client library attempts POST requests and on 501 and 405 response status codes fall-backs to GET requests. This commit will return the 501 status code even before sending a given HTTP request, resulting in a GET request being made as a fallback by the used library.
DavidHurta
force-pushed
the
exchange-prometheus-client
branch
from
2230cd1 to
64906e9
Compare
|
The |
|
/hold I want to test the new changes and make sure we have an agreement on whether to have this new logic only for HyperShift. |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest |
|
/unhold Feedback addressed. |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
I see David tested this earlier, but repeating to fill in some details, using the flow from OTA-520. Cluster Bot $ oc get -o json clusterversion version | jq -r '.status.desired'
{
"image": "registry.build04.ci.openshift.org/ci-ln-83x5tnb/release@sha256:def72cb591c2b8f990f05309df5f0a32c714cb0534247d55d25cd80f34dfd58d",
"version": "4.15.0-0.test-2023-12-01-230104-ci-ln-83x5tnb-latest"
}I updated my $ oc patch clusterversion version --type json -p '[{"op": "add", "path": "/spec/upstream", "value": "https://raw.githubusercontent.com/wking/cincinnati-graph-data/demo-cache-warming/cincinnati-graph.json"}]'
$ oc adm upgrade channel buggy
warning: No channels known to be compatible with the current version "4.15.0-0.test-2023-12-01-230104-ci-ln-83x5tnb-latest"; unable to validate "buggy". Setting the update channel to "buggy" anyway.
$ oc adm upgrade --include-not-recommended
Cluster version is 4.15.0-0.test-2023-12-01-230104-ci-ln-83x5tnb-latest
Upstream: https://raw.githubusercontent.com/wking/cincinnati-graph-data/demo-cache-warming/cincinnati-graph.json
Channel: buggy
No updates available. You may still upgrade to a specific release image with --to-image or wait for new updates to be available.
Supported but not recommended updates:
Version: 4.15.1
Image: quay.io/openshift-release-dev/ocp-release@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
Recommended: False
Reason: MultipleReasons
Message: Could not evaluate exposure to update risk A (evaluation is throttled)
A description: A.
A URL: https://bug.example.com/a
B. https://bug.example.com/b
Could not evaluate exposure to update risk C (evaluation is throttled until 00:19:00Z)
C description: C.
C URL: https://bug.example.com/c
Could not evaluate exposure to update risk D (evaluation is throttled until 00:19:00Z)
D description: D.
D URL: https://bug.example.com/d
Could not evaluate exposure to update risk E (evaluation is throttled until 00:19:00Z)
E description: E.
E URL: https://bug.example.com/e
$ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep 'evaluate PromQL'
I1202 00:05:40.751294 1 promql.go:170] evaluate PromQL cluster condition: "max(cluster_proxy_enabled{type=~\"https?\"})"
I1202 00:05:41.783447 1 promql.go:170] evaluate PromQL cluster condition: "group(cluster_version_available_updates{channel=\"buggy\"})\nor\n0 * group(cluster_version_available_updates{channel!=\"buggy\"})"
I1202 00:09:54.193249 1 promql.go:170] evaluate PromQL cluster condition: "group(csv_succeeded{name=~\"local-storage-operator[.].*\"}) or 0 * group(csv_count)"
I1202 00:13:42.027462 1 promql.go:170] evaluate PromQL cluster condition: "0 * max(cluster_version)"
I1202 00:18:59.637248 1 promql.go:170] evaluate PromQL cluster condition: "0 * 0 * max(cluster_version)"Not clear why that isn't getting 518b446 (#939)'s 1s |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Davoska, LalatenduMohanty, petr-muller, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Ok, I understand the slower PromQL evaluation now:
Makes sense, and is not related to the change made in this pull. |
|
Pre-merge test by conducting regression test against conditional updates // Launch a cluster using cluster-bot // Patch a dummy cincinnati // Check conditional updates // CVO evaluates the unknown risks every second // Patch to an invalid cincinnati // CVO looks good |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
@Davoska: This pull request references OTA-855 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 story to target the "4.15.0" version, but no target version was set. DetailsIn 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/test-infra repository. |
|
HyperShift CI is struggling more broadly. It passed earlier on this pull, and we know this is working for standalone. /override ci/prow/e2e-hypershift |
|
@wking: Overrode contexts on behalf of wking: ci/prow/e2e-hypershift DetailsIn 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/test-infra repository. |
|
/override ci/prow/e2e-hypershift-conformance |
|
@petr-muller: Overrode contexts on behalf of petr-muller: ci/prow/e2e-hypershift-conformance DetailsIn 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/test-infra repository. |
|
@Davoska: 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/test-infra repository. I understand the commands that are listed here. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build cluster-version-operator-container-v4.15.0-202312051932.p0.gfb57321.assembly.stream for distgit cluster-version-operator. |
7 participants
This pull request will create GET requests to the Prometheus API server.
The currently used Prometheus client library doesn't allow explicit specification of the type of requests (GET/POST) being made to the Prometheus API server. The tenancy port of the
thanos-querier.openshift-monitoring.svcservice being utilized in self-managed HyperShift requires different permissions depending on the type of requests. This PR will make sure that the CVO uses GET requests to the PromQL service.The used Prometheus client library attempts POST requests, and on 501 and 405 response status codes fall-backs to GET requests. This PR will return the 501 status code even before sending a given HTTP POST request, resulting in a GET request being made as a fallback by the used library.
An alternative is to use a different Prometheus client library (see DavidHurta@d4f4c9b). However, this vendors a lot of new packages creating potential vulnerabilities in the future. Another alternative is to fully implement specific interfaces, which may result in potential regression and bugs.