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

Simplify input type Helm #977 #979

Closed
wants to merge 5 commits into from
Closed

Simplify input type Helm #977 #979

wants to merge 5 commits into from

Conversation

gburiola
Copy link
Contributor

@gburiola gburiola commented Mar 22, 2023

Fixes issue #977

test

this target file supports both formats of input_type: helm:

  • with a local chart
  • with URL and without input_path

As we can see in the output below, the helm binary is called in two different ways for each mode.

launching helm with arguments: ['template', '--include-crds', '--skip-tests', '--output-dir', '/tmp/tmp6318ts8t', '--generate-name', '/home/lburiola/git/github/kapitan/tests/test_resources/charts/acs-engine-autoscaler']

launching helm with arguments: ['template', '--include-crds', '--skip-tests', '--repo', 'https://charts.jetstack.io', '--version', '1.1.0', '--output-dir', '/tmp/tmp0iqle40w', '--generate-name', 'cert-manager']

FULL OUTPUT

$ ../../bin/kapitan compile -t acs-engine-autoscaler -v
2023-03-22 18:37:30,374 git.cmd      DEBUG    Popen(['git', 'version'], cwd=/home/lburiola/git/github/kapitan/tests/test_resources, universal_newlines=False, shell=None, istream=None)
2023-03-22 18:37:30,384 git.cmd      DEBUG    Popen(['git', 'version'], cwd=/home/lburiola/git/github/kapitan/tests/test_resources, universal_newlines=False, shell=None, istream=None)
2023-03-22 18:37:30,452 kapitan.cli  DEBUG    Running with args: Namespace(cache=False, cache_paths=[], compose_node_name=False, embed_refs=False, fetch=False, force=False, force_fetch=False, func=<function trigger_compile at 0x7feda5336b00>, ignore_version_check=False, indent=2, inventory_path='./inventory', jinja2_filters='lib/jinja2_filters.py', labels=[], name='compile', output_path='.', parallelism=4, prune=False, quiet=False, refs_path='./refs', reveal=False, schemas_path='./schemas', search_paths=['.', 'lib'], subparser_name='compile', targets=['acs-engine-autoscaler'], use_go_jsonnet=False, validate=False, verbose=True, yaml_dump_null_as_empty=False, yaml_multiline_string_style='double-quotes')
2023-03-22 18:37:30,543 kapitan.resources DEBUG    Using reclass inventory config defaults
2023-03-22 18:37:30,578 kapitan.utils DEBUG    hashable_lru_cache: True not serialiseable, using generic lru_cache instead
2023-03-22 18:37:30,614 kapitan.targets DEBUG    validating target name matches the name of yml file acs-engine-autoscaler
2023-03-22 18:37:30,614 kapitan.targets DEBUG    load_target_inventory: found valid kapitan target acs-engine-autoscaler
2023-03-22 18:37:30,614 kapitan.targets INFO     Rendered inventory (0.07s)
2023-03-22 18:37:30,873 git.cmd      DEBUG    Popen(['git', 'version'], cwd=/home/lburiola/git/github/kapitan/tests/test_resources, universal_newlines=False, shell=None, istream=None)
2023-03-22 18:37:30,879 git.cmd      DEBUG    Popen(['git', 'version'], cwd=/home/lburiola/git/github/kapitan/tests/test_resources, universal_newlines=False, shell=None, istream=None)
2023-03-22 18:37:31,271 kapitan.inputs.base DEBUG    Compiling /home/lburiola/git/github/kapitan/tests/test_resources/charts/acs-engine-autoscaler
2023-03-22 18:37:31,272 kapitan.helm_cli DEBUG    launching helm with arguments: ['template', '--include-crds', '--skip-tests', '--output-dir', '/tmp/tmp6318ts8t', '--generate-name', '/home/lburiola/git/github/kapitan/tests/test_resources/charts/acs-engine-autoscaler']
2023-03-22 18:37:31,363 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./acs-engine-autoscaler/templates/secrets.yaml
2023-03-22 18:37:31,364 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp6318ts8t/acs-engine-autoscaler/templates/secrets.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./acs-engine-autoscaler/templates/secrets.yaml
2023-03-22 18:37:31,439 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./acs-engine-autoscaler/templates/deployment.yaml
2023-03-22 18:37:31,440 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp6318ts8t/acs-engine-autoscaler/templates/deployment.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./acs-engine-autoscaler/templates/deployment.yaml
2023-03-22 18:37:31,440 kapitan.inputs.base DEBUG    Compiling 
2023-03-22 18:37:31,440 kapitan.helm_cli DEBUG    launching helm with arguments: ['template', '--include-crds', '--skip-tests', '--repo', 'https://charts.jetstack.io', '--version', '1.1.0', '--output-dir', '/tmp/tmp0iqle40w', '--generate-name', 'cert-manager']
2023-03-22 18:37:33,127 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-serviceaccount.yaml
2023-03-22 18:37:33,127 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/cainjector-serviceaccount.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-serviceaccount.yaml
2023-03-22 18:37:33,130 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/serviceaccount.yaml
2023-03-22 18:37:33,130 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/serviceaccount.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/serviceaccount.yaml
2023-03-22 18:37:33,133 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-serviceaccount.yaml
2023-03-22 18:37:33,133 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-serviceaccount.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-serviceaccount.yaml
2023-03-22 18:37:33,152 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-rbac.yaml
2023-03-22 18:37:33,152 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/cainjector-rbac.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-rbac.yaml
2023-03-22 18:37:33,227 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/rbac.yaml
2023-03-22 18:37:33,227 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/rbac.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/rbac.yaml
2023-03-22 18:37:33,235 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-rbac.yaml
2023-03-22 18:37:33,235 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-rbac.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-rbac.yaml
2023-03-22 18:37:33,239 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/service.yaml
2023-03-22 18:37:33,239 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/service.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/service.yaml
2023-03-22 18:37:33,243 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-service.yaml
2023-03-22 18:37:33,243 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-service.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-service.yaml
2023-03-22 18:37:33,250 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-deployment.yaml
2023-03-22 18:37:33,250 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/cainjector-deployment.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/cainjector-deployment.yaml
2023-03-22 18:37:33,259 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/deployment.yaml
2023-03-22 18:37:33,259 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/deployment.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/deployment.yaml
2023-03-22 18:37:33,270 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-deployment.yaml
2023-03-22 18:37:33,270 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-deployment.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-deployment.yaml
2023-03-22 18:37:33,276 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-mutating-webhook.yaml
2023-03-22 18:37:33,276 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-mutating-webhook.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-mutating-webhook.yaml
2023-03-22 18:37:33,283 kapitan.inputs.base DEBUG    Wrote /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-validating-webhook.yaml
2023-03-22 18:37:33,283 kapitan.inputs.helm DEBUG    Wrote file /tmp/tmp0iqle40w/cert-manager/templates/webhook-validating-webhook.yaml to /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler/./cert-manager/templates/webhook-validating-webhook.yaml
2023-03-22 18:37:33,283 kapitan.targets INFO     Compiled acs-engine-autoscaler (2.01s)
2023-03-22 18:37:33,291 kapitan.targets DEBUG    Copied /tmp/tmpom1otnvu.kapitan/compiled/acs-engine-autoscaler into ./compiled/acs-engine-autoscaler
2023-03-22 18:37:33,581 kapitan.targets DEBUG    Removed /tmp/tmpom1otnvu.kapitan

@Moep90
Copy link
Contributor

Moep90 commented Mar 22, 2023

  • Add Tests
  • Update Website, if needed

@@ -55,7 +74,7 @@ Special flags:

See the [helm doc](https://helm.sh/docs/helm/helm_template/) for further detail.

#### Example
## Example
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix precommit error

markdownlint-fix.........................................................Failed
- hook id: markdownlint-fix
- exit code: 1

docs/pages/input_types/helm.md:77 MD001/heading-increment/header-increment Heading levels should only increment by one level at a time [Expected: h2; Actual: h4]

kapitan/inputs/base.py Show resolved Hide resolved
@gburiola
Copy link
Contributor Author

@gburiola
Copy link
Contributor Author

  • Update Website, if needed

@Moep90 I don't understand this. The PR already updates the documentation. Do you think it's not sufficient or are you talking about updating something else?

Copy link
Contributor

@uberspot uberspot left a comment

Choose a reason for hiding this comment

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

Looks safe and a nice simplification for using helm 👍

kapitan/inputs/helm.py Show resolved Hide resolved
kapitan/inputs/base.py Show resolved Hide resolved
kapitan/targets.py Outdated Show resolved Hide resolved
@Moep90
Copy link
Contributor

Moep90 commented Mar 23, 2023

  • Update Website, if needed

@Moep90 I don't understand this. The PR already updates the documentation. Do you think it's not sufficient or are you talking about updating something else?

NVM, my bad.

@ademariag ademariag added the enhancement enhancement to an existing feature label Mar 24, 2023
@ramaro
Copy link
Member

ramaro commented Mar 24, 2023

Hey @gburiola this is awesome! One question - how does this deal with caching? E.g. will subsequent executions of kapitan compile use a cached helm repo on disk or will it download the repo everytime?

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.

Generally looks good to me, just requesting a small comment

@gburiola
Copy link
Contributor Author

Hey @gburiola this is awesome! One question - how does this deal with caching? E.g. will subsequent executions of kapitan compile use a cached helm repo on disk or will it download the repo everytime?

It doesn't cache anything. It fetches the chart on the fly using helm cli with parameter --repo <CHART_URL>.

Bear in mind that the the current way using dependencies also makes an HTTP call to the depency URL every time kapitan compile is called.

@gburiola gburiola requested a review from ramaro March 25, 2023 22:48
@MatteoVoges
Copy link
Contributor

Bear in mind that the the current way using dependencies also makes an HTTP call to the depency URL every time kapitan compile is called.

Are you sure about that? I thought that we have the flags --fetch / --force-fetch and --cache to specify which behavior we want for our dependencies, so that they initially get fetched, get fetched everytime or even get cached...
Note that we also have a force_fetch property in the inventory as well, that enables fetching on user-selected resources.
Could you check that this behavior integrates with your changes?

@gburiola
Copy link
Contributor Author

Are you sure about that?

Yes. --fetch always dowloads external file, unpacks and doesn't replace local copy on disk if it already exists.
--force-fetch does the same as above, but overrites existing copy on disk.

--cache only refers to kapitan dependencies management.

I'm not using dependency mgmt in this case. I'm using the helm cli --repos URL directly.

I've updated the docs on 614807e

@ramaro
Copy link
Member

ramaro commented Mar 31, 2023

Bear in mind that the the current way using dependencies also makes an HTTP call to the depency URL every time kapitan compile is called.

OK, worth making it very obvious in the documentation of the behaviour of this alternative way if we still think it's worth it! Is there no way helm will do some sort of caching at all? If not, this feels suboptimal to me.

@ademariag
Copy link
Contributor

ademariag commented Mar 31, 2023

@gburiola @ramaro helm does seem to support caching? https://helm.sh/docs/helm/helm/

We also need to understand what happens if multiple compile of the same helm compete to download the charts at the same time.

@github-actions github-actions bot added the Stale label Nov 9, 2023
@github-actions github-actions bot removed the Stale label May 11, 2024
@github-actions github-actions bot added the Stale label Aug 18, 2024
@ademariag ademariag closed this Sep 1, 2024
@ademariag ademariag deleted the issue_977 branch September 1, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement enhancement to an existing feature Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants