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 clsuterName to support clusters with non-default names #236

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xirehat
Copy link

@xirehat xirehat commented Nov 5, 2024

By adding clusterName to values, you can install OpenCost in clusters with names other than cluster.local and connect to Prometheus correctly.

This fixes #235

@xirehat
Copy link
Author

xirehat commented Nov 12, 2024

@mattray @toscott @brito-rafa

@AjayTripathy
Copy link
Collaborator

Thanks for this PR @xirehat ... can you share a bit more on how this was tested, and regression tested (ie that it still works for clusters using svc.local)?

@xirehat
Copy link
Author

xirehat commented Nov 15, 2024

When I run helm template opencost -f values.yaml ./ with empty clusterName: "", output shows me this:

...
          env:
            - name: PROMETHEUS_SERVER_ENDPOINT
              value: "http://prometheus-server.prometheus-system.svc.cluster.local:80"
...

When I run helm template opencost -f values.yaml ./ with clusterName: "example.production", output shows me this:

...
          env:
            - name: PROMETHEUS_SERVER_ENDPOINT
              value: "http://prometheus-server.prometheus-system.svc.example.production:80"
...

It works properly. @AjayTripathy

Copy link

This pull request has been marked as stale because it has been open for 60 days with no activity. Please remove the stale label or comment or this pull request will be closed in 5 days.

@xirehat
Copy link
Author

xirehat commented Jan 15, 2025

@mattray @toscott @brito-rafa
Why don't you check this PR? Has this project been abandoned?

@github-actions github-actions bot removed the Stale label Jan 16, 2025
@toscott
Copy link
Collaborator

toscott commented Jan 16, 2025

Hello @xirehat. This project has not been abandoned, but it has not been a focus area for me personally in quite a while.

For your issue, I believe a non obvious workaround to achieve your goal without helm chart changes would be to set:

opencost:
  prometheus:
    external
      enabled: true 
      url: http://prometheus-server.prometheus-system.svc.example.production:80

If you find that too confusing, I'm ok with supporting a cluster domain name override, but I would like to see the default for the value in the values.yaml file instead of embedded in the _helpers.tpl (makes it a bit easier for people to understand what its for when they can see the value.)

If you do choose to proceed with the change, please rebase your pull request and increment the version. I can put aside some time this weekend to review outstanding prs.

Thanks!

@xirehat
Copy link
Author

xirehat commented Jan 16, 2025

If you find that too confusing, I'm ok with supporting a cluster domain name override, but I would like to see the default for the value in the values.yaml file instead of embedded in the _helpers.tpl (makes it a bit easier for people to understand what its for when they can see the value.)

@toscott I accept your advice. The changes have been applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants