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

Setting Helm deployer imageStrategy.helm does not have any effect #2681

Closed
aeneasr opened this issue Aug 19, 2019 · 6 comments
Closed

Setting Helm deployer imageStrategy.helm does not have any effect #2681

aeneasr opened this issue Aug 19, 2019 · 6 comments

Comments

@aeneasr
Copy link

aeneasr commented Aug 19, 2019

Expected behavior

Using helm.ImageStrategy.helm: {} should set the right repository name and tag for image.repository and image.tag when using the helm deployer

Actual behavior

It appears that #826 should have closed #360 . However, using helm.ImageStrategy.helm: {} does not work at all because neither image.repository nor image.tag are being overwritten - they just keep their defaults from the original values.yaml file.

Seeing the skaffold.yaml file from below, I would expect:

  • console to use gcr.io/ory-artifacts/console:<some-hash>-dirty
  • oathkeeper to use gcr.io/ory-artifacts/oathkeeper:<some-hash>-dirty

However, that's not the case - the default values are being used instead:

  • console uses gcr.io/ory-artifacts/console:stable
  • oathkeeper uses oryd/oathkeeper:v0.17.1-beta.1

Information

  • Skaffold version: 0.36.0
  • Operating system: macOS
  • Contents of skaffold.yaml:
apiVersion: skaffold/v1beta13
kind: Config
build:
  local:
    useDockerCLI: false
  artifacts:
    # The order of these needs to stay the same
    -
      image: gcr.io/ory-artifacts/console
      context: console
      docker:
        dockerfile: Dockerfile-skaffold

    -
      image: gcr.io/ory-artifacts/oathkeeper
      context: skaffold/oathkeeper
      docker:
        dockerfile: Dockerfile

deploy:
  helm:
    flags:
      global: ["--tiller-namespace","backoffice"]
    releases:
      -
        name: console
        chartPath: helm/charts/console
        namespace: backoffice
        recreatePods: true
        valuesFiles:
          - skaffold/console/values.yaml
        imageStrategy:
          helm: {}
      -
        name: oathkeeper
        chartPath: ory/oathkeeper
        namespace: backoffice
        remote: true # required for remote charts
        skipBuildDependencies: true # required for remote charts
        recreatePods: true
        valuesFiles:
          - skaffold/oathkeeper/values.yaml
        imageStrategy:
          helm: {}

Steps to reproduce the behavior

Not possible at the moment but happy to provide something if it speeds up the process.

@aeneasr
Copy link
Author

aeneasr commented Aug 19, 2019

Wow, what an odysee trying to figure out the right combination of config values to get this working. Apparently, explicitly setting values.image resolves this issue.

works


deploy:
  helm:
    flags:
      global: ["--tiller-namespace","backoffice"]
    releases:
      -
        name: console
        chartPath: helm/charts/console
        namespace: backoffice
        recreatePods: true
        valuesFiles:
          - skaffold/console/values.yaml
        values:
          image: gcr.io/ory-artifacts/console
        imageStrategy:
          helm: {}

does not work

deploy:
  helm:
    flags:
      global: ["--tiller-namespace","backoffice"]
    releases:
      -
        name: console
        chartPath: helm/charts/console
        namespace: backoffice
        recreatePods: true
        valuesFiles:
          - skaffold/console/values.yaml
# Without this repo and tag are not correct
#        values:
#          image: gcr.io/ory-artifacts/console
        imageStrategy:
          helm: {}

@balopat
Copy link
Contributor

balopat commented Aug 19, 2019

Thank you for filing this and sharing the working solution. I agree, this is not intuitive. At least we should document it more clearly but it looks like maybe we can get rid of the requirement to set values.image.

@aeneasr
Copy link
Author

aeneasr commented Aug 19, 2019

Yes, that sounds like a good idea :) And imageStrategy.helm: {} should also be documented as the not-nil/nil of yaml is not very obvious for non-go devs.

@oscar-martin
Copy link

I have similar situation to the one described here but when I try to apply the suggested workaround, the tag becomes the name of the image:

build:
  local:
    push: false
  artifacts:
  - image: myprivaterepo:5003/repo/controller
    custom:
      buildCommand: ./sk-build.sh
      dependencies:
        paths:
          - .
        ignore:
          - "*.md"
deploy:
  helm:
    releases:
    - name: skaffold-controller
      chartPath: charts/controller
      valuesFiles:
        - charts/controller/values.yaml
      values:
        image: myprivaterepo:5003/repo/controller
      imageStrategy:
        helm: {}

This is what helm is said to use:

$ helm get skaffold-controller

USER-SUPPLIED VALUES:
affinity: {}
...
fullnameOverride: ""
image:
  pullPolicy: IfNotPresent
  pullSecrets: []
  repository: myprivaterepo:5003/repo/controller
  tag: controller

So tag is the name of the image instead of being the actual tag.

@aeneasr I am wondering why I get different behavior from yours.

$ skaffold version                        
v0.36.0

@dgageot dgageot self-assigned this Aug 30, 2019
@dgageot
Copy link
Contributor

dgageot commented Aug 30, 2019

Hi @aeneasr and @oscar-martin, I wonder if there's any way you can test #2772 and see if solves your issue.

@dgageot
Copy link
Contributor

dgageot commented Sep 4, 2019

For me, this is fixed in @2772. Please reopen in case you disagree. Maybe there's more things to be fixed

@dgageot dgageot closed this as completed Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants