Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Kustomize based PostRenderer #202

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

alex-berger
Copy link
Contributor

@alex-berger alex-berger commented Feb 1, 2021

This PR adds support for Kustomize based Post Renderer to the HelmRelease resource. This PR is a first draft, to gather feedback from the project maintainers and while it compiles is has not yet been well tested.

Goals:

  • Add simple built-in Post Renderer capability which supports as subset of Kustomize (patchesStrategicMerge, patchesJson6902 and images) and which will work with the default helm-controller image.
  • Add minimal framework to support more Post Renderers in future.

Non Goals:

  • Supporting arbitrary non-built-in Post Renderers which might require special image with external tools included.

Example Usage

---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: metrics-server
  namespace: kube-system
spec:
  interval: 1m
  chart:
    spec:
      chart: metrics-server
      version: "5.3.4"
      sourceRef:
        kind: HelmRepository
        name: bitnami
        namespace: kube-system
      interval: 1m
  postRenderers:
  - kustomize: 
       patchesStrategicMerge:
       -  kind: Deployment
          apiVersion: apps/v1
          metadata:
            name: metrics-server
            namespace: kube-system
          spec:
            template:
              spec:
                tolerations:
                - key: "workload-type"
                  operator: "Equal"
                  value: "cluster-services"
                  effect: "NoSchedule"

Motivation

As SRE / DevOps engineer I often have the need to patch Helm Charts from third-parties, either by forking them or by contributing to the upstream via pull requests. Both approaches are cumbersome and time intensive and often cause project delays and errors. Having simple built-in Helm Post Renderer support in helm-controller will allows me to patch Helm Releases immediately without having to fork a Helm Chart or waiting for upstream contributions to be integrated.

This PR also (partially) addresses #104

@alex-berger alex-berger force-pushed the feature/post-renderer branch from ccdb551 to 6a10dcb Compare February 1, 2021 12:53
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Thank you for putting time into this, as it has been sought after 🌷

To not directly import a lot of new functionalities (and half of Kustomize), and to encourage people to continue to make changes to charts upstream, I think it would be best if we limit the set of functionalities to what was discussed in the issue.

This would cover most people their base needs, and we can then start a discussion around any further expansions.

api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
internal/runner/post_renderer_kustomize.go Outdated Show resolved Hide resolved
@hiddeco hiddeco added area/helm Helm related issues and pull requests enhancement New feature or request labels Feb 2, 2021
@alex-berger alex-berger force-pushed the feature/post-renderer branch 2 times, most recently from 1ce5ee3 to c61f976 Compare February 2, 2021 12:35
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Couple of tiny nitpicks, otherwise looking OK to me. 👍

It would also be awesome if you could reflect to changes made to the in-code API in the documentation in docs/spec/v2beta1/helmreleases.md.

NB: you accidentally checked the full helm-controller binary into your commit, you probably want to remove this from the commit and push --force-with-lease :-)

internal/runner/post_renderer.go Outdated Show resolved Hide resolved
internal/runner/post_renderer.go Outdated Show resolved Hide resolved
internal/runner/post_renderer_kustomize.go Outdated Show resolved Hide resolved
internal/runner/post_renderer_kustomize.go Outdated Show resolved Hide resolved
api/v2beta1/helmrelease_types.go Outdated Show resolved Hide resolved
api/v2beta1/kustomization_types.go Show resolved Hide resolved
@hiddeco hiddeco requested a review from stefanprodan February 2, 2021 16:14
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Some additional end-to-end tests with base coverage would be very welcome too, to ensure the full picture is wired correctly.

api/go.mod Outdated Show resolved Hide resolved
@alex-berger alex-berger force-pushed the feature/post-renderer branch 2 times, most recently from 16c323f to c608105 Compare February 2, 2021 17:33
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Awesome work, and if you smash your commits together I would be happy to merge them! 🎉

The helm-controller release that is going to include these changes is going to take a little longer, as we want to move some of the API elements introduced here to fluxcd/pkg so they can be shared with the kustomize-controller. Work on this should however not take too long. 🌻

Signed-off-by: Alexander Berger <alex-berger@gmx.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants