support partials #207

skaes · 2017-11-12T15:37:36Z

We have a lot of repetition in our YAML files, mostly due to cronjobs.

I attacked the problem by adding partials. Mind you, this a proposal, which is
why I haven't added documentation yet, but there are tests.

The implemented partials system is very simplistic, although it looks
a bit like Rails templating. There's only one function supported, called
"partial", which takes two arguments: the name of the partial and an
argument hash. Partials are expected to be placed in a "partials"
subdirectory of the template directory and must have a '.yaml.erb'
suffix. Keys in the arguments hash are converted into local variables
for the partial. They are also accessible via the argument hash which is
passed in as variable 'locals'.

KnVerey · 2017-12-19T23:06:54Z

Hey @skaes ! I discussed this feature with my team, and we decided it is something we'd like to accept. In addition to the use cases you've shown in your tests, I think it would be great to be able to embed a common pod spec into multiple deployments. Before I look at the details, I'd like to request that the rendering functionality be extracted into its own class now that the logic required is growing. This is also going to be necessary in #228, so please coordinate with @maxlaverse (who isn't at Shopify either--it's great to have all these contributions! ❤️ ) about who will have time to do that piece first.

skaes · 2017-12-20T09:02:09Z

@KnVerey Maxime and myself are working both at XING. He's in my team. We're using kubernetes-deploy in building an internal PaaS. Which explains the many pull requests you've seen.

skaes · 2017-12-20T15:34:46Z

@KnVerey also, thrilled you like the idea. Will refactor tomorrow.

skaes · 2018-01-18T18:29:43Z

Can we make some progress on this pull request? I would like complete this one, before integrating #228.

KnVerey

I don't think this currently handles indentation correctly. If you can get that working with the current syntax, great! Otherwise something like partial 'my_partial', locals: { var1: 'foo' }, indent: 5 would work for me too.

Please leave a comment saying that the code is ready for re-review once it's updated. 😄

KnVerey · 2018-01-18T17:04:10Z

README.md


+`kubernetes-deploy` supports composing templates from so called partials in order to reduce duplication in Kubernetes YAML files. Given a template directory `DIR`, partials are searched for in `DIR/partials`and in 'DIR/../partials', in that order. They can be embedded in other ERB templates using the helper method `partial`. For example, let's assume an application needs a number of different CronJob resources, on could place a template called `cron`in one of those directories and then use it in the main deployment.yaml.erb like so:


typo: "on could place" -> "one could place"

KnVerey · 2018-01-18T17:04:54Z

README.md

+<%= partial "cron", name: "send-mail", schedule: "0 0 * * *", args: %w(send-mails), cpu: "200m", memory: "256Mi" %>
+```
+
+Inside a partial, the parameters can be accesses as normal variables, or via a hash called `locals`. Thus, the `cron` template could like this:


typo: "can be accesses" -> "can be accessed"

KnVerey · 2018-01-18T17:10:35Z

README.md

+          containers:
+          - name: cron-<%= name %>
+            image: ...
+            args: [ <%= args.map{|a| %Q("#{a}")}.join(", ") %> ]


I know this is just an example, but why all the transformations? This seems to generate [""cleanup""], vs. ["cleanup"] from just <%= args %> . Why is that desirable?

It was extracted from a more complex example, where part of the args entry was already present.

KnVerey · 2018-01-18T17:11:35Z

README.md

+          restartPolicy: OnFailure
+```
+
+Note that partial names must have the suffix `.yaml.erb`.


We should support .yml.erb too... we originally supported only one in the main renderer, and it led to some confusing debugging for a few users.

KnVerey · 2018-01-18T17:17:03Z

lib/kubernetes-deploy/deploy_task.rb

-    rescue NameError => e
-      @logger.summary.add_paragraph("Error from renderer:\n  #{e.message.tr("\n", ' ')}")
-      raise FatalDeploymentError, "Template '#{filename}' cannot be rendered"
+      @renderer.render_template(filename, raw_template)


Let's inline this since render_template is only called once.

fine with me

KnVerey · 2018-01-18T17:27:51Z

test/integration/kubernetes_deploy_test.rb

+      bindings: { "cronjob_api_version" => api_version },
+      sha: nil
+    )
+    assert_deploy_success(result)


We should also include some assertions that expected resources were created. With malformed yaml documents (not saying your new ones are malformed now, but we could mess them up in the future), it is possible to have a successful deploy that simply never tried to render some of the expected templates.

KnVerey · 2018-01-18T17:30:23Z

test/fixtures/test-partials/deployment.yaml.erb

@@ -0,0 +1,20 @@
+---
+apiVersion: extensions/v1beta1
+kind: Deployment


I'd also like to see a fixture that proves that your renderer correctly handles indentation. A common request I've heard is to be able to embed a full podspec (i.e. spec.template) from a partial into multiple deployment files. Let's do that with this deployment; otherwise, it isn't really contributing much to the test itself.

The renderer does not handle indentation. If you have an idea how to implement this feature without rewriting or monkey patching ERB, please let me know.

It would be really nice to come up with a solution for this. I will think about how to do this in a nice way for a bit as well.

When I implemented the first version of the code, I already thought about this problem. I couldn't think of any other way to support indenting other than adding an explicit indentation parameter to the partial method, just as @KnVerey suggested, or writing a renderer. I rejected the first idea as being to error prone and clumsy and rejected the second idea as way too costly and not backwards compatible.

Besides, you can always write partials using YAML flow style, which does not require indentation:

{ a: 1, b: { x: 1 } }

KnVerey · 2018-01-18T19:57:53Z

lib/kubernetes-deploy/renderer.rb

+      erb_binding = binding
+      bind_template_variables(erb_binding)
+      erb_binding.eval <<~EVA, __FILE__, __LINE__ + 1
+        def partial(partial, locals = {})


Since our base binding comes from this class, I believe this method can be defined like any other--no need to eval it in or to use self. in calling methods within it.

I wanted to keep the exposed interface available inside ERB as small as possible. The eval essentially reduces it to the method "partial" and "self", whereas your proposed implementation allows direct access to all instance variables, public and private methods of class Renderer.

Of course, anything can be accessed via self as well, but it requires a conscious effort to do so.

KnVerey · 2018-01-18T19:59:25Z

lib/kubernetes-deploy/renderer.rb

+        def partial(partial, locals = {})
+          partial_binding = binding
+          self.bind_template_variables(partial_binding)
+          self.bind_template_variables(partial_binding, locals)


nit: I'd remove the default from the second arg and call it just once with template_variables.merge(locals) so it's more clear what is going on.

KnVerey · 2018-01-18T20:03:10Z

lib/kubernetes-deploy/renderer.rb

+      @id = current_sha[0...8] + "-#{SecureRandom.hex(4)}" if current_sha
+    end
+
+    def template_variables


Please make this method, bind_template_variables and find_partial private

I don't think that improves anything.

It would make us happy :)

klautcomputing · 2018-01-19T16:13:22Z

README.md

@@ -120,6 +120,44 @@ All templates must be YAML formatted. You can also use ERB. The following local

 You can add additional variables using the `--bindings=BINDINGS` option. For example, `kubernetes-deploy my-app cluster1 --bindings=color=blue,size=large` will expose `color` and `size` in your templates.

+#### Using partials
+
+`kubernetes-deploy` supports composing templates from so called partials in order to reduce duplication in Kubernetes YAML files. Given a template directory `DIR`, partials are searched for in `DIR/partials`and in 'DIR/../partials', in that order. They can be embedded in other ERB templates using the helper method `partial`. For example, let's assume an application needs a number of different CronJob resources, one could place a template called `cron`in one of those directories and then use it in the main deployment.yaml.erb like so:


Missing space: s/called cronin/called cron in/

klautcomputing · 2018-01-19T16:28:19Z

lib/kubernetes-deploy/renderer.rb

+    end
+
+    def find_partial(name)
+      partial_names = [name + '.yaml.erb', name + '.yml.erb']


I think the function could be implemented nicer with Dir.glob. Something like: Dir.glob("**/#{name}.y[a]ml")` This could would then also have an explicit way of choosing one file over another if there are two files with the same name but in different directories, instead of now implicitly using the first one it comes across.

I think you might have missed that the README specifies a search order for the directories it's looking in, in order to support scoping.

I would consider the existence of two partials which differ only in the suffix a user error. We could of course check for that condition (within one directory).

BTW: I don't think niceness is objectively measurable.

I read the readme and that's why I said that the code just implicitly reflects the that. And also wasn't talking about the suffix, but a partial in DIR/partials and DIR/../partials.

More importantly: Dir.glob removes 2 eachs, a File.join, the need to do a File.exists, as well as partial_names, so basically the whole function. And would get rid of @partials_dirs. I think that's a cleaner way of doing it, which is why I suggested it.

@klautcomputing how about you show me the implementation you want and I just copy and paste it? I really don't have time to waste on minor details.

I changed the code, please find my PR here xing#1

klautcomputing · 2018-01-19T18:26:28Z

test/fixtures/test-partials/deployment.yaml.erb

@@ -0,0 +1,20 @@
+---
+apiVersion: extensions/v1beta1
+kind: Deployment


It would be really nice to come up with a solution for this. I will think about how to do this in a nice way for a bit as well.

skaes · 2018-01-20T08:33:01Z

@KnVerey @klautcomputing ready for further review.

KnVerey · 2018-01-23T21:05:36Z

In my and @klautcomputing opinion, the ability to correctly handle indentation is an essential requirement for this feature. As I mentioned in my original comment in response to this PR, that is commonly the use case I've heard for requesting partials internally. IMO it would be considered a major bug if it did not work. You're right that this is not trivial to research and implement, and I don't know how to do it offhand either. One thing I'd definitely look into is the implementation in Rails. If you do not have the time to pursue this further, one of us will implement it branched off your work when we have the time, and then merge both pieces together. Either way, thank you for your work so far.

skaes · 2018-01-24T14:30:40Z

@KnVerey @klautcomputing We have found a way to allow partials authors to use normal YAML notation when including YAML partials at arbitrary offsets from column zero. Since JSON is a subset of YAML, we simply convert the partial to a JSON string before including it in the outer template. I sincerely hope this satisfies your requirements now.

KnVerey

Hey @skaes. I only did a partial review, since I noticed that your test doesn't actually cover your solution (I put in a debugger and it never got hit), and if I add a template that does (e.g. a partial for a pod spec or an environment section, as I mentioned before), the rendering doesn't seem to work. Please modify your fixtures to prove your solution works and re-ping if I'm wrong 😄 .

KnVerey · 2018-01-25T19:16:43Z

lib/kubernetes-deploy/renderer.rb

+      }.merge(@bindings)
+    end
+
+    def bind_template_variables(binding, variables)


Please rename the arg to erb_binding or something to avoid confusion with the binding method (this confusion is aggravated by syntax highlighting because of the special status of that method -- see L30).

Note that fact that editors highlight the variable name 'binding' is a mistake on behalf of syntax highlighters. There is nothing special about the name 'binding'. But I get your point, so I changed it.

KnVerey · 2018-01-25T19:19:29Z

lib/kubernetes-deploy/renderer.rb

+    def render_template(filename, raw_template)
+      return raw_template unless File.extname(filename) == ".erb"
+
+      binding = TemplateContext.new(self).template_binding


Same comment for this variable--please name it something other than binding to avoid confusion

KnVerey · 2018-01-25T20:39:31Z

lib/kubernetes-deploy/renderer.rb

+      bind_template_variables(binding, template_variables)
+
+      src = ERB.new(raw_template).result(binding)
+      if src =~ /^--- *\n/m


It isn't safe to assume that multi-doc templates (or single-doc templates that have a header) do not contain partials.

Please note that src is actually YAML code that already went though the ERB step and therefore partials inside it have been expanded. Apparently the choice of variable name was unfortunate, I'll rename it.

Ouch. I added the code to the wrong method. Will fix.

KnVerey · 2018-01-25T22:08:00Z

test/integration/kubernetes_deploy_test.rb

@@ -802,6 +802,23 @@ def test_ejson_secrets_respects_no_prune_flag
    ejson_cloud.assert_secret_present('ejson-keys', managed: false)
  end

+  def test_partials
+    result = deploy_dir_without_profiling(


Our original discussion got lost, but I see why you can't use deploy_fixtures here. The best helper for this case would be deploy_raw_fixtures("test-partials", bindings: { 'supports_partials' => 'true' }). Incidentally, we should make a partial reference custom bindings (e.g. "supports_partials" in that command) to prove that works.

We also need another test about what happens when a referenced partial is not found (you can add at template to the 'invalid' fixture set for that purpose).

skaes · 2018-01-27T08:17:24Z

@KnVerey I think I have addressed all comments and known issues. I added a unit test class for testing the partials implementation, which should make it a lot simpler adding new features later on.

I have explained the limitations of the partials system in the README and changed the partials integration test to show how a pod spec can be included via a partial.

Please let me know if I have missed to address something.

klautcomputing

This implementation in the current form works and I am tentatively approving it. It would be nice if the nit pick comments would be addressed and then we can merge this. Please rebase and fixup your commits into one after @KnVerey had a change to review and after addressing her comments, too.

Thank you for implementing this!

klautcomputing · 2018-01-30T15:55:49Z

lib/kubernetes-deploy/renderer.rb

+      @id = current_sha[0...8] + "-#{SecureRandom.hex(4)}" if current_sha
+    end
+
+    def template_variables


It would make us happy :)

klautcomputing · 2018-01-30T15:57:51Z

lib/kubernetes-deploy/renderer.rb

+
+        template = @_renderer.find_partial(partial)
+        expanded_template = ERB.new(template, nil, '-').result(erb_binding)
+        if expanded_template =~ /^--- *\n/m


Could you refactor this to return expanded_template if expanded_template =~ /^--- *\n/m and thereby get rid of the else. Thanks

klautcomputing · 2018-01-30T16:03:15Z

README.md

+  b: 2
+```
+
+This is a limitation of the current implementation.


Thanks for explaining this in the readme.

klautcomputing · 2018-01-30T16:11:09Z

test/fixtures/for_unit_tests/partials/factorial.yml.erb

@@ -0,0 +1,7 @@
+value: <%= n %>


Yeah. I like it too. :-)

skaes · 2018-01-30T18:40:21Z

@klautcomputing when you say rebase and merge into one commit, do you really want me to force push this onto this pull request? That would make it hard for us to merge this work with our own master and also impossible for anyone to follow the discussion on the pull request, won't it?

skaes · 2018-01-30T19:03:51Z

can you please advise me how to fix the code style violations in the README?

klautcomputing · 2018-01-31T14:53:58Z

Rebase and fixup is how I usually do it, especially when there are so many commits. So, just one commit that has a meaningful message about what was implemented. But if that makes things difficult for you I leave it up to you and @KnVerey

skaes · 2018-01-31T17:08:55Z

Sorry, I misread the Policial messages. It complains about strings not being internationalized in the renderer unit test. Nevertheless, I really don't know how to fix this. Please advise.

KnVerey

Thanks for the JSON solution and new tests! To make sure my comments are clear (and to make sure what I'm asking for works), I pushed a branch with the changes I have in mind here: d0be76f...partial-review.

The Policial failure was in fact a bug. I re-ran it on your commit after the bug was fixed, and it passed.

Re: squashing commits, our standard process at Shopify is to continue pushing new commits while review is ongoing, but rebase and squash before merging, especially if there are more than a few commits. The idea is for master to contain meaningful commits. Github does not lose the conversation or lines of code it was made on even after the squash. Why would squashing make it harder to pull this work into your own master than it would be to pull in any other changes we make upstream?

KnVerey · 2018-02-01T18:43:53Z

README.md

+
+Inside a partial, parameters can be accessed as normal variables, or via a hash called `locals`. Thus, the `cron` template could like this:
+
+```erb


It looks like "erb" means "html/erb" to Github, which explains why nothing is getting syntax highlighted here. yaml would be more useful (same comment about the other code blocks below).

Was this forgotten/dismissed?

KnVerey · 2018-02-01T18:52:32Z

README.md

+          restartPolicy: OnFailure
+```
+
+Supported file names for a given partial `p` are `p.yaml.erb` or `p.yml.erb`, where the first form is preferred.


This makes it sound like you can name the actual file "p" and possibly use partial: 'p.yaml.erb'. How about something like:

Both .yaml.erb and .yml.erb file extensions are supported. Templates must refer to the bare filename (e.g. use partial: 'cron' to reference cron.yaml.erb)

KnVerey · 2018-02-01T19:03:31Z

lib/kubernetes-deploy/renderer.rb

+      bind_template_variables(erb_binding, template_variables)
+
+      ERB.new(raw_template).result(erb_binding)
+    rescue StandardError => e


This used to be NameError, which is the error you get when a template references something undefined. We generally consider it a best practice to rescue specifically the errors we are expecting; in this case, we want to make NameError pretty (by turning it into FatalDeploymentError) because they're the end user's fault, but for errors that are gem problems that should be fixed, getting a stack trace would be more helpful. Are there additional errors you encountered in working on this?

Well, erb templates contain arbitrary ruby code, which can raise arbitrary exceptions. I don't see why NameError should be treated special here. Either rescue all non-fatal exceptions, or none at all, would be my strategy. But I'm happy to change it back to NameError.

That's a fair point. I'm fine with StandardError.

KnVerey · 2018-02-01T19:34:37Z

lib/kubernetes-deploy/renderer.rb

+        begin
+          JSON.generate(YAML.load(expanded_template))
+        rescue Psych::SyntaxError => e
+          raise "#{e.class}#{e}. Partial did not expand to valid YAML, source: #{expanded_template}"


To get this formatted nicely for the user, we should do:

@logger.summary.add_paragraph("Rendered template content:\n#{expanded_template}") raise FatalDeploymentError, "Partial #{partial} is not valid YAML"

(I realize the logger bit isn't possible inside this class; see my other request)

Refactored the class and changed.

KnVerey · 2018-02-01T19:49:44Z

lib/kubernetes-deploy/renderer.rb

+        binding
+      end
+
+      def partial(partial, locals = {})


In my opinion, this method should simply delegate to a render_partial method on the renderer that itself makes use of the existing render method. This has several advantages:

The ability to both keep the Renderer's public interface clean and prevent this private class from reaching into its internals (both sends and making methods public for the sake of an implementation detail are smells IMO).

The ability to share the rendering logic and error handling between partials and regular templates.

The ability to use the logger to augment errors (see my other comment)

Clarity as to whether, given template -> partial1 -> partial2, variables set within partial1 will be available in partial2 (they shouldn't be, unless exposed purposefully through locals, and I think they would be right now)

Variables are not inherited from an embedding partial in the current implementation. The only variables available in all templates are the ones passed in via extra bindings plus current_sha and deployment_id. I would consider it an extreme nuisance to not have those available automatically everywhere.

I 100% agree that the extra bindings and standard variables should be available in all templates. You're right that local variables are not inherited, but instance variables are. Here's how I tested that on your branch:

# deployment.yaml.erb <% @test = "hello" %> <%= partial "pod", name: "pod1", args: ["echo", "foo"] %> # pod.yaml.erb ... args: <%= @test %>

Results in:

Yes, instance variables are available. Just like in Rails. In my view, this is a feature.

KnVerey · 2018-02-01T19:57:13Z

lib/kubernetes-deploy/renderer.rb

+
+        # If we're at top level we don't need to worry about the result being
+        # included in another partial.
+        return expanded_template if expanded_template =~ /^--- *\n/m


I'm always hesitant to use regexes to parse language tokens. For example, this version would miss document separator lines:

that are followed by tabs

that end with line breaks other than \n

that include comments

probably other conditions that I'm not thinking of

The worst consequence of failing to return here would be that YAML.load would silently drop documents if there are more than one, which is pretty bad. Imagine you've deployed your resources using partials, then someone accidentally commits a tab (or whatever thing it doesn't occur to us to handle), and this causes the deploy to prune some of the resources!

Incidentally, we should have a test for the case where a partial contains multiple documents.

The most pragmatic solution I could come up with is to parse the partial as a stream, return it verbatim if there are multiple documents, and always turn it into JSON if there's just one document. This has the limitation of requiring document separators to be placed in parent templates, but that seems more reasonable than risking resource loss. If you or @klautcomputing have a better idea, I'm all for it!

The change you are proposing would invalidate templates in dozens of our projects which have been happily using the originally proposed implementation of partials system without any complaints for two months now. I do not approve of it.

My suggestion is to continue using a regexp, but protect against potential document loss by using YAML.load_stream to detect whether we missed detecting multiple documents.

There's already a test which has a partial containing multiple documents. See partial 'stream.yaml.erb' in test_can_render_template_with_correct_indentation.

I think I've found a solution that will work for everyone:

docs = YAML.parse_stream(expanded_template) # If the partial contains multiple documents or has an explicit document header, # we know it cannot validly be indented in the parent, so return it immediately. return expanded_template unless docs.children.one? && docs.children.first.implicit # Make sure indentation isn't a problem by producing a single line of parseable YAML. # Note that JSON is a subset of YAML. JSON.generate(docs.children.first.to_ruby)

Here's evidence that docs.children.first.implicit is detecting exactly what we want:

[7] pry(main)> docs = { [7] pry(main)* explicit_doc: YAML.parse_stream("---\nfoo: bar"), [7] pry(main)* implicit_doc: YAML.parse_stream("foo: bar"), [7] pry(main)* multiple_docs: YAML.parse_stream("foo: bar\n---\nbar: baz") [7] pry(main)* } [9] pry(main)> docs.each do |type, value| [9] pry(main)* puts "#{type} -> Multiple docs: #{!value.children.one?} / First has explicit header: #{!value.children.first.implicit}" [9] pry(main)* end explicit_doc -> Multiple docs: false / First has explicit header: true implicit_doc -> Multiple docs: false / First has explicit header: false multiple_docs -> Multiple docs: true / First has explicit header: false

Interesting. I'll study this tomorrow.

KnVerey · 2018-02-01T23:16:15Z

lib/kubernetes-deploy/renderer.rb

+        @_renderer.__send__(:bind_template_variables, erb_binding, variables)
+
+        template = @_renderer.__send__(:find_partial, partial)
+        expanded_template = ERB.new(template, nil, '-').result(erb_binding)


The errors that can be raised by this are not currently handled (unlike in the main render method)

You are right, the error is handled on the main method. I thought that would be fine. But I'm happy to change it.

To clarify why I'd like this, it's to be able to have the error message we show reflect the specific file the error happened in, have line number references (if any) make sense, and be able to include a dump of the relevant partial only.

skaes · 2018-02-03T09:03:53Z

@KnVerey I think I have addressed all items of the latest code review, except sharing of instance variables during rendering. If you insist, I will change that too. I will rebase and force push once you are OK with the contents.

KnVerey

The implementation looks good now, but I noticed we're producing some pretty terrible output when there are errors in partials; see my comment and my suggested fix (9cf798f...partial_review3).

I think I have addressed all items of the latest code review, except sharing of instance variables during rendering. If you insist, I will change that too.

Please do make that change too. In the absence of a compelling use case for allowing instance variables to be passed around instead of using locals, I'd prefer to start with the more restrictive option. The fact that rails partials allow this doesn't strike me as particularly relevant, since they are typically set in controllers in that context, and those have no analogue here.

KnVerey · 2018-02-21T03:06:42Z

test/unit/kubernetes-deploy/renderer_test.rb

+  end
+
+  def test_invalid_partial_raises
+    assert_raises(KubernetesDeploy::FatalDeploymentError) do


Let's assert on the actual error message her and in the test below (can use our custom assert_raises_message or the return value of this method)

KnVerey · 2018-02-21T05:02:09Z

lib/kubernetes-deploy/renderer.rb

+      bind_template_variables(erb_binding, template_variables)
+
+      ERB.new(raw_template, nil, '-').result(erb_binding)
+    rescue StandardError => err


Gave the feature another spin, including with some invalid templates, and noticed that rescuing StandardError both here and in render_partial is leading to some confusing error output when a partial is invalid (first screenshot) or includes a missing partial (second screenshot):

Let's not only fix that, but also provide the render sequence to help people debug. Here's one way to do that: 9cf798f...partial_review3

Since logging is our main UI in this project, I'd also like to see some integration tests with thorough assertions for those error cases. That commit also shows what I have in mind there. For reference, here's the "after" output:

skaes · 2018-02-22T13:19:52Z

Merged your proposed changes and ensured instance variables are not shared between partials. Please let me know if this is good to go or not.

KnVerey

LGTM and CI is green. Please rebase and squash as previously discussed, and I'll merge and release this. Thanks for all your work on this feature!

skaes · 2018-02-24T09:01:30Z

Rebased and squashed. Please note there are still random test failures when running against a minikube 1.8 cluster, butthey are unrelated to the PR.

klautcomputing

Looking forward to seeing that getting used in production!

I wasn't sure whether the yaml/erb code highlighting in the makefile was overlooked or decided to keep it as is. But that's my only comment.

klautcomputing · 2018-02-26T03:45:12Z

README.md

+
+Inside a partial, parameters can be accessed as normal variables, or via a hash called `locals`. Thus, the `cron` template could like this:
+
+```erb


Was this forgotten/dismissed?

skaes · 2018-02-26T15:35:33Z

@klautcomputing Force pushed latest change request. Might have overlooked changing it.

skaes · 2018-02-26T18:07:44Z

\o/

skaes mentioned this pull request Nov 12, 2017

Support Helm-style templating #44

Closed

KnVerey requested review from kirs and KnVerey November 30, 2017 01:47

KnVerey mentioned this pull request Dec 19, 2017

Allow template rendering from the command line #228

Closed

KnVerey suggested changes Jan 18, 2018

View reviewed changes

KnVerey requested review from klautcomputing and removed request for kirs January 19, 2018 16:33

klautcomputing suggested changes Jan 19, 2018

View reviewed changes

klautcomputing mentioned this pull request Jan 22, 2018

Enhance code quality xing/kubernetes-deploy#1

Open

KnVerey reviewed Jan 25, 2018

View reviewed changes

klautcomputing approved these changes Jan 30, 2018

View reviewed changes

KnVerey suggested changes Feb 2, 2018

View reviewed changes

KnVerey reviewed Feb 21, 2018

View reviewed changes

KnVerey approved these changes Feb 23, 2018

View reviewed changes

skaes force-pushed the support-partials branch from 60e978f to a856be7 Compare February 24, 2018 07:40

klautcomputing approved these changes Feb 26, 2018

View reviewed changes

Support partials to reduce duplication in yaml files.

a81d488

skaes force-pushed the support-partials branch from a856be7 to a81d488 Compare February 26, 2018 15:34

klautcomputing merged commit 639ba3d into Shopify:master Feb 26, 2018

klautcomputing had a problem deploying to rubygems February 27, 2018 16:13 Failure

klautcomputing deployed to rubygems February 27, 2018 16:14 Active


		`kubernetes-deploy` supports composing templates from so called partials in order to reduce duplication in Kubernetes YAML files. Given a template directory `DIR`, partials are searched for in `DIR/partials`and in 'DIR/../partials', in that order. They can be embedded in other ERB templates using the helper method `partial`. For example, let's assume an application needs a number of different CronJob resources, on could place a template called `cron`in one of those directories and then use it in the main deployment.yaml.erb like so:


		Inside a partial, parameters can be accessed as normal variables, or via a hash called `locals`. Thus, the `cron` template could like this:

		```erb

support partials #207

support partials #207

Conversation

skaes commented Nov 12, 2017

KnVerey commented Dec 19, 2017

skaes commented Dec 20, 2017

skaes commented Dec 20, 2017

skaes commented Jan 18, 2018

KnVerey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skaes Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klautcomputing Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

skaes Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klautcomputing Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klautcomputing Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

klautcomputing Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

skaes commented Jan 20, 2018

KnVerey commented Jan 23, 2018

skaes commented Jan 24, 2018 • edited Loading

KnVerey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skaes commented Jan 27, 2018 • edited Loading

klautcomputing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skaes commented Jan 30, 2018

skaes commented Jan 30, 2018

klautcomputing commented Jan 31, 2018

skaes commented Jan 31, 2018 • edited Loading

KnVerey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skaes Jan 19, 2018 •

edited

Loading

klautcomputing Jan 19, 2018 •

edited

Loading

skaes Jan 20, 2018 •

edited

Loading

klautcomputing Jan 19, 2018 •

edited

Loading

klautcomputing Jan 22, 2018 •

edited

Loading

klautcomputing Jan 19, 2018 •

edited

Loading

skaes commented Jan 24, 2018 •

edited

Loading

skaes commented Jan 27, 2018 •

edited

Loading

skaes commented Jan 31, 2018 •

edited

Loading

skaes Feb 3, 2018 •

edited

Loading