Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

RFC: Replace Helm charts with Kustomize for application packaging #101

Closed
wants to merge 5 commits into from

Conversation

juliogreff
Copy link
Contributor

@juliogreff juliogreff commented Jun 6, 2019

This is a proposal for how we'd use kustomize in Shipper, if we were to ditch helm charts. What I want to get out of this RFC is:

  1. With a more concrete suggestion of how things could look like (this RFC), do we wanna move away from helm into kustomize?

  2. Assuming we do, is the proposed way the actual way we wanna go? Did I miss any challenges? Will this make users' lives more difficult or easier? Does it actually solve the problems it's supposed to solve? Does it create any new ones I didn't foresee?

@juliogreff juliogreff force-pushed the jgreff/rfc-kustomize branch 3 times, most recently from 644eb21 to f6bd413 Compare June 11, 2019 11:29
@juliogreff juliogreff force-pushed the jgreff/rfc-kustomize branch from f6bd413 to 968edc4 Compare June 12, 2019 08:29
@juliogreff juliogreff marked this pull request as ready for review June 12, 2019 08:29
@kanatohodets
Copy link
Contributor

Nice, the inline patch looks really good to me. Thinking about use cases: if I want to add a totally new Kubernetes object to my deployment, like my-cool-configmap, how would I do it?

Could I use a "role"?

apiVersion: shipper.booking.com/v1alpha1
kind: Application
metadata:
  name: bikerental
spec:
  template:
    clusterRequirements: {}
    kustomization:
      image:
        name: juliogreff/bikerental
        tag: deadcafe
      bases:
        - https://example.com/core
        # like this? 
        - https://example.com/my-cool-configmap
      namePrefix: bikerental-
      commonLabels:
        app: bikerental
      patchesStrategicMerge:
        - https://example.com/sidecars/sidecar.yaml
    ....

Or would I need an inheritance hierarchy?

apiVersion: shipper.booking.com/v1alpha1
kind: Application
metadata:
  name: bikerental
spec:
  template:
    clusterRequirements: {}
    kustomization:
      image:
        name: juliogreff/bikerental
        tag: deadcafe
      bases:
        # hopefully not a fork, but just inherited... 
        # and then I guess something happens with version resolution internally... aaagh
        - https://example.com/core-with-my-cool-configmap 
      namePrefix: bikerental-
      commonLabels:
        app: bikerental
      patchesStrategicMerge:
        - https://example.com/sidecars/sidecar.yaml
     ...

If the approach is more like a role, I'm pretty excited about this. I assume so, since the field is a list.

Some things we'll need to figure out, I think:

  • version resolution + caching strategy for kustomize bases/patches in shipper.
  • (internally, but I also think this is a problem in the wider ecosystem) hosting kustomize patches somewhere shipper can pull them from, ideally with versioning/access control/etc.

Regarding versioning of the bases/patches: a key requirement is the ability for Shipper to request a 'floating' version like 0.3.x, but resolve that to a concrete version, like 0.3.1. This is akin to Docker tags + image SHA1s. Just like our plan for floating Chart version ranges, Release objects should always have a concrete version in them, so that rollbacks can happen without needing the Kustomize repository available. This might be a good question to raise on #kustomize on the K8s slack.

== Open questions

* The migration path for Shipper to move from Helm charts to Kustomize is
entirely unclear. We'll probably need to have both ways of packaging
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think dual life is inevitable, at least for a while. It might be possible to automatically convert 'values' sections to patches, but it might not be worth the effort. My first cut would probably be just distinct 'chart' and 'kustomize' fields, a validation in the webhook which prevents both from being populated at once, or 'values' from being populated if 'kustomize' is.

The application object ought to be able to apply more powerful changes
to base kustomizations, as in, any arbitrary patch it wants. We still
have steps to take to allow that, but moving the Kustomization itself
inline will solve two issues:

1. The `image` value is no longer special. It's just part of kustomize
itself.

2. The kustomization itself is no longer external to the application
object, so external versioning (in source control, for instance) is no
longer an issue.
@juliogreff juliogreff force-pushed the jgreff/rfc-kustomize branch from 9a642e8 to 5b01b76 Compare June 13, 2019 10:34
@juliogreff
Copy link
Contributor Author

@kanatohodets re: adding a completely new object:

Could I use a "role"?

Yes. The only downside of having multiple bases I found so far is that you can't patch objects in one base from the other.

Or would I need an inheritance hierarchy?

You can do that too, and you get to patch objects from your base as well as a bonus.

BUT! The idiomatic way of doing it is just by declaring new resources, directly in .spec.kustomize.resources. Docs here.

apiVersion: shipper.booking.com/v1alpha1
kind: Application
metadata:
  name: bikerental
spec:
  template:
    clusterRequirements: {}
    kustomization:
      image:
        name: juliogreff/bikerental
        tag: deadcafe
      bases:
        - https://example.com/core
      resources:
        - https://example.com/my-cool-configmap.yaml
     ...

@kanatohodets
Copy link
Contributor

kanatohodets commented Jun 19, 2019

I think the only question remaining for me is about sidecar configuration. As discussed in person, we'll shift from sidecars being configured directly in their rendered YAML (with if conditions in gotmpl, like Helm) to consuming a ConfigMap which the user can work with, following a particular naming convention. So the user would provide a 'patchStrategicMergeInline` which modifies the appropriate ConfigMap. Can Kustomize mutate contents of ConfigMaps in a sensible way?

This also might raise some questions about the ordering of patchStrategicMergeInline vs patchStrategicMerge.

@kanatohodets
Copy link
Contributor

I lied, another question: we have some resources which should be suffixed with a per-release deadbeef (most of them) and some which should not (the main Service object, which should overwrite whatever is currently in the cluster). I think you mentioned a strategy for this, but I don't remember what it was. How can we handle these cases, where the 'suffix' should not be universally applied?

@kanatohodets
Copy link
Contributor

I think the answer to the sidecar ConfigMap configuration question is https://github.com/kubernetes-sigs/kustomize/blob/master/docs/fields.md#configmapgenerator

@osdrv osdrv added the enhancement New feature or request label Jul 29, 2019

Since we do Semantic Versioning, and to avoid the need of a new major version
release when we do remove support for Helm charts, Helm support should be
dropped before Shipper turns 1.0.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have this migration plan as a series of steps instead of a paragraph to emphasize the scope of each step.

@kanatohodets
Copy link
Contributor

It looks like Kustomize supports inline patches as of September this year, which is cool!

I think the answer to my question about suffixing some resources with the hash for that release is that we'd include a custom transformer in our implementation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants