Conversation
1fdde8d to
a03d6b2
Compare
|
There's feedback on #8841 that needs to be taken into account here. |
@mattfarina Can you please address what feedback? |
|
@mattfarina Btw, I thought about it more and I think I should revert this commit: d1f9bcb My initial thought was that I want to be able to access the file in the chart by its path and not by an alias. What do you think about this? |
|
@itaispiegel Using a colon as the separator won't work on Windows. Helm is used on Windows, too. As for the idea of having a key/value pair, that requires the Helm user (chart user) to know about the internal workings of the chart. For many that is not the case. The consumer of a package (just like someone using apt to install mariadb) doesn't usually know the app that's being installed or how it works. I'm not sure the UX is a great experience for that setup. Am I missing something? |
I'm not sure this is an issue. For example, you can see that in Docker they use a colon as a delimiter for the
How is this different than passing values to a chart during the installation? For example, if I want to install the MariaDB chart, I'd need to go over the README file, understand the internal values, and set them accordingly. That's also why I think that changing the behavior to fail if the file isn't included (instead of considering it empty) is a necessary behavior. Because then if you include the wrong file, the chart will fail to install, instead of it being installed in a weird state. I don't think though that it should block this PR. |
|
Hey guys, I'm going on vacation so I won't be able to continue working on this until I come back in October. |
I believe @mattfarina is referring to these comments: #8841 (review) We discussed them in a developer call, but I don't believe there are any concrete test cases covering this scenario with the |
|
Wait. Why was #3276 (comment) never discussed filepath globbing. Just There are a significant number of changes introduced here compared to the originally agreed upon proposal in #3276 (comment). We'll have to restart the entire review process from square one. |
|
Hey everyone, just wanted to let you know that I came back from my vacation, but just started some new stuff so I'm quite busy. |
|
You need not forget about |
|
What about named aliases for each individual directory? With this approach, the chart does not have to rely on absolute paths. And when some existing release is updated, it will be possible to individually replace the named directory. It's kind of like a docker mount named volumes only in the opposite direction |
I like this idea. I think this decouples how the chart was written with how the files can be supplied to the chart. However, I don't think globbing files will be possible anymore with this approach. I'm keen to help out, so let me know if there's anything I can help 😄 |
|
Hi everyone, I would like to contribute to this feature. My plan would be to:
example with multiple Also I thought about not only including files from file directories but also maybe github repostiories. Does it make sense to incorporate such a functionality as part of this PR, to be able to include files from github repositories? |
|
Be my guest :) |
|
To give more context :) From my experience working on different enterprise projects, it is common to have different gh repositories for application and ops. But its just a proposition :) |
|
@joejulian that's insightful thank you! But I think fundamentally helm would be useless without the ability to template. Also most package installs do allow several flags to alter the way it is used. And lastly, linux package managers are fundamentally different than helm install. Once those are installed, users are intended to change user configuration files, but with kubernetes and helm that should be done at the time of install, not at a later step. I could be wrong, but that's how I see it anyways. My team uses helm extensively for the ability to deploy to different environments the same packages. If we took out the ability to change the configuration per environment, it's just a bunch of yaml files. So I think at the core, this is what helm was meant to solve. |
|
I think it's most important to think about the core interface with Helm. Right now it's values.yaml and chart in, and on the other side you get a set of Kubernetes resources. To change the contract to be values.yaml and another set of files, I think would be a mistake. Especially when you start to think about things like Argo and Flux. I would like to understand the use cases more and figure out how we could still keep the contract of helm as it is, while still accommodating whatever use cases are applicable here. Why would tooling to help people put files and directories into the values structure not be sufficient? |
|
Honestly the comparison here to We do this all the time manually with Not being able to pass in the most common method of configuration (files..... pg_config, myTemplate.pug, nginx.conf) in a coherent way from local / repository filesystems is crazy to me.
Have reviewed the various workarounds including...
... and while functional these are all tremendously silly ways of solving the problem. That people are going to this degree of workaround just to get a conf file into a |
|
@zfalen-deloitte if your chart supports passing configuration files via values, I guess the |
|
@yann-soubeyrand I am hoping for this PR to be accepted. My use case is that I'm creating containers for EPICS control systems. I create 1 generic container per type of device I want to control. I provide a YAML file to configure the specific instance of the device. If that were the whole story then However, I also wish to support other EPICS users who might not want to use my YAML based approach to generating the required startup files. I therefore allow arbitrary files to be included for configuration by other more traditional methods which require a handful of files to be supplied. I don't know up front what these will be. I'm currently solving this by making the helm chart on the fly before each deployment and it's not that pretty. Maybe this sounds like bad design but I'm constrained by the 1980's heritage of the control system that I'm trying to make work in Kubernetes. Most of the benefits of doing that still apply even though I'm stuck with some antique configuration files. |
|
@yann-soubeyrand I stand by my statement. This level of configuration is a tremendously silly way to solve for a simple need. Let alone supporting an arbitrary number of files. Arguments exist for all ways of doing this. Current model forces extra complexity and boilerplate where there is no need. A huge variety of use cases exist for a values-only model. Quite a few also exist that benefit from mounting non-YAML files. Where configuration specificity is high, model A makes sense. I want downstream users to avoid headaches by providing simple YAML config options to drive setup. Where generality is high, or where a long-existing configuration model is present, model B makes more sense - it is faster to develop, more grokkable (in some cases), and lets me ship more flexibly. We're just adding noise to the equation by needing to reinvent the thing in YAML and reverse engineer them back into files 🤷🏼♂️ |
|
So... it's 2024, is there any chance that this can be merged? I am still doing the workaround of copying the configs into the chart folder. |
|
I also am still using the same ugly workaround. |
|
well, well. 4 years, my child is 4 years old. but I'm still waiting for this feature. |
|
@nooperpudd 4 years? |
|
Well, it's mid-2024! :( |
|
It sure feels like every time I come across a problem with modern open source the problem was either debated until everyone gave up and moved on (from the problem or some from open source at all), or some brave soul was tenacious to work a solution and then it never gets merged because ????? This managed to be both 😭 😭 😭 I understand maintainers and sponsors and so forth have their visions and perspectives and disagreements. But this sure feels disrespectful to everyone involved - is there some way to find consensus for these type of things or is the solution to just not become attached to a thing? It's so confusing. Back to tilting at windmills. |
|
I also wish the dialogue from maintainers was more open-minded. But given that I’ve accepted they are not (and likely won’t be), we had to design a solution around that. What we did was create a new schema and a Helm library function to template out our common use cases. We call it Helmix, our own mixture of Helm. It’s written entirely in text/template (ouch!) The main idea is that all configuration files you want to be easily override-able must now be in YAML format. This isn’t ideal, but it makes it possible to automatically modify configuration without reinventing the wheel each time. Here’s an example that templates a simple configurable configmap from files on disk: secrets/foobar.yaml helmix.yaml We have a lot of defaults defined, but the entire ConfigMap is fully configurable via our schema. The goal is to keep only the most important information visible in the YAML. This is specifically designed for Kubernetes objects. The project isn’t open source, but the idea should be straightforward for others to re-implement in their own way. If you find this approach useful, let me know, I’d love to hear if others have alternative or better solutions. |
|
This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs. |
|
Vai fechar não bot, pode deixar aberto isso ai. |
|
Vai ficar aberto até resolver |
|
This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs. |
|
Vai ficar aberto até resolver |
|
This pull request has been marked as stale because it has been open for 90 days with no activity. This pull request will be automatically closed in 30 days if no further activity occurs. |
|
Vai ficar aberto até resolver, não vou deixar fechar |
What this PR does / why we need it:
This PR adds the flag
--include-pathwhich allows you to include external files in the install, upgrade and template commands.Example
Given the configmap:
And the directory structure:
Running
helm template test_chart --include-path /tmp/externalswill generate the manifest:If the
--include-pathis not passed, the output will be:Special notes for your reviewer:
This PR continues the work that @hunsche started in his PR: #8841.
I tried contacting him to discuss his work, but unfortunately, I couldn't reach him so I implemented this myself after discussing this at the Helm dev call which took place on August 19th.
This is my first contribution to Helm, and my first time working with Go. I'd be glad to get feedback for this regarding code style, best practices, and design.
close #3276
close #8227
Related documentation PR: helm/helm-www#1180
Thanks in advance! :)