-
Notifications
You must be signed in to change notification settings - Fork 22
Breakout charts #251
base: main
Are you sure you want to change the base?
Breakout charts #251
Conversation
This patch sets the spire chart tests to always run. This enables changes in tests to be tested and sets a base for split out charts. Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
…rk it will take. Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>
ad062a4
to
dc5f623
Compare
@kfox1111 @faisal-memon @marcofranssen Please open an Issue to make this testing accessible from the developer environment without the use of our Merge Request system, and then rework the merge request system to chain its testing into the developer environment testing approach. Consider this a high-priority request. We are attempting to reduce the number of open MRs, so we can regain velocity of accepting merge requests instead of velocity in context switching between them. |
I am in favor of doing this. i think it will be a nice for users to have the option to deploy the components individually. |
I think we are all in favor to do this, but we also discussed and agreed to postpone it till the major refactors and new features are added, it will complicate the release process which will give us additional load. IMHO park this PR and pick it up at a later stage once the other big changes are in. Probably that is the reason @kfox1111 also left it as a draft PR just to prototype the feasibility at this stage. What we need to do first is refactor the test structure to support multiple top level charts in CI. Also there that is easier if all the other changes like nesting etc are in to not get into to much merge conlficts and keeping branches in sync. |
Yeah. Mostly it was to see how much work it might take. It for sure is very disruptive to existing pr's. Most of that caused just by file renames. After doing it as a test, almost all the changes are adding of things to the existing charts such as Chart.yaml metadata to make the linter happy, adding a missing
Sounds like the plan for it is not until at least 0.9.x. So kind of parked already.
Yup. After doing it, the PR is feasible as is if the merge issues are fixed. But the timing is probably not right.
Yeah. I think thats why its out at least to 0.9.x. Currently, the tests target just charts/spire so it will work with other charts in the charts/ directory, they just wont be tested directly. But, if those charts are included in charts/spire as a dependency, which they all should be, which will cause them all to be tested that way. So maybe its not so bad. They all are still being tested, just not individually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've spent a few days thinking about this.
Helm Charts is a package manager to ensure that a collection of microservices, called an Application, is deployed in a consistent, supported way.
If we are to support helm charts for items considered less than a complete application, then we are playing with redefining Helm's core tenets, saying that independent components of an Application are "applications".
For this reason I would highly suggest that we not perform this kind of splitting, and if we do, that we do so in different top-level offerings, outside of spire/charts.
Breaking up subcharts is commonly done with helm charts and IMO isn't against the spirit of helm. For example, I use this chart regularly: You can get a very nice integrated with your k8s cluster chart by installing that. or you can install the individual pieces you need as standalone charts if the unified one doesn't work for you. It has allowed the unified chart workers to focus on making the user experience the best for their particular need, while not excluding all of the advanced ways individuals may need to assemble the pieces themselves. |
I agree with @kfox1111. Users can choose to deploy the whole stack in a single chart, as individual components, or a hybrid of both. |
Discussed on sync meeting today. @kfox1111 to break down this PR into some smaller components to make it easier to review. |
As per previous discussions, this pr breaks the child charts into standalone charts.
This was requested by @bramaq and others.
fixes: #235