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

Ignore helm charts when scanning manifests #993

Merged
merged 3 commits into from
Mar 9, 2018

Conversation

squaremo
Copy link
Member

@squaremo squaremo commented Mar 9, 2018

At the minute, if you have any Helm charts in the path you give to flux, it'll probably bork. The templated manifests in <chart>/templates/ will either not parse as yamels, or be applied with weird field values.

So: when walking the repo looking for manifests, skip any directory that looks like a chart. What looks like a chart? The telltale things are:

  • a Chart.yaml file; and,
  • a values.yaml file.

Both of these, and only these, are mandatory. Looking for anything else would give false negatives (-> failing to skip a chart); not looking for those would give false positives (-> failing to include a non-chart).

The first commit prepares the ground a little by making the test fixture more varied (i.e., including subdirectories and non-yamels), so chart files can be added to it in the latter commit to verify that everything still works by virtue of the chart being ignored.

squaremo added 2 commits March 8, 2018 16:29
A couple of tests assume that the test files are 1-1 with
resources. This commit removes that assumption by comparing things to
the curated ServiceMap() instead.

It also puts one of the files in a subdirectory, to make sure things
still work.
This commit makes the kubernetes manifest code skip over charts. This
is a convenience so that people don't have to segregate their config
and charts.
@squaremo squaremo requested a review from tamarakaufler March 9, 2018 12:17
@squaremo squaremo merged commit e78dc32 into master Mar 9, 2018
@squaremo squaremo deleted the feature/ignore-helm-charts branch March 9, 2018 12:45
@stefanprodan
Copy link
Member

@squaremo I discovered that not all files inside the templates dir are ignored. For example a PodDisruptionBudget gets applied even if it's part of a chart.

apiVersion: policy/v1beta1
kind: PodDisruptionBudget
metadata:
  name: {{ template "memcached.fullname" . }}
spec:
  selector:
    matchLabels:
      app: {{ template "memcached.fullname" . }}
      chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
      release: "{{ .Release.Name }}"
      heritage: "{{ .Release.Service }}"
  minAvailable: {{ .Values.pdbMinAvailable }}

With the above file, Flux will run into this error:

ts=2018-05-07T08:44:46.764045009Z caller=loop.go:89 component=sync-loop err="loading resources from repo: parsing file at \"/tmp/flux-working640881799/repo/charts/memcached/templates/pdb.yaml\": parsing YAML doc from \"charts/memcached/templates/pdb.yaml\": yaml: unmarshal errors:\n  line 4: cannot unmarshal !!map into string"

A chart example can be found here: https://github.com/stefanprodan/weave-flux-helm-demo/tree/master/charts/memcached

@squaremo
Copy link
Member Author

squaremo commented May 8, 2018

@squaremo I discovered that not all files inside the templates dir are ignored. For example a PodDisruptionBudget gets applied even if it's part of a chart.

It's not getting applied. It is being considered as a resource though, which is still a problem. There is more than one way to load manifests from files, and (evidently) not all of them skip over chart directories. I'm going to log this as another issue.

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

Successfully merging this pull request may close these issues.

3 participants