How to add additional images and helm deployments with override via patch?

[quick-question]

I'm attempting to use profiles to facilitate building and deploying multiple apps (using helm charts) at the same time. For example, skaffold run -p app1,app2 but I can't seem to get it to work.

The profiles each look like this:

- name: app1
  patches:
    - op: add
      path: /build/artifacts/-
      value:
        - image: app1
          context: ./app1
    - op: add
      path: /deploy/helm/releases/-
      value:
        - name: app1
          chartPath: ./app1/helm/app1-chart
          values:
            imageName: "app1"

Can anyone tell me what I am doing wrong or provide an example?

[nkubala]

hey @quick-question. this behavior should be supported in skaffold - can you try doing skaffold run -p app1 -p app2 instead of a comma-separated list?

[quick-question]

That does not seem to work either. I am getting this error:
time="2019-09-03T15:23:04-04:00" level=fatal msg="creating runner: applying profiles: applying profile app1: invalid path: /build/artifacts/-"

I'll also mention that this is occurring on v0.35.0

[quick-question]

@nkubala Could you provide an example of how someone might have two profiles that add their artifacts and releases via override by patch? I haven't been able to find a good example in the docs.

[nkubala]

it looks like the profile behavior is working as intended here - the problem is that /build/artifacts/- isn't a valid JSON path, so the patch applier can't update the object. can you try removing the - at the end of your paths and see if that fixes your issue?

ah, ok nevermind my JSON patch knowledge was a little off, I realize that the trailing - is supposed to put that value at the end of the array. we're using a yamlpatch library that's supposed to be a yaml implementation of JSON patches, but it looks like this library doesn't support the hyphens in the path :(

are you depending on the ordering of your build/deploy artifacts? if not you should just be able to remove that hyphen and get the same results.

[quick-question]

Hmm according to http://jsonpatch.com/ that should be a valid:

Finally, if you need to refer to the end of an array you can use - instead of an index.

Is that not supported in this case?

[quick-question]

are you depending on the ordering of your build/deploy artifacts? if not you should just be able to remove that hyphen and get the same results.

Great, I'll give that a try tomorrow and see if that works!

[quick-question]

Sorry for the delay. I will be trying this out later today.

[quick-question]

Well sorry again for the delay!

@nkubala Unfortunatley, removing the hyphens did not work.
Here is the error I'm getting:
time="2019-09-18T15:44:17-04:00" level=fatal msg="creating runner: applying profiles: applying profile app1: invalid path: /build/artifacts/"

[prokher]

I suppose I have an issue very close to this one. Consider the following skaffold.yaml:

apiVersion: skaffold/v1beta15
kind: Config
build:
    tagPolicy: {}
profiles:
    - name: preview
      patches:
          - op: replace
            path: /build/tagPolicy
            value:
                gitCommit: {}

When I run:

> skaffold -v debug build --profile=preview

The output is the following:

INFO[0000] Skaffold &{Version:v0.39.0 ConfigVersion:skaffold/v1beta15 GitVersion: GitCommit:4145d76cbccd0ea46ee31541f5f550368e257fa3 GitTreeState:clean BuildDate:2019-09-26T23:53:09Z GoVersion:go1.12.10 Compiler:gc Platform:linux/amd64} 
INFO[0000] applying profile: preview                    
DEBU[0000] overlaying profile on config for field Build 
DEBU[0000] overlaying profile on config for field artifacts 
DEBU[0000] overlaying profile on config for field insecureRegistries 
DEBU[0000] overlaying profile on config for field tagPolicy 
INFO[0000] no values found in profile for field TagPolicy, using original config values 
DEBU[0000] overlaying profile on config for field BuildType 
INFO[0000] no values found in profile for field BuildType, using original config values 
DEBU[0000] overlaying profile on config for field Test  
DEBU[0000] overlaying profile on config for field Deploy 
DEBU[0000] overlaying profile on config for field statusCheckDeadlineSeconds 
DEBU[0000] overlaying profile on config for field DeployType 
INFO[0000] no values found in profile for field DeployType, using original config values 
DEBU[0000] overlaying profile on config for field PortForward 
FATA[0000] creating runner: applying profiles: applying profile preview: invalid path: /build/tagPolicy 

As a result I simply cannot make Skaffold profiles work. What I am trying to achieve is to have different tagging strategies in different profiles.

[quick-question]

Can we at least get an example of how to use multiple profiles in the documentation?

[ghost]

Any updates on this?

[agarciatome]

I am having the same issue. This seems to be a bug. The syntax works if the array is not empty though.
Json patch documentation states: "The - character can be used instead of an index to insert at the end of an array." and an empty array should not be an exception..

[nkubala]

ok, I think I puzzled this one out. the short answer is that there's an issue in the yamlpatch library we're using, but if you care about an explanation and then possible workarounds, read on...

we use the yamltag omitempty in our schemas to prevent providing nil for fields that weren't provided by the user. this is fine, but causes the yamlpatch library to freak out when you try and patch something to a field that you didn't already provide a default value for.

@prokher was close in his example of trying to specify a dummy value for the tagPolicy before overriding it with a profile patch. but, you missed one level of nesting in the field and didn't actually provide a value :)

what you actually want is

build:
  tagPolicy:
    dateTime: {}

and then your example will work.

apiVersion: skaffold/v1beta15
kind: Config
build:
    tagPolicy:
      dateTime: {}
profiles:
    - name: preview
      patches:
          - op: replace
            path: /build/tagPolicy
            value:
                gitCommit: {}

alternatively, you can use the add directive instead of replace and it will work for a nil value in your original skaffold.yaml:

apiVersion: skaffold/v1beta15
kind: Config
build:
    tagPolicy: {}
profiles:
    - name: preview
      patches:
          - op: add
            path: /build/tagPolicy
            value:
                gitCommit: {}

@quick-question as for combining artifacts, I was able to get this to work on version 1.1.0 using this yaml:

profiles:
    - name: tag1
      patches:
      - op: add
        path: /build/artifacts/-
        value:
          image: gcr.io/nkubala-demo/image1
          context: . # this is just to show that it unmarshals correctly
    - name: tag2
      patches:
      - op: add
        path: /build/artifacts/-
        value:
          image: gcr.io/nkubala-demo/image2

applying this to the getting-started example in the repo:

➜  getting-started git:(master) ✗ skaffold build -p tag1 -p tag2

Generating tags...
 - gcr.io/k8s-skaffold/skaffold-example -> gcr.io/nkubala-demo/gcr.io/k8s-skaffold/skaffold-example:v1.0.0-270-g19a4dec1b-dirty
 - gcr.io/nkubala-demo/image1 -> gcr.io/nkubala-demo/image1:v1.0.0-270-g19a4dec1b-dirty
 - gcr.io/nkubala-demo/image2 -> gcr.io/nkubala-demo/image2:v1.0.0-270-g19a4dec1b-dirty
Checking cache...
 - gcr.io/k8s-skaffold/skaffold-example: Found Remotely
 - gcr.io/nkubala-demo/image1: Found. Pushing
The push refers to repository [gcr.io/nkubala-demo/image1]
5498666ab1b1: Preparing
03901b4a2ea8: Preparing
03901b4a2ea8: Layer already exists
5498666ab1b1: Layer already exists
v1.0.0-270-g19a4dec1b-dirty: digest: sha256:d1e380117117bfd9a61cbccb1eb324e7b594f4ea2a1fb4e84e2ca6ee80238941 size: 739
 - gcr.io/nkubala-demo/image2: Found. Pushing
The push refers to repository [gcr.io/nkubala-demo/image2]
5498666ab1b1: Preparing
03901b4a2ea8: Preparing
5498666ab1b1: Layer already exists
03901b4a2ea8: Layer already exists
v1.0.0-270-g19a4dec1b-dirty: digest: sha256:d1e380117117bfd9a61cbccb1eb324e7b594f4ea2a1fb4e84e2ca6ee80238941 size: 739

so the hyphens do work as expected. but, we still have the issue of things breaking down when an empty value is provided through the original skaffold.yaml. @agarciatome I agree that an empty array should be no exception....but unfortunately it is because of a bug in the yamlpatch library, so this is out of our control. you'll need to provide at least one valid artifact in your top level skaffold.yaml.

hopefully this was helpful.

[kvokka]

Thank you @nkubala for a such a great answer!

But this answer means, that there is no way to solve it, because there is no corresponding issue in https://github.com/krishicks/yaml-patch repo

[prokher]

@nkubala Indeed, thank you for your great answer!

[agarciatome]

@nkubala thanks a lot for your detailed answer. I'll try to work around my scenario then!

[nkubala]

@kvokka unfortunately that's correct. maybe you can follow up with an issue on that repo?

hopefully you all can find ways to work around this in your projects. providing any non-nil values will enable using the replace directive from a profile, so possibly using a placeholder value is the way to go. I'm going to close this issue for now, but if there's any movement on fixing the issue in the yamlpatch library, feel free to reopen!

[tcolgate]

Another workaround here is to have a profile that adds a default artifacts, and a second that strips it out. Unfortunately this doesn't interact well with -p, the profiles given on the cli are applied before the activations, so using -p to enable a profile will still fail (it works if you just activate all of them, but then you have to selectively disable profiles)