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

HelmChart CRD doesn't support objects in `spec.set` #276

Open
mxplusb opened this issue Mar 28, 2019 · 5 comments
Open

HelmChart CRD doesn't support objects in `spec.set` #276

mxplusb opened this issue Mar 28, 2019 · 5 comments

Comments

@mxplusb
Copy link

@mxplusb mxplusb commented Mar 28, 2019

Describe the bug

With this manifest at `/var/lib/rancher/k3s/server/manifests/sql.yaml:

apiVersion: k3s.cattle.io/v1
kind: HelmChart
metadata: 
  name: sql
  namespace: kube-system
spec:
  chart: stable/mssql-linux
  targetNamespace: aperture-system
  set:
    acceptEula:
      value: "y"
    edition:
      value: Express
    collation: SQL_Latin1_General_CP1_CI_AS
    lcid: 1033
    hadr: 0
    image:
      repository: microsoft/mssql-server-linux
      tag: 2017-CU5
      pullPolicy: IfNotPresent
    service:
      headless: false
      type: ClusterIP
      port: 1433

k3s does not deploy the chart. This manifest works:

apiVersion: k3s.cattle.io/v1
kind: HelmChart
metadata: 
  name: sql
  namespace: kube-system
spec:
  chart: stable/mssql-linux
  targetNamespace: aperture-system
  valuesContent: |-
    acceptEula:
      value: "y"
    edition:
      value: Express
    collation: SQL_Latin1_General_CP1_CI_AS
    lcid: 1033
    hadr: 0
    image:
      repository: microsoft/mssql-server-linux
      tag: 2017-CU5
      pullPolicy: IfNotPresent
    service:
      headless: false
      type: ClusterIP
      port: 1433

To Reproduce

  1. Create sql.yaml from first manifest.
  2. /usr/local/bin/k3s-uninstall.sh
  3. curl -sfL https://get.k3s.io | sh -
  4. Deploy the local storage provisioner so the chart will deploy.
    1. kubectl apply -f https://raw.githubusercontent.com/rancher/local-path-provisioner/master/deploy/local-path-storage.yaml
    2. kubectl patch storageclass local-path -p '{"metadata": {"annotations":{"storageclass.kubernetes.io/is-default-class":"true"}}}'
  5. cp sql.yaml /var/lib/rancher/k3s/server/manifests/

Expected behavior

I expect the HelmChart CRD to pick up the chart and deploy it.

Additional context

Looking at the spec, there's a Set field:

type HelmChartSpec struct {
	TargetNamespace string                        `json:"targetNamespace,omitempty"`
	Chart           string                        `json:"chart,omitempty"`
	Version         string                        `json:"version,omitempty"`
	Repo            string                        `json:"repo,omitempty"`
	Set             map[string]intstr.IntOrString `json:"set,omitempty"`
	ValuesContent   string                        `json:"valuesContent,omitempty"`
}

And it is supported, but I'm not sure why it's not working as intended. Unfortunately I can't find the logs or I would share them.

@erikwilson

This comment has been minimized.

Copy link
Member

@erikwilson erikwilson commented Mar 28, 2019

I think it may support only key: value pairs instead of objects.

@erikwilson

This comment has been minimized.

Copy link
Member

@erikwilson erikwilson commented Mar 29, 2019

Some more info, the set spec works as described, but if we try to flatten the keys for set using yaml and something like:

apiVersion: k3s.cattle.io/v1
kind: HelmChart
metadata: 
  name: sql
  namespace: kube-system
spec:
  chart: stable/mssql-linux
  targetNamespace: aperture-system
  set:
    acceptEula.value: y
    edition.value: Express
    collation: SQL_Latin1_General_CP1_CI_AS
    lcid: 1033
    hadr: 0
  valuesContent: |-
    image:
      repository: microsoft/mssql-server-linux
      tag: 2017-CU5
      pullPolicy: IfNotPresent
    service:
      headless: false
      type: ClusterIP
      port: 1433

The above will never deploy because of dot notation quoting. :(

If it is converted to json:

{
	"apiVersion": "k3s.cattle.io/v1",
	"kind": "HelmChart",
	"metadata": {
		"name": "sql",
		"namespace": "kube-system"
	},
	"spec": {
		"chart": "stable/mssql-linux",
		"targetNamespace": "aperture-system",
		"set": {
			"acceptEula.value": "y",
			"edition.value": "Express",
			"collation": "SQL_Latin1_General_CP1_CI_AS",
			"lcid": 1033,
			"hadr": 0
		},
		"valuesContent": "image:\n  repository: microsoft/mssql-server-linux\n  tag: 2017-CU5\n  pullPolicy: IfNotPresent\nservice:\n  headless: false\n  type: ClusterIP\n  port: 1433"
	}
}

Strangely enough that will deploy.

It may be possible to use some escaping in the yaml to fix the behavior, but I am going to mark this is an enhancement to support nested keys for set, perhaps we can flatten the object before to generate the --set args.

@erikwilson erikwilson changed the title HelmChart CRD doesn't support `spec.set` HelmChart CRD doesn't support objects in `spec.set` Mar 29, 2019
@mxplusb

This comment has been minimized.

Copy link
Author

@mxplusb mxplusb commented Mar 29, 2019

Ah, yeah, I missed that it didn't support nested keys, which would explain my results. Good to know for sure. :)

@erikwilson

This comment has been minimized.

Copy link
Member

@erikwilson erikwilson commented Mar 29, 2019

I was able to get it to work using only set, but if the value isn't an int or string it needs to be quoted ("y" and "false"):

apiVersion: k3s.cattle.io/v1
kind: HelmChart
metadata: 
  name: sql
  namespace: kube-system
spec:
  chart: stable/mssql-linux
  targetNamespace: aperture-system
  set:
    acceptEula.value: "y"
    edition.value: Express
    collation: SQL_Latin1_General_CP1_CI_AS
    lcid: 1033
    hadr: 0
    image.repository: microsoft/mssql-server-linux
    image.tag: 2017-CU5
    image.pullPolicy: IfNotPresent
    service.headless: "false"
    service.type: ClusterIP
    service.port: 1433
@denysvitali

This comment has been minimized.

Copy link

@denysvitali denysvitali commented Dec 2, 2019

I can confirm that this is an ongoing issue: I'm trying to deploy an Helm chart that checks on a boolean value to deploy a certain resource (here is the check) but since I cannot specify any boolean value without having it converted to string, I cannot go to the else branch of that control expression because if ( string ) is true, no matter what's the string value.

valuesContent seems to work though, but obviously the original values.yml and the one from valuesContent aren't merged. Bummer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.