skaffold image does not have version.

[tejal29]

Expected behavior

In #2055, we added version string to label value like "skaffold-"
This was to get some metrics on which versions of skaffold are in used.
However, that is breaking users who use skaffold image which does not have version set
See here

Actual behavior

skaffold image shd have version set.

Information

• Skaffold version: version...
• Operating system: ...
• Contents of skaffold.yaml:

<paste your skaffold.yaml here>

Steps to reproduce the behavior

1. ...
2. ...

[corneliusweig]

@tejal29 As said in #2062, I don't think this is actually the case. Take a look at

docker run --rm -ti gcr.io/k8s-skaffold/skaffold:latest skaffold version \
  -o "{{.Version}}  {{.GitCommit}} {{.GitTreeState}}"

which prints

a117236  a1172365f8c099635277a46c20acfa1e1d6ed2a7 clean%

The version is therefore set with value a117236.

[Jonathan34]

docker image tagged v0.28.0 works fine btw.

[corneliusweig]

Btw, the reason why the version is not formatted as v0.28.0-125 is that cloudbuild does a shallow checkout (without tags, of course) from the Skaffold repo. Thus the whole git describe --tags does not work as expected, but prints the abbreviated commit sha instead. Probably not worth to fix this, as it would cause a lot of complexity.

[corneliusweig]

@tejal29 So I tested this locally with

docker run --rm -ti -v /var/run/docker.sock:/var/run/docker.sock \
  gcr.io/k8s-skaffold/skaffold:a1172365f8c099635277a46c20acfa1e1d6ed2a7 /bin/bash

When I run the getting-started example inside this container, I get the label app.kubernetes.io/managed-by: skaffold-a117236 as expected and not an error as reported by @Jonathan34 .

@Jonathan34 I think this must be a different problem, maybe connected to istio?

[Jonathan34]

@corneliusweig I saw that error happening for a standard deployment (and service and namespace object) too using 0.29:

Error from server (Invalid): error when applying patch:
{"metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"extensions/v1beta1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"app.kubernetes.io/managed-by\":\"skaffold-\",\"environment\":\"development\",\"skaffold.dev/builder\":\"google-cloud-build\",\"skaffold.dev/cleanup\":\"true\",\"skaffold.dev/deployer\":\"kustomize\",\"skaffold.dev/profiles\":\"gcb\",\"skaffold.dev/tag-policy\":\"git-commit\",\"skaffold.dev/tail\":\"true\"},\"name\":\"myservice\",\"namespace\":\"development\"},\"spec\":{\"selector\":{\"matchLabels\":{\"environment\":\"development\"}},\"template\":{\"metadata\":{\"labels\":{\"app\":\"myservice\",\"app.kubernetes.io/managed-by\":\"skaffold-\",\"environment\":\"development\",\"skaffold.dev/builder\":\"google-cloud-build\",\"skaffold.dev/cleanup\":\"true\",\"skaffold.dev/deployer\":\"kustomize\",\"skaffold.dev/profiles\":\"gcb\",\"skaffold.dev/tag-policy\":\"git-commit\",\"skaffold.dev/tail\":\"true\"}},\"spec\":{\"containers\":[{\"env\":[{\"name\":\"REDIS_ADDR\",\"value\":\"redis:6379\"},{\"name\":\"APP_PORT\",\"value\":\"50051\"},{\"name\":\"HTTP_PORT\",\"value\":\"8888\"},{\"name\":\"LISTEN_ADDR\",\"value\":\"0.0.0.0\"}],\"image\":\"gcr.io/<redacted>/myservice:62577b7@sha256:2d7b6d58d944b24da7f81bc1741d0639f9c8120b41b31e4592aec73b7ab76c2e\",\"livenessProbe\":{\"exec\":{\"command\":[\"/bin/grpc_health_probe\",\"-addr=:50051\"]},\"initialDelaySeconds\":15,\"periodSeconds\":10},\"name\":\"server\",\"ports\":[{\"containerPort\":50051},{\"containerPort\":8888}],\"readinessProbe\":{\"exec\":{\"command\":[\"/bin/grpc_health_probe\",\"-addr=:50051\"]},\"initialDelaySeconds\":15},\"resources\":{\"limits\":{\"cpu\":\"300m\",\"memory\":\"128Mi\"},\"requests\":{\"cpu\":\"200m\",\"memory\":\"64Mi\"}}},{\"command\":[\"/cloud_sql_proxy\",\"-instances=slb-it-ucs-prod:europe-west1:ucs-ponddb-ro=tcp:3306\",\"-credential_file=/credentials/credentials.json\"],\"image\":\"gcr.io/cloudsql-docker/gce-proxy:1.13\",\"name\":\"cloudsqlproxy\",\"securityContext\":{\"allowPrivilegeEscalation\":false,\"runAsUser\":2},\"volumeMounts\":[{\"mountPath\":\"/cloudsql\",\"name\":\"cloudsql\"},{\"mountPath\":\"/credentials\",\"name\":\"service-account-token\"}]}],\"terminationGracePeriodSeconds\":5,\"volumes\":[{\"emptyDir\":null,\"name\":\"cloudsql\"},{\"name\":\"service-account-token\",\"secret\":{\"secretName\":\"service-account-token\"}}]}}}}\n"},"labels":{"app.kubernetes.io/managed-by":"skaffold-"}},"spec":{"template":{"metadata":{"labels":{"app.kubernetes.io/managed-by":"skaffold-"}},"spec":{"$setElementOrder/containers":[{"name":"server"},{"name":"cloudsqlproxy"}],"$setElementOrder/volumes":[{"name":"cloudsql"},{"name":"service-account-token"}],"containers":[{"image":"gcr.io/<redacted>/myservice:62577b7@sha256:2d7b6d58d944b24da7f81bc1741d0639f9c8120b41b31e4592aec73b7ab76c2e","name":"server"}],"volumes":[{"$retainKeys":["name"],"emptyDir":null,"name":"cloudsql"}]}}}}
to:
Resource: "extensions/v1beta1, Resource=deployments", GroupVersionKind: "extensions/v1beta1, Kind=Deployment"
Name: "myservice", Namespace: "development"
Object: &{map["apiVersion":"extensions/v1beta1" "kind":"Deployment" "metadata":map["annotations":map["deployment.kubernetes.io/revision":"36" "kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"extensions/v1beta1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"app.kubernetes.io/managed-by\":\"skaffold-f25b09f\",\"environment\":\"development\",\"skaffold.dev/builder\":\"google-cloud-build\",\"skaffold.dev/cleanup\":\"true\",\"skaffold.dev/deployer\":\"kustomize\",\"skaffold.dev/profiles\":\"gcb\",\"skaffold.dev/tag-policy\":\"git-commit\",\"skaffold.dev/tail\":\"true\"},\"name\":\"myservice\",\"namespace\":\"development\"},\"spec\":{...}
for: "STDIN": Deployment.apps "myservice" is invalid: [metadata.labels: Invalid value: "skaffold-": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?'), spec.template.labels: Invalid value: "skaffold-": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')]

[corneliusweig]

@Jonathan34 is that deterministic or does it only happen once in a while? It would also help, if you could provide a minimal example where this happens.

[Jonathan34]

@Jonathan34 is that deterministic or does it only happen once in a while? It would also help, if you could provide a minimal example where this happens.

It happens every time. Can we do a hangout/zoom (or whatever else) ? That might be easier to illustrate and run some live debugging.

[tejal29]

@corneliusweig I didnt make it clear on #2062, i am fixing the bug where skaffold should not append a "-<version>" if version is empty.

Regarding why version is empty for @Jonathan34 i am not yet sure.

[balopat]

1. the issue with the skaffold image versions is the following:
   a.) gcr.io/k8s-skaffold/skaffold:latest is the bleeding edge, built by deploy/cloudbuild.yaml on every push to master. This is unusual, typically latest points to latest stable version. I think we should fix that: have latest point to last version at release time and have an edge version instead that is published on every push to master .
   b.) versioned images, e.g. gcr.io/k8s-skaffold/skaffold:v0.27work because deploy/cloudbuild-release.yaml actually explicitly passes in VERSION the github tag that skaffold compilation then picks up
2. for @Jonathan34's issue I would love to understand where the actual image is from - can you share the digest of the image? Also what happens if you pull the latest latest? Does the behavior change?

[Jonathan34]

agree with 1.

2. for @Jonathan34's issue I would love to understand where the actual image is from - can you share the digest of the image? Also what happens if you pull the latest latest? Does the behavior change?

i pulled latest (yesterday) and put the sha in that comment: #2062 (comment)

[corneliusweig]

🤦‍♂️ I should have checked more thoroughly: when using the exact image as @Jonathan34, the version is indeed emtpy:

docker run --rm -ti gcr.io/k8s-skaffold/skaffold@sha256:275894cbe96cbf2c5262bba710530acbad91bd8af329b6d6eea9ca6086d14642 \
   skaffold version -o "'{{.Version}}'   '{{.GitCommit}}'  '{{.BuildDate}}'  '{{.GitTreeState}}'  '{{.Platform}}'  "

yields

''   '63f353f05fd6258bb4a682e658bf5656e7f88a77'  '2019-05-08T16:47:13Z'  'clean'  'linux/amd64'  %

So the version is indeed unset.

Commit 63f353f is a merge with two parents, but that does not break git describe so that it outputs "".
Could there be any chance that the VERSION env variable is set to an empty string for the non-release cloudbuild?

[balopat]

Looking at the logs it looks like it was resolving to empty for some reason...
still looking, it's a bit puzzling:

 GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -ldflags " -extldflags \"\" -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.version= -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.buildDate=2019-05-08T16:47:13Z -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.gitCommit=63f353f05fd6258bb4a682e658bf5656e7f88a77 -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.gitTreeState=clean " -gcflags "all=-trimpath=/go/src/github.com/GoogleContainerTools/skaffold" -asmflags "all=-trimpath=/go/src/github.com/GoogleContainerTools/skaffold" -tags "kqueue" -o out/skaffold-linux-amd64 github.com/GoogleContainerTools/skaffold/cmd/skaffold

[corneliusweig]

@balopat I even tried a shallow git checkout, but that worked as expected (so no empty version string)

[balopat]

yeah, this is completely related to our cloudbuild/dockerfile mishap...
more info as I'm debugging:

on a "good" latest build we have:

Step #2: Step 29/32 : RUN make out/skaffold-linux-amd64 VERSION=$VERSION && mv out/skaffold-linux-amd64 /usr/bin/skaffold
Step #2: ---> Running in 89fde45d9bbb
Step #2: make: 'out/skaffold-linux-amd64' is up to date.

I.e. somehow the Dockerfile build picks up the result of the previous cross compilation (btw which is correctly copied to GCS with the correct version info in both the cases)

In the "wrong" version we have

Step #2: Step 29/32 : RUN make out/skaffold-linux-amd64 VERSION=$VERSION && mv out/skaffold-linux-amd64 /usr/bin/skaffold
Step #2: ---> Running in c3abdfb46676
Step #2: GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -ldflags " -extldflags \"\" -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.version= -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.buildDate=2019-05-08T16:47:13Z -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.gitCommit=63f353f05fd6258bb4a682e658bf5656e7f88a77 -X github.com/GoogleContainerTools/skaffold/pkg/skaffold/version.gitTreeState=clean " -gcflags "all=-trimpath=/go/src/github.com/GoogleContainerTools/skaffold" -asmflags "all=-trimpath=/go/src/github.com/GoogleContainerTools/skaffold" -tags "kqueue" -o out/skaffold-linux-amd64 github.com/GoogleContainerTools/skaffold/cmd/skaffold
Step #2: Removing intermediate container c3abdfb46676

because this line in the Dockerfile results to an empty value...

I understand when it doesn't work. Now I'm puzzled by how it can work at all, but the "caching" is hinting towards some kind of statefulness in our build process that I still have to wrap my head around... :D

[balopat]

the fix is in: #2099