search

argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.13k stars 3.21k forks source link

v3.3.1: only the leader workflow-controller pod can expose metrics. #8283

Open wshi5985 opened 2 years ago

wshi5985 commented 2 years ago

Checklist

* [ ] Double-checked my configuration. * [ ] Tested using the latest version. * [ ] Used the Emissary executor. ## Summary We upgraded argo to 3.3.1 from 3.1.3, There are 2 workflow-controller replica pods somehow after upgrade, only the leader workflow-controller pod can expose metrics. What version are you running? v3.3.1 ## Diagnostics From our prometheus server UI, it shows one of the workflow-controller pod's metrics target DOWN with error: Get "http://10.127.217.135:9090/metrics": dial tcp 10.127.217.135:9090: connect: connection refused While the other pod(the leader pod) shows UP, status is ok. We tried run "wget" to service endpoint workflow-controller-metrics port 9090 (from a testing pod), it could intermittently get metrics result back, and half the time it returns error "wget: can't connect to remote host (172.20.116.209): Connection refused" no related errors found in pod log. ```bash # sometimes it can get result back $ wget http://workflow-controller-metrics.argo:9090/metrics Connecting to workflow-controller-metrics.argo:9090 (172.20.116.209:9090) saving to 'metrics' metrics 100% |*************************************************************************************************************************| 23326 0:00:00 ETA 'metrics' saved # sometimes connection refused $ wget http://workflow-controller-metrics.argo:9090/metrics Connecting to workflow-controller-metrics.argo:9090 (172.20.116.209:9090) wget: can't connect to remote host (172.20.116.209): Connection refused ``` Argo runs in our multiple eks clusters, we upgraded argo to v3.3.1 in two clusters, both have the same issue. the rest clusters with argo v3.1.3 does not have this problem. ---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

github-actions[bot] commented 2 years ago

@wshi5985: There are no area labels on this issue. Adding a label will help expedite your issue. If you are unsure what to do, make your best guess. We can change it later

Details I am a bot created to help the [argoproj](https://github.com/argoproj) developers manage community feedback and contributions. You can check out my [manifest file](https://github.com/argoproj/argo-workflows/blob/master/.github/governance.yml) to understand my behavior and what I can do. If you want to use this for your project, you can check out the [DeFiCh/oss-governance-bot](https://github.com/DeFiCh/oss-governance-bot) repository.
wshi5985 commented 2 years ago

/area metrics

alexec commented 2 years ago

@whynowy we only have one pod serving metrics in v3.3. The service should always route to that pod. Is that something we can do, do you know?

alexec commented 2 years ago

@wshi5985 could you please try adding the readiness check in the attached PR?

whynowy commented 2 years ago

Why adding a readiness check helps? @alexec

alexec commented 2 years ago

The kubelet uses readiness probes to know when a container is ready to start accepting traffic. A Pod is considered ready when all of its containers are ready. One use of this signal is to control which Pods are used as backends for Services. When a Pod is not ready, it is removed from Service load balancers.

https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/

You can put a service in front of the pods for the metrics (we should vend that). It should work.

alexec commented 2 years ago

Non-leader pods should not expose metrics. They don't have anything interesting.

whynowy commented 2 years ago

This means the standby pod will be periodically restarted.

alexec commented 2 years ago

I think that is a liveness probe. If the readiness probe fails, then the pod is just not ready. Correct me if I'm wrong?

wshi5985 commented 2 years ago 
workflow-controller-869578f6c7-7jvhj   0/1     Running   0          3m27s
workflow-controller-869578f6c7-sc9kx   1/1     Running   0          12m

as expected, one pod not ready. 

  Warning  Unhealthy         24s    kubelet            Readiness probe failed: Get "http://10.127.212.119:9090/metrics": dial tcp 10.127.212.119:9090: connect: connection refused

This means there will always be one pod in notReady mode ..... This will trigger our monitor alert i think. Can we just let both pod expose metrics as before ?

alexec commented 2 years ago

Removing the metrics endpoint was not intentional, side-effect of other changes. Do you think it provides useful data?

wshi5985 commented 2 years ago

plus it is tricky for update/deploy, since new pod could not come up while the old pod is leader. i had to delete the old replicaset to let the new pod come up.

wshi5985 commented 2 years ago

@alexec we are not using those metrics right now. we noticed this when prometheus server shows errors.

alexec commented 2 years ago

I'm unclear. Is there another problem here? You should not have to delete any pods for the stand-by leader to come up.

wshi5985 commented 2 years ago

i don't see other problems.

there is a rolling update strategy, 

    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%

when apply the new deploy yaml, one new pod tried to come up, the old 2 pods were holding there waiting for the new one up. but the new one kept in notready mode. the 3 pods hanging there. deploy stuck. until i deleted the old replicaset which deleted the 2 old pods. the first new pod became leader and ready, and the second pod came up in notready mode too.

alexec commented 2 years ago

Are you saying we can't use readiness because it prevents a rolling update?

wshi5985 commented 2 years ago

It looks like a pod could not be picked as leader if it is not ready

just did another test, set rolling update strategy to 50%, with readinessProbe configured when change applied, 2 new pods tried to come up, the old leader pod kept up, the old stand-by pod terminated.
both 2 new pods failed readiness probe, none of them ready, none of them could be picked up as leader. until i delete the old replicaset(old pods gone), then one of the new one ready and picked as leader, the other one in notReady mode as expected.

if remove readinessProbe, both pod ready, and one picked as leader. no need to delete old replicaset.

wshi5985 commented 2 years ago

btw, if we remove the workflow-controller-metrics service, what will be the impact (beside no metric exposed)? anything depends on the exposed metrics ?

alexec commented 2 years ago

Leader election is unreleated to readiness. I don't believe readiness (or liveness) will affect it.

whynowy commented 2 years ago

Why don't abandon using Service to expose metrics, instead, recommend to configure pod discovery in prometheus?

wshi5985 commented 2 years ago

during deployment, when readinessprobe failed, pod "ready" status showed "0/1"(instead of "1/1") and deployment stuck. i am not sure why. and our setup use prometheus-operator, which use serviceMonitor -> service for metrics collection.

alexec commented 2 years ago

I don't think you can use rolling update at all.

Consider this. You have 3 replicas. If you want to use service, you must have readiness check, otherwise you get errors.

Only one is leader, so only one is ready.

When rolling update occurs, the leader will not be deleted until another replica becomes ready. This will never happen, because replicas only become ready when they become leader. Catch 22!

Instead, you could use:

  strategy:
    type: Recreate

Could you please try that?

wshi5985 commented 2 years ago

with recreate strategy, it unblocks deploy. but with one pod always in notready status, it may trigger alert too. (we monitor unhealthy pods in production env) if there is no other side effect. i guess we will keep the initial config, (no readiness check) i understand only leader pod exposes meaningful metrics, but what will be the impact if standby expose metrics too ?

alexec commented 2 years ago

The impact is that we need to change the way the controller works. It currently doesn't do anything until it becomes leader. This is to design-out bugs. To change that, we need to do work and create risk.

wshi5985 commented 2 years ago

Thanks for the quick response and explain. just one more question. this change(Removing the non-leader metrics endpoint) started from which version ? Thanks.

alexec commented 2 years ago

v3.3

wshi5985 commented 2 years ago

Thanks

MatthewHou commented 2 years ago

@whynowy Can you plz elaborate a bit more on configure pod discovery? At the moment we use ServiceMonitor CRD to collect metrics from workflow-controller. We also noticed that 1 of the scrape target is failing due the change in non-leader controller metrics. How should we work around it? Looks like PodMonitor won't work unless some additional labels could added to the leader pod

Why don't abandon using Service to expose metrics, instead, recommend to configure pod discovery in prometheus?

tyrken commented 2 years ago

FWIW adding the readinessCheck (& switching to strategy Recreate) didn't actually fix the kube-prometheus-stack TargetDown alert - the endpoints were still there (in our EKS 1.22 cluster) and so used by ServiceMonitor to config Prom - not sure why as your discussion above sounds reasonable.

... but even if that worked it doesn't seem a good solution, as then we'd have a pod continually sitting "unready" which would trigger another alert looking for containers staying "unready" for too long. Could we not have the metrics endpoint back, but serving up no metrics values? This idea just as an alternative to @MatthewHou 's suggestion above if you don't want to edit your own metadata on the fly (not sure which is easiest/cleanest).

sherifabdlnaby commented 1 year ago

@tryken Totally Agree! Deliberately making replicas unhealthy is a hacky workaround (And I think it won't work in Prometheus' issue).

An "Unhealthy" Pod will trigger other alerts for Pods staying unhealthy for a long time because you shouldn't have unhealthy pods in the cluster for too long. Also, the pod is not really unhealthy... it's just idle 🤷🏻

Other solutions like adding Per-Pod Monitor in Prometheus to let it ignore non-leader pods are also hacky, as you'll need to re-adjust Prometheus whenever there is a new leader.

I believe having /metrics on the non-primary leader that serves no metrics is the best option.

yevhen-harmonizehr commented 1 year ago

@alexec sorry to bother, but is there any chance this will be reopened? I can see it is still an very annoying issue.

sherifabdlnaby commented 1 year ago

This issue shouldn't have been closed :/

terrytangyuan commented 1 year ago

https://github.com/argoproj/argo-workflows/pull/11295 should fix this. Could you try latest tag?

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

srinivin commented 1 year ago

Still seeing the issue. Using the version 3.4.11.

I have replicaset as 3 and when trying to access metrics through the service, i see the response is empty otherwise works if i explicitly portforward from the leader pod.

terrytangyuan commented 1 year ago

@sakai-ast Would you like to help take a look to see what's still missing?

sakai-ast commented 1 year ago

@srinivin @terrytangyuan I think this is correct behavior. You sometimes can get metrics through the service, but it's not always because the service can't detect the leader pod, so sometimes the service randomly accesses to the non-leader pods, and then you get empty response.

Before https://github.com/argoproj/argo-workflows/pull/11295, you get an error like "Connection refused" when your service accesses the non-leader pods.

I also tested the behavior of v3.4.11 and v3.4.9(before the fix version) with curl to the service locally.

v3.4.11

when accessing non-leader pod

~ $ curl -v http://argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080/metrics
*   Trying 192.168.194.156:8080...
* Connected to argo-argo-workflows-workflow-controller.argo.svc.cluster.local (192.168.194.156) port 8080
> GET /metrics HTTP/1.1
> Host: argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080
> User-Agent: curl/8.3.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Date: Sun, 24 Sep 2023 04:10:59 GMT
< Content-Length: 0

when accessing leader pod

~ $ curl -v http://argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080/metrics
*   Trying 192.168.194.156:8080...
* Connected to argo-argo-workflows-workflow-controller.argo.svc.cluster.local (192.168.194.156) port 8080
> GET /metrics HTTP/1.1
> Host: argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080
> User-Agent: curl/8.3.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: text/plain; version=0.0.4; charset=utf-8
< Date: Sun, 24 Sep 2023 04:11:30 GMT
< Transfer-Encoding: chunked
<
# HELP argo_workflows_count Number of Workflows currently accessible by the controller by status (refreshed every 15s)
# TYPE argo_workflows_count gauge
argo_workflows_count{status="Error"} 0
argo_workflows_count{status="Failed"} 0
argo_workflows_count{status="Pending"} 0
...

v3.4.9

when accessing non-leader pod

~ $ curl -v http://argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080/metrics
*   Trying 192.168.194.156:8080...
* connect to 192.168.194.156 port 8080 failed: Connection refused
* Failed to connect to argo-argo-workflows-workflow-controller.argo.svc.cluster.local port 8080 after 2 ms: Couldn't connect to server
* Closing connection
curl: (7) Failed to connect to argo-argo-workflows-workflow-controller.argo.svc.cluster.local port 8080 after 2 ms: Couldn't connect to server

when accessing leader pod

~ $ curl -v http://argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080/metrics
*   Trying 192.168.194.156:8080...
* Connected to argo-argo-workflows-workflow-controller.argo.svc.cluster.local (192.168.194.156) port 8080
> GET /metrics HTTP/1.1
> Host: argo-argo-workflows-workflow-controller.argo.svc.cluster.local:8080
> User-Agent: curl/8.3.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: text/plain; version=0.0.4; charset=utf-8
< Date: Sun, 24 Sep 2023 04:14:19 GMT
< Transfer-Encoding: chunked
<
# HELP argo_workflows_count Number of Workflows currently accessible by the controller by status (refreshed every 15s)
# TYPE argo_workflows_count gauge
argo_workflows_count{status="Error"} 0
argo_workflows_count{status="Failed"} 0
argo_workflows_count{status="Pending"} 0
argo_workflows_count{status="Running"} 0
...

If your service always gets an empty response, that is an another problem.

srinivin commented 1 year ago

Thanks for the update.

I assume in these cases, shouldnt it behave same as a like sql write and read replicas?

I mean though we direct write to only primary relicas, however read can happen from any replicas. same ways, though the leader pod is responsible for triggering workflows and other capablities, shouldnt the non-leader pods provide the metrics metadata. I assume the state of the metrics can be in a sharable persistence layer b/w leader and non-leader pods?