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

[BUG] Trait patch fails on patchKey field with default value depending on syntax used #6510

Open
Kolossi opened this issue Apr 25, 2024 · 0 comments

Comments

@Kolossi
Copy link
Contributor

Kolossi commented Apr 25, 2024

Describe the bug

Patching trait fails to produce the correct output. This is under the very specific scenario of a trait definition using a parameter with a default value rather than one being specified in the trait usage in the application, using this as the value of a // +patchKey= field, AND the trait definition accessing the parameter with e.g. parameter.volumeName rather than "\(parameter.volumeName)"

To Reproduce

Steps to reproduce the behavior:

  1. Starting in some clean working directory, copy a kubeconfig file for a working cluster and store as ./.kube/config (e.g. in WSL, copy ~/.kube/docker.config)
  2. create a subdir ./kubevela
  3. Extract v1.9.9 webservice.cue from cluster/github repo to ./kubevela/webservice.cue
  4. Create a new trait ./kubevela/my-trait.cue with the following content:
"my-trait": {
    type: "trait"
}

template: {
    patch: spec: template: spec: {
        // +patchKey=name
        volumes: [{
            //name: "\(parameter.volumeName)" // works whether or not default value taken
            name: parameter.volumeName      // incorrect if default value taken
            hostPath: path: parameter.hostMountPath
        },]
    }

    parameter: {
        // +usage=mount path on host (default /var/run/datadog)
        hostMountPath: *"/etc/mydata" | string

        // +usage=name of host mount volume (default datadog)
        volumeName: *"my-default-vol" | string
    }
}
  1. Create ./repro/app.yml with the following content:
apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
  name: my-app
spec:
  components:
    - name: my-comp
      type: webservice
      properties:
        image: my/im
        volumeMounts:
          emptyDir:
            - name: myEmpty
              mountPath: "/tmp/myEmpty"
      traits:
        - type: my-trait
#          properties:                 #intentionally commented out initially
#            volumeName: my-vol        #intentionally commented out initially
  1. To exclude local software issues, use docker-based vela-cli to perform the dry-run, So create ./doit.sh with the following content:
#! /bin/bash

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

docker run --rm -it --name vela-cli -v $SCRIPT_DIR:/root/work -v $SCRIPT_DIR/.kube/config:/root/.kube/config oamdev/vela-cli:v1.9.9 dry-run -d /root/work/kubevela -f /root/work/app.yml --offline
  1. give doit.sh execute perms : chmod 755 doit.sh
  2. run ./doit.sh to produce dryrun manifest.
  3. note that manifest is:
apiVersion: apps/v1
kind: Deployment
metadata:
  annotations: {}
  labels:
    app.oam.dev/appRevision: ""
    app.oam.dev/component: my-comp
    app.oam.dev/name: my-app
    app.oam.dev/namespace: default
    app.oam.dev/resourceType: WORKLOAD
    workload.oam.dev/type: webservice
  name: my-comp
  namespace: default
spec:
  selector:
    matchLabels:
      app.oam.dev/component: my-comp
  template:
    metadata:
      labels:
        app.oam.dev/component: my-comp
        app.oam.dev/name: my-app
    spec:
      containers:
      - image: my/im
        name: my-comp
        volumeMounts:
        - mountPath: /tmp/myEmpty
          name: myEmpty
      volumes:
      - emptyDir:
          medium: ""
        hostPath:
          path: /etc/mydata
        name: myEmpty

Expected behavior

The volumes section of the above manifest should read:

      volumes:
      - emptyDir:
          medium: ""
        name: myEmpty
      - hostPath:
          path: /etc/mydata
        name: my-default-vol

ie each volume should be a separate list item, and should have the correct name: applied.

Note that correct output can be produced either by

  1. Change the trait definition to access the parameter.volumeName using the syntax name: "\(parameter.volumeName)" - this line is available commented out in the my-trait.cue given above.

  2. If the explicit volumeName property is passed to the trait in the app.yml, the correct output is also produced - these lines are available commented out in the app.yml given above.

and it still works if both the above changes are made. It's only use of a default value AND using the syntax name: parameter.volumeName that causes the problem.

KubeVela Version

The lowest oamdev/vela-cli image version this works with is v1.6.2, before that the dry-run fails. Up to v1.9.11 the same output problem occurs.

Cluster information

Not particularly relevant as doing dry-run --offline, but I'm using docker k8s currently v1.29.2

Additional context

Note that if the volumeMount is not added in the component, all works fine in any of the cases of defaulting parameter value or not, and whatever syntax is used in the trait definition. - it's only when combining the described conditions with multiple items in a patched list that things start to go wrong.

This issue took several days to track down!

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

No branches or pull requests

1 participant