Add initial e2e tests

[olivierlemasle]

This adds E2E tests to prometheus-adapter.
These tests:

• bring up a kind cluster (or re-use an existing k8s cluster)
• build the prometheus-adapter image
• deploy prometheus-operator and a prometheus instance
• deploy prometheus-adapter using the manifests in deploy/manifests

It checks that everything can be deployed, and, as a first test, checks that:

• nodes metrics bring positive values
• there are some pod metrics

It also prints the prometheus-adapter logs.

[dgrisonnet]

what do you think about reusing the prometheus-operator e2e test framework for that? https://github.com/prometheus-operator/prometheus-operator/blob/1dfecab7c2706f3591d6bb6d5eb8f6b2518f1b45/test/framework/prometheus.go#L309-L320

[olivierlemasle]

That's a good idea. I tried but got stuck in dependency hells, as prometheus-operator brings Kubernetes, prometheus and opentelemetry dependencies in different versions than prometheus-adapter.
However, I could use prometheus-operator client (which is a different module with fewer dependencies) to make this function less verbose, if you think it's better.

[dgrisonnet]

That's a good idea. I tried but got stuck in dependency hells, as prometheus-operator brings Kubernetes, prometheus and opentelemetry dependencies in different versions than prometheus-adapter.

Oh that's unfortunate, I am fine with the current approach, just though that it could maybe be simplified by importing existing code.

However, I could use prometheus-operator client (which is a different module with fewer dependencies) to make this function less verbose, if you think it's better.

Yeah that's a good idea

[olivierlemasle]

I refactored the tests to use the prometheus-operator client.

[dgrisonnet]

Could you make this ServiceMonitor more minimal? We shouldn't need all the relabeling rules here and the only path that we want to scrape on Kubelet is /metrics/resources.

[olivierlemasle]

I made the ServiceMonitor more minimal. However, I'm still scraping /metrics/cadvisor, not /metrics/resource, which do not provide container_cpu_usage_seconds_total{id='/'}.

[dgrisonnet]

It was renamed to container_cpu_usage_seconds in kubernetes/kubernetes#86282. Since /metrics/resource is a more lightweight version of /metrics/cadvisor, it would be better to use the new endpoint and the new metric.

[olivierlemasle]

Actually, kubernetes/kubernetes#86282 was kind of reverted by kubernetes/kubernetes#89540 in Kubernetes 1.19, so the metric is named container_cpu_usage_seconds_total (for k8s < 1.18 and k8s >= 1.19), and that's the metric documented.

What I meant is that, using the path /metrics/resource, querycontainer_cpu_usage_seconds_total{id='/'} returns nothing, because the label id is never empty (however it works on path /metrics/cadvisor).

So I've updated the configmap manifest to use node_cpu_usage_seconds_total and node_memory_working_set_bytes instead. I can make a separate PR for that if you prefer. I was unsure because you suggested yourself in #531 (comment) to use {id='/'} for node queries.

[dgrisonnet]

What I meant is that, using the path /metrics/resource, querycontainer_cpu_usage_seconds_total{id='/'} returns nothing, because the label id is never empty (however it works on path /metrics/cadvisor

Oh that's interesting, I wasn't aware that id wasn't included in the metric exposed /metrics/resource. I guess an alternative could then be to group by node label since it is injected by prometheus-operator, but at this point I am not really sure what's best between that and depending on node-exporter. For sure we should move away from /metrics/cadvisor since it is bound to disappear.

[dgrisonnet]

Thinking about it again, let's move away from /metrics/cadvisor for node metrics and rely on node-exporter instead since we can't get the node-level information from /metrics/resource. That's what most of the community is using anyway and we've already had asks to switch: #516

So for the tests we would want:

1. ServiceMonitor on kubelet /metrics/resource for CPU queries
2. ServiceMonitor on node-exporter for Node queries

[olivierlemasle]

@dgrisonnet I'm not sure I understand the issue with the way it works in this PR.

node_cpu_usage_seconds_total and node_memory_working_set_bytes are node metrics provided by the kubelet itself (cf https://github.com/kubernetes/kubernetes/blob/8f94681cd294aa8cfd3407b8191f6c70214973a4/pkg/kubelet/metrics/collectors/resource_metrics.go#L30-L42), so we dont't need node-exporter, do we?

That being said, I guess that prometheus-adapter is often used in conjunction with node-exporter (in kube-prometheus or openshift's cluster-monitoring-operator), so I'm not against using node-exporter in default manifests and in E2E.

[dgrisonnet]

😩 I totally forgot that node-level metrics were introduced sorry about that.

IIRC node-exporter and kubelet are getting the data from the same place so reducing the dependencies to just kubelet would definitely be better.

[dgrisonnet]

Thank you for bearing with me on this one. As a follow-up we should enable the tests in CI.

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgrisonnet, olivierlemasle

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 [dgrisonnet]

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

[olivierlemasle]

Thank you @dgrisonnet 🎉

I've just updated kubernetes/test-infra#27948