search

rancher / charts-build-scripts

Apache License 2.0
9 stars 32 forks source link

Support pulling crds from an upstream #129

Closed joshmeranda closed 5 months ago

joshmeranda commented 5 months ago

Problem

While updating the upstream for rancher-monitoring chart, I found that the crd generation code is unable to work with the structure of kube-prometheus-stack. The charts-build-scripts expect crds to be stored in a crds directory at the chart root directory; however, kube-prometheus-stack has moved its crds into a subchart. This causes several problems for crd generation:

  1. Since we clear the charts directory when loading dependencies, the upstream crd subchart is removed before the crd is generated, so we cannot simply support a user specified location to look for crds over the default crds.
  2. The upstream crd subchart is poorly defined so that the version is 0.0.0 and does not match the real upstream version 57.0.3, so we cannot simply manage a rancher-monitoring-crd since the rancher-monitoring and rancher-monitoring-crd charts will not have matching versions.
  3. The current additionalCharts configuration validation code does not allow for specifying both upstreamOptions and crdOptions so we cannot pull from an upstream and then use the crd template.

Solution

Allow for specifying both upstreamOptions to pull the crds directory, and crdOptions to use the crd template.

fixes #13 unblocks #44614

joshmeranda commented 5 months ago

@adamkpickering I might be misunderstanding you but you are correct that if c.CRDChartOptions != nil we won't enter that next else branch to pull the charts. But it will enter the new nested if c.Upstream != nil check to pull down the crds. This works as expected for rancher-logging (no crd upstream) and rancher-monitoring (with crd upstream after rebase)

adamkpickering commented 5 months ago

Ah you're right, I'm not sure how I missed that. Moving too fast I guess. The code looks good to me then.

We do need to update the documentation and comments though - there are places that say that crdOptions is mutually exclusive with upstreamOptions. Specifically templates/template/docs/packages.md and pkg/options/additionalchart.go.

There is duplication of this documentation in rancher/charts due to the templating. Don't worry about this - I will bring this up internal to the MAPPS team, and we'll have to open another PR or make other changes to address this problem.

joshmeranda commented 5 months ago

Ope I forgot about the code docs. I'll go through and update what I can in the code. Thanks!