actions / actions/actions-runner-controller

`ServiceMonitor` label selector logic for github-webhook-server component is broken

Open
#2,106 1 comment 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs triage
Dominant language
Go
Stars
6.5k
Forks
1.5k
Avg merge
2d 2h
Merged PRs (30d)
27

Description

Checks
Controller Version

0.26.0

Helm Chart Version

0.21.1

CertManager Version

1.8.0

Deployment Method

Helm

cert-manager installation

n/a. this has nothing to do with the controller. this is a helm chart bug.

Checks
  • This isn't a question or user support case (For Q&A and community support, go to Discussions. It might also be a good idea to contract with any of contributors and maintainers if your business is so critical and therefore you need priority support
  • I've read releasenotes before submitting this issue and I'm sure it's not due to any recently-introduced backward-incompatible changes
  • My actions-runner-controller version (v0.x.y) does support the feature
  • I've already upgraded ARC (including the CRDs, see charts/actions-runner-controller/docs/UPGRADING.md for details) to the latest and it didn't fix the issue
  • I've migrated to the workflow job webhook event (if you using webhook driven scaling)
Resource Definitions
n/a. this has nothing to do with the controller. this is a helm chart bug.
To Reproduce
1. run

$ helm template test actions-runner-controller/actions-runner-controller \
    --version 0.21.1 \
    --set "githubWebhookServer.enabled=true,metrics.serviceMonitor=true"
Describe the bug

the Service by the name of test-actions-runner-controller-github-webhook-server has the following labels:

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test

the ServiceMonitor by the name of test-actions-runner-controller-github-webhook-s-service-monitor (which should be selecting the above Service) has the label selectors:

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test-github-webhook-server

i.e. this ServiceMonitor does not select any Services (and certainly not the Service of the github-webhook-server).

furthermore, the ServiceMonitor for the main controller component (test-actions-runner-controller-service-monitor) now selects a mixed set of controller- and github-webhook-server pods (because both Services have the same name & instance labels)... which means that the metrics for the main controller component will be somewhat "confused" when the github-webhook-server component is enabled.

Describe the expected behavior

there's an error in the labeling logic for the github-webhook-server Service.

this:

https://github.com/actions/actions-runner-controller/blob/actions-runner-controller-0.21.1/charts/actions-runner-controller/templates/githubwebhook.service.yaml#L7-L8

should be

  labels:
    {{- include "actions-runner-controller-github-webhook-server.selectorLabels" . | nindent 4 }}

by the same token, it would probably be more correct to add the same labels to the ServiceMonitor itself as well... though that would be a cosmetic fix only:

https://github.com/actions/actions-runner-controller/blob/actions-runner-controller-0.21.1/charts/actions-runner-controller/templates/githubwebhook.serviceMonitor.yaml#L5-L6

alternatively, you could turn the ServiceMonitor for the github-webhook-server component into a PodMonitor since the pod labels are already correct.

if you did that, you would no longer need:

https://github.com/actions/actions-runner-controller/blob/actions-runner-controller-0.21.1/charts/actions-runner-controller/templates/githubwebhook.service.yaml#L19-L23

however, that would make the helm config metrics.serviceMonitor somewhat misleading.

Whole Controller Logs
n/a. this has nothing to do with the controller. this is a helm chart bug.
Whole Runner Pod Logs
n/a. this has nothing to do with the controller. this is a helm chart bug.
Additional Context

on a related note...

the label app.kubernetes.io/instance is not used correctly in this helm chart.

i realize that this was most likely a concession to backward compatibility. but if you ever have to release a breaking change for the helm chart, you might as well fix the labeling to be e.g.:

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test
app.kubernetes.io/component: controller

and

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test
app.kubernetes.io/component: github-webhook-server

instead of

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test

and

app.kubernetes.io/name: actions-runner-controller
app.kubernetes.io/instance: test-github-webhook-server

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with charts/actions-runner-controller/templates/githubwebhook.service.yaml and githubwebhook.serviceMonitor.yaml, then run the Helm template command from the reproduction steps. Verify that the generated ServiceMonitor selects the github-webhook-server Service and that the controller ServiceMonitor does not select its pods; the corrected rendered labels are the done condition.

Written by the indexing model from the issue text.

Assessment

Tech stack
helm, kubernetes
Domain
infrastructure, observability
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.