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

[Feature] Find a solution to reduce size of the chart #8162

Closed
2 tasks done
eddycharly opened this issue Aug 29, 2023 · 19 comments · Fixed by #8623
Closed
2 tasks done

[Feature] Find a solution to reduce size of the chart #8162

eddycharly opened this issue Aug 29, 2023 · 19 comments · Fixed by #8623
Assignees
Labels
enhancement New feature or request helm Issues dealing with the Helm chart release-high High issues which SHOULD be addressed in the specified milestone. These may get bumped.

Comments

@eddycharly
Copy link
Member

Problem Statement

Helm chart is growing beyond the size helm can manage.
We need to find a solution to the release size under control.

Xref #8130 (comment)

Solution Description

Let's throw ideas on how we can do that.

Alternatives

No response

Additional Context

No response

Slack discussion

https://kubernetes.slack.com/archives/C032MM2CH7X/p1693306174401429

Research

  • I have read and followed the documentation AND the troubleshooting guide.
  • I have searched other issues in this repository and mine is not recorded.
@eddycharly eddycharly added enhancement New feature or request triage Default label assigned to all new issues indicating label curation is needed to fully organize. helm Issues dealing with the Helm chart release-high High issues which SHOULD be addressed in the specified milestone. These may get bumped. and removed triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Aug 29, 2023
@chipzoller chipzoller added this to the Kyverno Release 1.11.0 milestone Aug 29, 2023
@MarcelMue
Copy link
Collaborator

From the discussion in the Weekly:

  • We want to initially see if there are some quick wins for 1.11
  • We want to also understand which long term solutions are possible (maybe inclusion of CRD apply in the controller)

@chipzoller
Copy link
Member

Also need an analysis of how close we actually are here and how much headroom we have.

@JimBugwadia
Copy link
Member

As a short term solution, can we strip comments / help on all non-default versions?

@MarcelMue
Copy link
Collaborator

We should have enough headroom in the near future with the removal of descriptions in #8185

@eddycharly
Copy link
Member Author

IMO we should try to reduce more:

Helm secret size (1015880 bytes) is below the max allowed (1030000 bytes)

@eddycharly
Copy link
Member Author

Side note prometheus-community/helm-charts#3548

@MarcelMue
Copy link
Collaborator

Side note prometheus-community/helm-charts#3548

This is also the approach I mentioned in the call initially, but we need to remove/deal (possibly with globals) with any rendering of names / labels we have in the CRDs ideally. Subcharts are supposed to be able to be standalone in helm, so you can't just "import" values from the parent chart. If we only had plain-text CRDs, then this would be the trivial solution.

-> helm docs: https://helm.sh/docs/chart_template_guide/subcharts_and_globals/

@eddycharly
Copy link
Member Author

Isn't the solution related to using the special crds folder ?
Last time i checked subcharts contributed the same data size to the secret as direct templates.

@MarcelMue
Copy link
Collaborator

Isn't the solution related to using the special crds folder ? Last time i checked subcharts contributed the same data size to the secret as direct templates.

No - if you check the PR, it is moved from the crds subfolder to a subchart called crds and then there into the crds folder: https://github.com/prometheus-community/helm-charts/pull/3547/files

You can also find the chart.yaml of the subchart here: https://github.com/prometheus-community/helm-charts/blob/main/charts/kube-prometheus-stack/charts/crds/Chart.yaml

@eddycharly
Copy link
Member Author

But when you install the kube-prometheus-stack chart, the manifests from grafana for example are part of the secret, it's a subchart though.

@tmsdce
Copy link

tmsdce commented Sep 11, 2023

Hi,

Maybe it can help others. On our side we use ArgoCD and the CRD size issue went away thanks to SSA with the use of a specific annotation applied on CRD objects argocd.argoproj.io/sync-options: ServerSideApply=true (I think SSA is also supported on Flux).

Also, as ArgoCD uses helm template, this "trick" is only possible because the CRDs are part of the chart templates through the .Values.crds.annotations (it would also work in a dedicated chart or subchart). If they were moved to the special crds folder however, this wouldn't work as this folder is not templated by helm.

The key was to use SSA and it is quite straightforward to leverage it when using gitops operators, but may be more cumbersome to handle without these tools.

@MariamFahmy98
Copy link
Collaborator

I believe we can create a separate helm chart for the CRDs.

@chipzoller
Copy link
Member

I really think the most sustainable thing going forward is to handle CRD creates/updates via an initContainer. It's more initial work but we can do whatever we want with total freedom and it makes this issue go away.

@tobru
Copy link
Contributor

tobru commented Sep 24, 2023

Handling CRDs directly with the controller or in the Pod has one big drawback: You'd need elevated privileges to manage CRDs, it makes RBAC painful.

@chipzoller
Copy link
Member

Yes, but you can use specific resources names because they're your CRDs.

@MariamFahmy98
Copy link
Collaborator

MariamFahmy98 commented Oct 10, 2023

This is how prometheus handles the CRDs through helm charts:

  1. kube-prometheus-stack has a subchart for crds (can be found here).
    I installed it to check if the CRDs are a part of the helm secret and it is not.
  2. A separate helm chart for CRDs prometheus-operator-crds.

@eddycharly - I amn't sure why grafana manifests exist in the helm secret whereas CRDs don't.

This is the dependencies of the kube-prometheus-stack. You will see that there's a separate helm chart for grafana and they include it in the dependencies of the chart, however for CRDs, they use the crds subchart they have created instead of using the prometheus-operator-crds i.e. the separate chart for CRDs. I wonder if this is why crds don't exist in secret templates whereas the grafana does exist.

By default, the chart installs the CRDs.

@MariamFahmy98
Copy link
Collaborator

This is how I checked if CRDs templates exist in the helm secret:

kubectl get secrets <helm-secret-name> -o jsonpath="{ .data.release }" | base64 -d | base64 -d | gunzip -c | jq '.chart.templates[].data' | tr -d '"' | base64 -d >> templates.o

Then search for CustomResourceDefinition in templates.o, and there is none.

Let me know if it isn't the correct approach.

@treydock
Copy link
Member

CRDs in a subchart is how Kyverno used to ship the CRDs and it caused some issues for the community since you'd have to install the CRD chart before the Kyverno chart, so Kyverno Helm install was at minimum a 2 step process rather than 1 step. Some ways to reduce size would move many/all of the helpers to a common chart and then maybe move some of the various Kyverno components in the current chart into subcharts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request helm Issues dealing with the Helm chart release-high High issues which SHOULD be addressed in the specified milestone. These may get bumped.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants