Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CR and CRB to the helm chart #2504

Merged
merged 4 commits into from
Apr 27, 2023
Merged

Add CR and CRB to the helm chart #2504

merged 4 commits into from
Apr 27, 2023

Conversation

totogtr
Copy link
Contributor

@totogtr totogtr commented Apr 12, 2023

In response to #2212 , the ARC helm chart is missing ClusterRoleBinding and ClusterRole for the ActionsMetricsServer resulting on missing permissions.

This also fix the labels of the ActionsMetricsServer Service as it is selected by the ServiceMonitor with those labels.

@Link- Link- added needs triage Requires review from the maintainers community Community contribution labels Apr 17, 2023
@@ -5,7 +5,7 @@ metadata:
name: {{ include "actions-runner-controller-actions-metrics-server.fullname" . }}
namespace: {{ .Release.Namespace }}
labels:
{{- include "actions-runner-controller.labels" . | nindent 4 }}
{{- include "actions-runner-controller-actions-metrics-server.selectorLabels" . | nindent 4 }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, good catch!

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @totogtr! Thank you for your contribution! I just reviewed this and had two comments regarding the values used to enable the CR and CRB creations. Would you mind confirming when you have some time?

@totogtr
Copy link
Contributor Author

totogtr commented Apr 24, 2023

Hey @mumoshu, thanks a lot for reviewing. Indeed the enable conditions were wrong and fixed by your suggestion, I must have done some bad copy pasting here sorry !

Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot for your contribution @totogtr!

@mumoshu mumoshu merged commit 21722a5 into actions:master Apr 27, 2023
mumoshu added a commit that referenced this pull request May 11, 2023
Sorry for the delayed release!

This chart version is mainly for upgrading ARC to v0.27.4, along with a few fixes to the chart itself (#2504, #2498)

Please also refer to https://github.com/actions/actions-runner-controller/releases/tag/v0.27.4 for more details about changes made in ARC v0.27.4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution needs triage Requires review from the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants