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

Add parameter to set kube version for Helm charts rendering #671

Merged
merged 1 commit into from
Jan 26, 2021

Conversation

dverbeir
Copy link
Contributor

Fixes issue #543

Proposed Changes

Some charts generate manifests slightly differently depending on the target Kubernetes version (e.g. targeting different APIs). In order to obtain the appropriate rendered manifests, the Kubernetes version can be set using the kapitan kube_version parameter:

parameters:
  kapitan:
    kube_version: 1.16
  • target_obj is provided to the Helm "compiler" which extracts the kube_version parameter and passes it to the go libtemplate to be set when rendering the chart.

@google-cla
Copy link

google-cla bot commented Dec 24, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Dec 24, 2020
@uberspot uberspot requested a review from ramaro December 29, 2020 07:57
@ramaro
Copy link
Member

ramaro commented Dec 30, 2020

Hey @dverbeir thanks for this! I think we could make it more standard with the rest of the helm options if implementing kube_version instead as a parameter for the helm compile item, e.g.

parameters:
   compile:
      - input_type: helm
        output_path: .
        input_paths:
          - charts/prometheus
        kube_version: 1.16
        ....

That way we can still have a global value passed to it and have targets define their own e.g.:

parameters:
  my_kube_version: 1.20

  kapitan:
   compile:
      - input_type: helm
        output_path: .
        input_paths:
          - charts/prometheus
        kube_version: ${my_kube_version}
        ....

Also this means that target_obj wouldn't need to be passed into Helm(), as this value will be accessible via comp_obj.
What do you think?

@google-cla
Copy link

google-cla bot commented Dec 31, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@dverbeir
Copy link
Contributor Author

Also this means that target_obj wouldn't need to be passed into Helm(), as this value will be accessible via comp_obj.
What do you think?

Unfortunately, as input:compile_file() doesn't receive the comp_obj, this doesn't really simplify anything. But I agree that it looks more standard to have kube_version under the compile item. I pushed a revised changeset.

@dverbeir
Copy link
Contributor Author

dverbeir commented Jan 5, 2021

@googlebot I signed it!

@google-cla
Copy link

google-cla bot commented Jan 5, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@dverbeir
Copy link
Contributor Author

dverbeir commented Jan 8, 2021

@googlebot I signed it!

@google-cla
Copy link

google-cla bot commented Jan 8, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@dverbeir
Copy link
Contributor Author

dverbeir commented Jan 8, 2021

@googlebot What a pain!

Could somebody check what's wrong with Google CLA? We set up a corporate CLA 4 days ago and it's still not active... Don't you want contributions???

@google-cla
Copy link

google-cla bot commented Jan 8, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@uberspot
Copy link
Contributor

uberspot commented Jan 8, 2021

@googlebot What a pain!

Could somebody check what's wrong with Google CLA? We set up a corporate CLA 4 days ago and it's still not active... Don't you want contributions???

Hmm, we don't have full control over it. Are you using the exact same email for your git commits as the one you used for the CLA? Afaik that's the main thing that has to match (?).

@google-cla
Copy link

google-cla bot commented Jan 8, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@dverbeir
Copy link
Contributor Author

dverbeir commented Jan 8, 2021

@googlebot What a pain!
Could somebody check what's wrong with Google CLA? We set up a corporate CLA 4 days ago and it's still not active... Don't you want contributions???

Hmm, we don't have full control over it. Are you using the exact same email for your git commits as the one you used for the CLA? Afaik that's the main thing that has to match (?).

I understand it's not under you control, of course. Sorry for letting my frustration out...
I was just hoping you could help accelerate this.

Exact same email is used. Problem is likely with corporate CLA itself. We contacted cla-submissions@google.com but no response so far...

@google-cla
Copy link

google-cla bot commented Jan 8, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

Copy link
Member

@ramaro ramaro left a comment

Choose a reason for hiding this comment

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

Looking good! Just the small nit and we can merge. Don't worry about the CLA issue as you signed it, sometimes the bot is flaky and we can still merge.

docs/compile.md Outdated Show resolved Hide resolved
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

Some charts generate manifests slightly differently depending on the target
Kubernetes version (e.g. targeting different APIs). In order to obtain the
appropriate rendered manifests,  the Kubernetes version can be set using the
`kube_version` compile parameter:

    parameters:
      kapitan:
        compile:
          - input_type: helm
            output_path: .
            input_paths:
              - charts/prometheus
            kube_version: "1.16"

When not specified, the earlier default of v1.12 is used.

This currently only influences Helm chart compilation but the same parameter
could be shared with other "compilers".

Signed-off-by: David Verbeiren <david.verbeiren@tessares.net>
@ademariag
Copy link
Contributor

@dverbeir as a side effect of moving the repository to kapicorp, the CLA problem should go away :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants