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

update documentation to include installation with in-cluster prometheus in openshift #1144

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

mittal-ishaan
Copy link
Contributor

Related Issue

Proposed Changes

This PR adds the documentation for installing kubecost in openshift clusters without having the bundled prometheus but using the in-cluster prometheus.

Depends upon:

opencost/opencost#2944
kubecost/cost-analyzer-helm-chart#3700

@mittal-ishaan mittal-ishaan marked this pull request as ready for review October 23, 2024 22:38
@mittal-ishaan mittal-ishaan requested a review from a team as a code owner October 23, 2024 22:38
Copy link
Contributor

@chipzoller chipzoller left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@chipzoller
Copy link
Contributor

Are all comments resolved? If so, please resolve.

@@ -42,7 +42,7 @@ Install Kubecost using OpenShift specific values. Note that the below command fe

```sh
helm upgrade --install kubecost kubecost/cost-analyzer -n kubecost --create-namespace \
-f https://raw.githubusercontent.com/kubecost/cost-analyzer-helm-chart/develop/cost-analyzer/values-openshift.yaml
-f https://raw.githubusercontent.com/kubecost/cost-analyzer-helm-chart/<$VERSION>/cost-analyzer/values-openshift.yaml
Copy link
Member

@thomasvn thomasvn Dec 17, 2024

Choose a reason for hiding this comment

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

The drawback to doing this is that now this link will no longer be checked by the CI's link-checker. What happens if we someday decide to rename or delete values-openshift.yaml?

Although referencing develop branch comes with drawbacks, I still think it's more valuable than putting <$VERSION> here. @mittal-ishaan @chipzoller thoughts?

Copy link
Contributor Author

@mittal-ishaan mittal-ishaan Dec 23, 2024

Choose a reason for hiding this comment

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

We can use develop. But we can also start with having versions. like v2.5 and then update docs when every new minor or major version gets released.

Copy link
Contributor

Choose a reason for hiding this comment

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

@thomasvn, the problem here with listing files from the develop branch is we put users in a precarious and potentially unsupported position when they install a versioned release but use a values file which doesn't correspond to that release. We really need to get away from that practice entirely when we're talking about our official docs. Rather than putting in a placeholder like <$VERSION> we should make this the same as the version referenced by the docs. For example, if users are reading version 2.5 of our docs then any files that are associated with that release should be pointed to the v2.5 branch. This obviously means that any new files that get referenced are "released" in that branch.

Co-authored-by: Thomas Nguyen <thomasn@kubecost.com>
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.

3 participants