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

[agent-operator] Move crds(agent, prometheus) into separate subcharts #2781

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

Conversation

kuzm1ch
Copy link

@kuzm1ch kuzm1ch commented Nov 11, 2023

Fix: grafana/loki#7488

Prometheus is installed using another chart (or any other method). The Prometheus CRDs are provided by this installation method. Installing also the CRD from the loki chart may produce CRD upgrade and downgrade with possible data lost.

These changes allow users to disable installation of specific CRD groups so users can avoid the situation when multiple helm charts manage/install the same CRDs.

Also, moving CRDs into subchart has some other benefits - prometheus-community/helm-charts#3548.

@kuzm1ch kuzm1ch requested a review from a team as a code owner November 11, 2023 21:23
@CLAassistant
Copy link

CLAassistant commented Nov 11, 2023

CLA assistant check
All committers have signed the CLA.

@Vinaum8
Copy link

Vinaum8 commented Feb 27, 2024

man, i need this!

@Vinaum8
Copy link

Vinaum8 commented Feb 27, 2024

@kuzm1ch i think that you need mark the code owner for this code for PR is to review.

CODE OWNERS - Grafana Helm Charts

@kuzm1ch
Copy link
Author

kuzm1ch commented Feb 27, 2024

@Vinaum8 agent-operator is not in a list in the https://github.com/grafana/helm-charts/blob/main/.github/CODEOWNERS, so I'll tag @grafana/helm-charts-admins.

If this simple PR is ok in general for @grafana/helm-charts-admins, I will fix conflicts and it can be merged.

@kuzm1ch
Copy link
Author

kuzm1ch commented Feb 27, 2024

@maorfr @torstenwalter @Xtigyro @zanhsieh
could you take a look

@zanhsieh zanhsieh changed the title Move crds(agent, prometheus) into separate subcharts [agent-operator] Move crds(agent, prometheus) into separate subcharts Mar 2, 2024
Copy link
Collaborator

@zanhsieh zanhsieh left a comment

Choose a reason for hiding this comment

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

@kuzm1ch
Can you fix the conflicts please?

@kuzm1ch
Copy link
Author

kuzm1ch commented Mar 5, 2024

@kuzm1ch Can you fix the conflicts please?

done

@Vinaum8
Copy link

Vinaum8 commented Mar 21, 2024

@zanhsieh still need something in this PR?

@zanhsieh
Copy link
Collaborator

@Vinaum8 Grafana repo required two approvals in order to merge, so nothing I can do now.

@Vinaum8
Copy link

Vinaum8 commented Mar 31, 2024

@Vinaum8 Grafana repo required two approvals in order to merge, so nothing I can do now.

Thanks man!

@bobertrublik
Copy link

It would be awesome if we could merge this. We're managing CRDs separately from the Helm chart deployment and have issues integrating this chart.

@Vinaum8
Copy link

Vinaum8 commented Apr 4, 2024

It would be awesome if we could merge this. We're managing CRDs separately from the Helm chart deployment and have issues integrating this chart.

me too.

i waiting for this merge.

@kuzm1ch kuzm1ch force-pushed the feat/crds_subchart branch 3 times, most recently from dec583e to de3fc9e Compare April 6, 2024 18:58
@kuzm1ch
Copy link
Author

kuzm1ch commented Apr 6, 2024

PR updates:

  • cherry-pick my commits on top of main to ensure sign-off are existed for all of them
  • updated docs based on changes
  • added empty values to ensure that lint action is passed
  • bump version. Probably I am not the right person to do it, but this PR doesn't bring breaking changes or major features , just ability to disable some of crds related to another project. Default behavior is the same -> all CRDs are installed.

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.

helm chart should not install Prometheus CRDs
5 participants