Skip to content
This repository has been archived by the owner on Dec 21, 2019. It is now read-only.

Separate documents issue. #51

Closed
mikaelcabot opened this issue May 23, 2017 · 16 comments · Fixed by #57
Closed

Separate documents issue. #51

mikaelcabot opened this issue May 23, 2017 · 16 comments · Fixed by #57

Comments

@mikaelcabot
Copy link

kubectl apply accepts yaml documents with multiple docs without --- in head.
But without this seperator in head of each template the generated template from kontemplate becomes invalid.

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

This can become an issue when users split resources across multiple files and don't start the resources with ---.

Inside of a single file containing multiple resources I expect people to be aware of this, but a good thing to do here would be to issue a warning to users.

What I'm thinking is logic such as the following (pseudocode):

if (moreThanOneTemplateInResourceSet) {
  foreach (file in yamlFiles) {
    if (file.firstLineThatIsntAComment != "---\n" ) {
      warn("The file ${file.name} does not start with a YAML resource separator ('---')!")
    }
  }
}

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

Note that I'd prefer not to add any logic to actually parse & validate YAML itself in Kontemplate.

@mikaelcabot
Copy link
Author

Agree with not adding logic for validating the YAML.
It's a matter of how far you want to "pollute" your code I guess, but a warning like that could sure help, but then again you would have to check if that document contained more objects ---. - And if not then it would be ok if the first line didn't start with ---. Hmmmm...

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

but then again you would have to check if that document contained more objects

No, inside of a file it's up to the user to write valid YAML ;-)

@mikaelcabot
Copy link
Author

Ok, maybe it's just me then :) - I didn't know you had to start every YAML file with --- for it to be valid ...

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

You don't, but it's only Kontemplate's responsibility to warn the user if it's at the start of the file (because it's kontemplate that concatenates the files).

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

Other fix idea: What happens if kubectl is sent something like this

---
# no actual resource here
---

?

If that is valid then I can apply an actual "fix" of just prefixing each YAML file with --- when passing through to kubectl.

@mikaelcabot
Copy link
Author

How about just checking if the document contains multiple objects seperated by ---. And if so warn if the document does not start with --- ?

@mikaelcabot
Copy link
Author

I'm really not so sure anymore - as I guess this would be a validator's job ...

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

How about just checking if the document contains multiple objects seperated by ---.

This can still cause the same issue if people include multiple resource sets in one run that have one YAML file with one resource each (without ---).

In fact that makes this a bug.

@tazjin tazjin added the bug label May 23, 2017
@mikaelcabot
Copy link
Author

heh, good that got cleared out then 👍

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

Yep, thanks for noticing!

@tazjin
Copy link
Owner

tazjin commented May 23, 2017

Multiple object separators seem to work without any issues:

vincent@urdhva ~ % cat | kubectl create -f -
---
---
apiVersion: v1
kind: Namespace
metadata:
  name: yaml-test
namespace "yaml-test" created

@mikaelcabot
Copy link
Author

Then that seems to be the easiest fix

tazjin added a commit that referenced this issue May 23, 2017
In cases where users have multiple resource sets, or a single resource
set that contains multiple YAML files they may run into issues if they
forget to start their resource files with `---`.

This fix inserts an additional `---` at the beginning of each YAML
template.

In cases where the user started their file YAML file "properly" this
will result in an empty resource being templated / passed to kubectl,
however that does not seem to cause any issues.

This fixes #51
@tazjin
Copy link
Owner

tazjin commented Jun 1, 2017

I've decided agains the "easy" fix.

@heavenlyhash suggested that kontemplate should potentially execute kubectl multiple times (once per resource set), this way the resource sets internally have to be consistent but including multiple resource sets won't conflict if the user leaves out the ---.

That's enough to fix the specific bug I have and in addition error messages could potentially be a bit nicer!

@tazjin
Copy link
Owner

tazjin commented Jun 11, 2017

The new plan is as follows (and I hope to implement it in the next days because vacation is freeing up my time atm!):

  1. Refactor templater to introduce a new RenderedResourceSet type, this will contain the fully-rendered templates but also retain information about the enclosing resource set, e.g.:
type RenderedResource struct {
  Filename string
  Output string // rendered template
}

type RenderedResourceSet struct {
  Name string // Resource set name/path (for output)
  Resources []RenderedResource
}

The kubectl-calling code will then be refactored to execute kubectl once per resource set, while providing some additional logging to stderr that can be useful when debugging issues.

As a sanity check particular to the problem described in this issue I may get around to adding logic that checks if multiple YAML-files are in a single resource set and, if yes, whether they contain --- somewhere. If not a warning will be issued, but kontemplate keeps doing its thing anyways.

tazjin added a commit that referenced this issue Jun 11, 2017
As a first step in resolving #51 this refactors the `templater`
package to return rendered resource sets as a distinct type.

This also fixes #56
tazjin added a commit that referenced this issue Jun 11, 2017
Instead of passing the rendered output of all resource sets to kubectl
simultaneously, build upon the previous commit and pass resource sets
individually to new instances of kubectl.

This resolves #51
tazjin added a commit that referenced this issue Jun 11, 2017
As a first step in resolving #51 this refactors the `templater`
package to return rendered resource sets as a distinct type.

This also fixes #56
tazjin added a commit that referenced this issue Jun 11, 2017
Instead of passing the rendered output of all resource sets to kubectl
simultaneously, build upon the previous commit and pass resource sets
individually to new instances of kubectl.

This resolves #51
@tazjin tazjin added this to the 1.1.0 milestone Jun 11, 2017
@tazjin tazjin added this to the 1.1.0 milestone Jun 11, 2017
tazjin added a commit that referenced this issue Jun 11, 2017
This release features some cleanup and under-the-hood changes, as well
as "ecosystem-features" that don't directly affect the way Kontemplate
itself functions.

* Resource-sets are now passed on to kubectl in individual
  invocations. This means that kubectl errors can be scoped to
  individual resource set files and issues such as #51 are less of a
  problem.
* A Dockerfile is provided and published at `tazjin:kontemplate` on
  Docker Hub. This image contains `kontemplate`, `kubectl` and `pass`
  and can be used - for example - as an image for a step in a CI
  system.
* Kontemplate is now available on Homebrew, check the README for
  installation instructions.

* If different resource sets don't contain `---` separators in YAML,
  `kubectl` calls will no longer fail. (#51)
* Autocompleted trailing slashes in shells are now filtered from
  include & exclude lists to enhance the CLI experience slightly.
tazjin added a commit that referenced this issue Jun 11, 2017
This release features some cleanup and under-the-hood changes, as well
as "ecosystem-features" that don't directly affect the way Kontemplate
itself functions.

* Resource-sets are now passed on to kubectl in individual
  invocations. This means that kubectl errors can be scoped to
  individual resource set files and issues such as #51 are less of a
  problem.
* A Dockerfile is provided and published at `tazjin:kontemplate` on
  Docker Hub. This image contains `kontemplate`, `kubectl` and `pass`
  and can be used - for example - as an image for a step in a CI
  system.
* Kontemplate is now available on Homebrew, check the README for
  installation instructions.

* If different resource sets don't contain `---` separators in YAML,
  `kubectl` calls will no longer fail. (#51)
* Autocompleted trailing slashes in shells are now filtered from
  include & exclude lists to enhance the CLI experience slightly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants