carvel-dev / kapp-controller

Continuous delivery and package management for Kubernetes.
https://carvel.dev/kapp-controller
Apache License 2.0
267 stars 102 forks source link

optional paths in template steps. #60

Open voor opened 3 years ago

voor commented 3 years ago

Describe the problem/challenge you have Right now every path under ytt or similar template steps in the App CR is required, and the App CR will fail if it's not present:

  template:
    error: 'Templating dir: exit status 1'
    exitCode: 1
    stderr: |
      ytt: Error: Checking file '/etc/kappctrl-mem-tmp/kapp-controller-fetch-template-deploy695734710/config/aws': lstat /etc/kappctrl-mem-tmp/kapp-controller-fetch-template-deploy695734710/config/aws: no such file or directory
    updatedAt: "2020-12-14T16:27:39Z"

Describe the solution you'd like We have a standardized way we write out App CRs, providing an easy to override mechanism for different environments. Not all App CRs need that overriding capability, but having the folders there reduces complexity. If there was a way to say "this path is optional, don't fail if it's not there, just proceed without it" that would reduce the need to populate a bunch of folders with a .gitkeep file or similar.

Anything else you would like to add: Not a high priority, as almost 99% of our apps do need some type of configuration per environment, just wanted to track it for posterity.

cppforlife commented 3 years ago

my main concern here would be accidental mistakes that do not get caught early on. not sure how to best address that concern 🤔

voor commented 3 years ago

Maybe it's just a completely new option optionalPaths or similar with the warning that if they're not there things will just proceed. :shrug:

cppforlife commented 3 years ago

Maybe it's just a completely new option optionalPaths

gets a little funky since ytt is path-order sensitive. or should paths and optionalPaths be mutually exclusive -- that also feels a little weird. or may be optionalPaths contains paths that also need to be present in paths key...

this also raises an interesting question of who should check this optionality: is it ytt's job or is it kapp-controllers?

voor commented 3 years ago

Maybe it's just a completely new option optionalPaths

gets a little funky since ytt is path-order sensitive. or should paths and optionalPaths be mutually exclusive -- that also feels a little weird. or may be optionalPaths contains paths that also need to be present in paths key...

this also raises an interesting question of who should check this optionality: is it ytt's job or is it kapp-controllers?

Yeah, I imagine kapp-controller would check for the path, and either include or not include it into the resulting ytt call.

anwarchk commented 3 years ago

Would this be first MR candidate @cppforlife Happy to submit a MR.

cppforlife commented 3 years ago

@anwarchk i think before implementation can come we need to figure out what is the interface for this. how do we deal with existing paths? should we add another key like extendedPaths that is mutually exclusive to paths and allows to specify properties like optional: true?

voor commented 2 years ago

My concern is that there's already a lot of ways to define paths in the App CR for ytt, so adding more complexity here is going to increase the level of maintenance and have a lot more ways to do the same thing. Ultimately, we just don't want ytt to crash when a directory isn't present -- if the directory does exist and it's empty, then ytt wouldn't complain. Would it be simpler to have some sort of templating step before ytt that just creates empty directories so ytt doesn't complain?

voor commented 2 years ago

Bumping this, since it's still a relevant feature.

joe-kimmel-vmw commented 2 years ago

Thanks @voor we will bring this up at our next team meeting