[MON-1695] expose /api/v1/labels end point for Thanos query. #1299

raptorsun · 2021-07-25T20:15:50Z

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

Expose /api/v1/labels end point for Thanos query.
End point for /api/v1/label//values is added but not working yet, need to update kube-rbac-proxy to support at least trailing wildcard patterns.

This PR requires the 2 PRs below done before merge, and they are merged now:

This PR is merged for allowing trailing wildcard in kube-rbac-proxy --allow-paths .
This PR is merged for version bump to 0.11.

philipgough

/lgtm

philipgough · 2021-07-26T06:00:08Z

/hold

do we want to wait for the kube-rbac-proxy change before testing/merging?

raptorsun · 2021-07-26T10:20:56Z

/hold

do we want to wait for the kube-rbac-proxy change before testing/merging?

yes, let's hold it before modifying kube-rbac-proxy.

raptorsun · 2021-07-26T10:22:40Z

This PR is created for allowing trailing wildcard in kube-rbac-proxy --allow-paths .

raptorsun · 2021-07-26T16:00:15Z

/retest

raptorsun · 2021-07-28T12:25:36Z

/retest

raptorsun · 2021-08-02T07:33:55Z

now we are waiting for a new release from kube-rbac-proxy that includes This PR

assets/alertmanager/alertmanager.yaml

paulfantom · 2021-08-04T13:10:41Z

jsonnet/thanos-querier.libsonnet

@@ -493,6 +493,7 @@ function(params)
                  '--insecure-listen-address=127.0.0.1:9095',
                  '--upstream=http://127.0.0.1:9090',
                  '--label=namespace',
+                  '-enable-label-apis',


Suggested change

'-enable-label-apis',

'--enable-label-apis',

updated, thanks 👍

paulfantom · 2021-08-11T10:44:06Z

jsonnet/thanos-querier.libsonnet was moved to https://github.com/openshift/cluster-monitoring-operator/blob/master/jsonnet/components/thanos-querier.libsonnet

raptorsun · 2021-09-14T09:43:51Z

/retest-required

simonpasquier · 2021-09-14T13:23:42Z

test/e2e/user_workload_monitoring_test.go

+		t.Fatal(err)
+	}
+
+	// Grant enough permissions to read labels. todo: check if need create new role


The labels endpoints are similar to the query endpoints: if you are authorized for /api/v1/query, you should also be for /api/v1/labels.

Thanks for the clarification. The test for labels will use the same role as that for query.

can you remove the todo: ... part (or address it)?

yup. already checked: no need for another account

simonpasquier · 2021-09-14T13:24:07Z

test/e2e/user_workload_monitoring_test.go

+
+	t.Logf("Checking all labels")
+
+	// check /api/v1/labels end point


Suggested change

// check /api/v1/labels end point

// check /api/v1/labels endpoint.

still need fixing :)

simonpasquier · 2021-09-14T13:28:57Z

test/e2e/user_workload_monitoring_test.go

+	}
+
+	// check /api/v1/label/<label>/value using some examples
+	for _, label := range []string{"endpoint", "event", "node"} {


I would check that the namespace label returns only 1 item (e.g. userWorkloadTestNs).

OK, I will modify the test accordingly.

The test has been updated to include the namespace label.

juzhao · 2021-09-16T10:56:44Z

can get /api/v1/labels and /api/v1/label/$({label_name}/values for user project, example

# curl -k -H "Authorization: Bearer $token" 'https://thanos-querier.openshift-monitoring.svc:9092/api/v1/labels?namespace=ns1' | jq
{
  "status": "success",
  "data": [
    "__name__",
    "alertname",
    "alertstate",
    "cluster_ip",
    "condition",
    "configmap",
    "container",
    "container_id",
    "cpu",
    "created_by_kind",
    "created_by_name",
    "deployment",
    "device",
    "endpoint",
    "host_ip",
    "host_network",
    "id",
...
}

# curl -k -H "Authorization: Bearer $token" 'https://thanos-querier.openshift-monitoring.svc:9092/api/v1/label/alertname/values?namespace=ns1'  | jq
{
  "status": "success",
  "data": [
    "TestAlert"
  ]
}

/label qe-approved

raptorsun · 2021-09-17T12:37:29Z

/retest-required

raptorsun · 2021-09-24T09:11:34Z

/label docs-approved

simonpasquier · 2021-09-24T09:21:58Z

test/e2e/user_workload_monitoring_test.go

+		t.Fatalf("failed to query labels from Thanos querier: %v", err)
+	}
+
+	// check /api/v1/label/<label>/value using some examples


I think that checking /api/v1/label/namespace/values is enough

tests for other labels have been removed.

simonpasquier

/hold

still some comments to be addressed.

raptorsun · 2021-09-24T15:55:55Z

/hold

still some comments to be addressed.

E2E test has been updated according to comments.

openshift-ci · 2021-09-24T17:29:47Z

@raptorsun: 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/versions	`14bf8bc`	link	false	`/test versions`
ci/prow/e2e-aws-single-node	`14bf8bc`	link	false	`/test e2e-aws-single-node`

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/test-infra repository. I understand the commands that are listed here.

raptorsun · 2021-09-25T11:59:09Z

/retest-required

simonpasquier · 2021-09-27T08:14:11Z

/lgtm

openshift-ci · 2021-09-27T08:14:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jan--f, PhilipGough, raptorsun, simonpasquier

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:

~~OWNERS~~ [PhilipGough,jan--f,raptorsun,simonpasquier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

raptorsun · 2021-09-27T08:15:42Z

/unhold

openshift-bot · 2021-09-27T08:38:11Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-27T08:50:11Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-27T09:14:14Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

raptorsun · 2021-09-27T09:34:04Z

/label px-approved

openshift-bot · 2021-09-27T09:38:12Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci bot requested review from philipgough and simonpasquier July 25, 2021 20:16

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 25, 2021

raptorsun force-pushed the feature/MON-1695 branch from f1d13bc to 7ebeecb Compare July 25, 2021 20:19

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 25, 2021

raptorsun force-pushed the feature/MON-1695 branch from 7ebeecb to 2277097 Compare July 25, 2021 20:21

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 25, 2021

philipgough approved these changes Jul 26, 2021

View reviewed changes

openshift-ci bot assigned philipgough Jul 26, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2021

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2021

raptorsun force-pushed the feature/MON-1695 branch from 2277097 to 2f7ebd9 Compare July 26, 2021 10:20

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2021

raptorsun force-pushed the feature/MON-1695 branch from 2f7ebd9 to f6cdefc Compare July 28, 2021 09:16

raptorsun force-pushed the feature/MON-1695 branch from f6cdefc to 213f9f9 Compare August 4, 2021 09:22

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2021

raptorsun force-pushed the feature/MON-1695 branch from 213f9f9 to 86cd670 Compare August 4, 2021 09:46

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2021

paulfantom reviewed Aug 4, 2021

View reviewed changes

assets/alertmanager/alertmanager.yaml Outdated Show resolved Hide resolved

paulfantom reviewed Aug 4, 2021

View reviewed changes

raptorsun force-pushed the feature/MON-1695 branch 2 times, most recently from 136aa84 to 84cd17b Compare August 4, 2021 16:04

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2021

simonpasquier reviewed Sep 14, 2021

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Sep 16, 2021

raptorsun force-pushed the feature/MON-1695 branch from b4c69b4 to e57442e Compare September 17, 2021 09:48

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Sep 24, 2021

simonpasquier reviewed Sep 24, 2021

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2021

raptorsun force-pushed the feature/MON-1695 branch from e57442e to 14bf8bc Compare September 24, 2021 15:54

e2e test for prometheus endpoint /api/v1/labels

14bf8bc

openshift-ci bot assigned simonpasquier Sep 27, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2021

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Sep 27, 2021

openshift-merge-robot merged commit 709876a into openshift:master Sep 27, 2021

simonpasquier mentioned this pull request Dec 17, 2021

Enable the /api/v1/series endpoint on the Thanos tenancy port #1519

Merged

2 tasks


		t.Logf("Checking all labels")

		// check /api/v1/labels end point

	// check /api/v1/labels end point
	// check /api/v1/labels endpoint.

[MON-1695] expose /api/v1/labels end point for Thanos query. #1299

[MON-1695] expose /api/v1/labels end point for Thanos query. #1299

Conversation

raptorsun commented Jul 25, 2021 • edited Loading

philipgough left a comment

Choose a reason for hiding this comment

philipgough commented Jul 26, 2021

raptorsun commented Jul 26, 2021

raptorsun commented Jul 26, 2021

raptorsun commented Jul 26, 2021

raptorsun commented Jul 28, 2021

raptorsun commented Aug 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulfantom commented Aug 11, 2021

raptorsun commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juzhao commented Sep 16, 2021 • edited Loading

raptorsun commented Sep 17, 2021

raptorsun commented Sep 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonpasquier left a comment

Choose a reason for hiding this comment

raptorsun commented Sep 24, 2021

openshift-ci bot commented Sep 24, 2021 • edited Loading

raptorsun commented Sep 25, 2021

simonpasquier commented Sep 27, 2021

openshift-ci bot commented Sep 27, 2021

raptorsun commented Sep 27, 2021

openshift-bot commented Sep 27, 2021

openshift-bot commented Sep 27, 2021

openshift-bot commented Sep 27, 2021

raptorsun commented Sep 27, 2021

openshift-bot commented Sep 27, 2021

raptorsun commented Jul 25, 2021 •

edited

Loading

juzhao commented Sep 16, 2021 •

edited

Loading

openshift-ci bot commented Sep 24, 2021 •

edited

Loading