Skip to content

馃悰 (helm/v2-alpha): keep metrics flags and metrics resources in sync. - #6028

Open
camilamacedo86 wants to merge 1 commit into
kubernetes-sigs:masterfrom
camilamacedo86:helm-v2-metrics
Open

camilamacedo86 wants to merge 1 commit into
kubernetes-sigs:masterfrom
camilamacedo86:helm-v2-metrics

Conversation

@camilamacedo86

@camilamacedo86 camilamacedo86 commented Sep 8, 2026

Copy link
Copy Markdown
Member

The ServiceMonitor now renders only when both prometheus.enabled and metrics.enabled are true. Charts generated from a manager that has no args field, an empty args list, or no --metrics-bind-address argument now get the metrics bind address added, so metrics.enabled, metrics.port, and metrics.secure always control the manager. Sidecar container arguments are left untouched.

Closes: #6021

@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

馃煛 Changes recommended

ServiceMonitors remain independently enabled when metrics are disabled, contrary to the synchronization goal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Synchronizes Helm metrics values with manager arguments and generated metrics resources.

Changes:

  • Synthesizes metrics arguments for managers without them.
  • Adds rendered-chart coverage for metrics toggles, ports, security, and sidecars.
  • Documents fallback bind-address generation.
File summaries
File Description
chart_generation_integration_test.go Expands Helm rendering tests and helpers.
manager.go Generates and templates manager metrics arguments.
helm-v2-alpha.md Documents metrics port behavior.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

馃挕 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

馃煛 Changes recommended

The implementation is sound, but the PR title violates the repository鈥檚 mandatory title format.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/15 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@camilamacedo86 camilamacedo86 changed the title 馃悰. fix(helm/v2-alpha): keep metrics flags and resources in sync 馃悰 (helm/v2-alpha): keep metrics flags and metrics resources in sync. Sep 8, 2026
@kubernetes-prow kubernetes-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

PR needs rebase.

Details

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

helm/v2-alpha: the generated chart exposes metrics values the manager never receives

2 participants