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

imageStrategy Helm broken since v0.38, possibly v0.37 #2902

Closed
MisterTimn opened this issue Sep 18, 2019 · 9 comments · Fixed by #2887
Closed

imageStrategy Helm broken since v0.38, possibly v0.37 #2902

MisterTimn opened this issue Sep 18, 2019 · 9 comments · Fixed by #2887
Assignees
Labels
area/deploy deploy/helm kind/bug Something isn't working priority/p1 High impact feature/bug.

Comments

@MisterTimn
Copy link

MisterTimn commented Sep 18, 2019

Expected behavior

Skaffold dev or run resulting in deployed and running pods

Actual behavior

InvalidImageName errors on pods because the imageStrategy: helm option on skaffold config isnt respected
Actual image name becomes: imageName:imageName instead of imageName:tag

Tested on multiple of my projects. I then reverted to 0.36 because I knew this was the last version on which my deployments worked, and there I had no issues. (Yes I ran skaffold fix --overwrite true before I ran skaffold dev/run)

Information

  • Skaffold version: version 0.38
  • Operating system: Manjaro Arch
  • Contents of skaffold.yaml:
apiVersion: skaffold/v1beta14
kind: Config
build:
  artifacts:
  - image: gitlab.ilabt.imec.be:4567/dyversify/demo-controller
    context: .
    jib: {}
deploy:
  helm:
    releases:
    - name: skaffold-demo-controller
      chartPath: helm/demo-controller
      valuesFiles:
      - helm/dyversify-values.yaml
      values:
        image: gitlab.ilabt.imec.be:4567/dyversify/demo-controller
      namespace: default
      recreatePods: true
      imageStrategy:
        helm: {}
profiles:
- name: obelisk-staging
  patches:
  - op: replace
    path: /deploy/helm/releases/0/valuesFiles/0
    value: helm/obelisk-staging-values.yaml
  activation:
  - kubeContext: obelisk-staging
- name: dyversify
  patches:
  - op: replace
    path: /deploy/helm/releases/0/valuesFiles/0
    value: helm/dyversify-values.yaml
  activation:
  - kubeContext: dyversify

Steps to reproduce the behavior

Cant expose my codebase, but basically run any project which uses the default Helm config

containers:
        - name: {{ .Chart.Name }}
          image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"

And use in your skaffold.yaml

 deploy:
  helm:
      imageStrategy:
        helm: {}

and it will give you the double imageNames resulting in an error.

@tejal29 tejal29 added kind/bug Something isn't working priority/p0 Highest priority. We are actively looking at delivering it. area/deploy deploy/helm labels Sep 18, 2019
@forgo
Copy link

forgo commented Sep 18, 2019

My team is also experiencing this issue.

@dmmatson
Copy link

Can confirm that rolling back from 0.38.0 to 0.37.1 fixes this issue for me.

@tejal29
Copy link
Contributor

tejal29 commented Sep 18, 2019

Thanks @forgo and @dmmatson for confirming the issue. The team will look in to this soon. For now, please revert to 0.37.1

@tejal29 tejal29 self-assigned this Sep 18, 2019
@tejal29
Copy link
Contributor

tejal29 commented Sep 18, 2019

I was able to reproduce this bug on my end. Will work on a fix.

@tejal29
Copy link
Contributor

tejal29 commented Sep 18, 2019

#2887 fixes this issue. I am going to add this to our integration test suite.

@georgekarrv
Copy link

I had to go all the way back to 0.35.0 to get correct imagenames

@MisterTimn
Copy link
Author

Might be unrelated/other issue then? I can also confirm that 0.37.1 works for me. Using it right now until next release.

Do you get the double name same as I had or other image names?

@georgekarrv
Copy link

well i checked out master and applied the diff of the above pr and it works for me so that's what I'll stick with until an actual release

@tejal29
Copy link
Contributor

tejal29 commented Sep 19, 2019

Thanks @georgekarrv, Just an update, i am working with @michaelbeaumont to get his PR merged in soon.

@tejal29 tejal29 added priority/p1 High impact feature/bug. and removed priority/p0 Highest priority. We are actively looking at delivering it. labels Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/deploy deploy/helm kind/bug Something isn't working priority/p1 High impact feature/bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants