Skip to content

Conversation

@francesco-furlan
Copy link
Contributor

What this PR does / why we need it:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Title of the PR starts with type and scope, (e.g. feat(agent,node-analyzer,sysdig-deploy):)
  • Chart Version bumped for the respective charts
  • Variables are documented in the README.md (or README.tpl in some charts)
  • Check GithubAction checks (like lint) to avoid merge-check stoppers
  • All test files are added in the tests folder of their respective chart and have a "_test" suffix

Check Contribution guidelines in README.md for more insight.

@francesco-furlan francesco-furlan self-assigned this Feb 20, 2024
@francesco-furlan francesco-furlan requested a review from a team as a code owner February 20, 2024 14:45
Comment on lines 50 to 52
{{- if and (include "agent.gke.autopilot" .) (or (not (hasKey $baseSettings "promscrape_web_address")) (eq (get $baseSettings "promscrape_web_address") "127.0.0.1:9990") ) }}
{{- $baseSettings := mergeOverwrite $baseSettings (dict "promscrape_web_address" "127.0.0.1:9991") -}}
{{- end }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should not be needed. prometheus.enabled is set to false earlier in the configuration, and promscrape will not run in that situation.
cc @thom-sd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch, thanks!

Copy link

@fuster92 fuster92 left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Javier Fuster <fusti92@gmail.com>
@francesco-furlan francesco-furlan merged commit 5934293 into main Feb 21, 2024
@francesco-furlan francesco-furlan deleted the chore/set-promscrape-port-when-autopilot-enabled branch February 21, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants