knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.58k stars 1.16k forks source link

Digest resolution not working? #11317

Closed xclud closed 3 years ago

xclud commented 3 years ago

/area build

I am using knative-serving along with tekton and kaniko. In a multi step Task my image is getting built and pushed to a private registry (yes, it supports api version 2) using kaniko. Then the next step runs a kn command to create a new Revision for the KService.

Unfortunately in the last step kn does resolve the digest and logs Service 'develop' with latest revision 'develop-00003' (unchanged) is available at URL:. However, when I run docker pull ... in the node CLI, kn creates a new revision (if triggered).

I have read this doc https://knative.dev/docs/serving/tag-resolution/ and i am sure i did not enable the digest resolution skipping.

I appreciate if you shed some light on this. Or have i just found a bug?

julz commented 3 years ago

Hi @xclud - I may be misunderstanding, but the message above sounds like the deploy succeeded, did you mean to say "doesn't resolve"? Could you post the full cli invocation and response (the whole thing, not just the last line), and also the output of "kubectl describe ksvc develop" please? It would also be helpful to know the knative version you are using.

Thanks!

xclud commented 3 years ago

@julz Thanks for the quick response. Yes the deployment is successful. Image is pushed successfully and kn works but it does not create a new Revision (due to the unchanged docker image !!!!?).

I am expecting revision 00004 but it remains on 00003.

Name:         develop
Namespace:    sun-frontend
Labels:       <none>
Annotations:  serving.knative.dev/creator: kubernetes-admin
              serving.knative.dev/lastModifier: kubernetes-admin
API Version:  serving.knative.dev/v1
Kind:         Service
Metadata:
  Creation Timestamp:  2021-05-06T05:56:23Z
  Generation:          3
  Managed Fields:
    API Version:  serving.knative.dev/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:status:
        .:
        f:address:
          .:
          f:url:
        f:conditions:
        f:latestCreatedRevisionName:
        f:latestReadyRevisionName:
        f:observedGeneration:
        f:traffic:
        f:url:
    Manager:      controller
    Operation:    Update
    Time:         2021-05-06T05:56:30Z
    API Version:  serving.knative.dev/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:kubectl.kubernetes.io/last-applied-configuration:
      f:spec:
        .:
        f:template:
          .:
          f:metadata:
            .:
            f:annotations:
              .:
              f:client.knative.dev/user-image:
            f:creationTimestamp:
          f:spec:
            .:
            f:containers:
    Manager:         kn
    Operation:       Update
    Time:            2021-05-06T11:50:09Z
  Resource Version:  16009570
  UID:               440d25b8-1106-49c3-a038-0477002f5ba0
Spec:
  Template:
    Metadata:
      Annotations:
        client.knative.dev/user-image:  docker.domainname.tld/pna/sun-frontend
      Creation Timestamp:               <nil>
    Spec:
      Container Concurrency:  0
      Containers:
        Image:  docker.domainname.tld/pna/sun-frontend
        Name:   user-container
        Ports:
          Container Port:  80
        Readiness Probe:
          Success Threshold:  1
          Tcp Socket:
            Port:  0
        Resources:
      Enable Service Links:  false
      Timeout Seconds:       300
  Traffic:
    Latest Revision:  true
    Percent:          100
Status:
  Address:
    URL:  http://develop.sun-frontend.svc.cluster.local
  Conditions:
    Last Transition Time:        2021-05-06T11:50:27Z
    Status:                      True
    Type:                        ConfigurationsReady
    Last Transition Time:        2021-05-06T11:50:28Z
    Status:                      True
    Type:                        Ready
    Last Transition Time:        2021-05-06T11:50:28Z
    Status:                      True
    Type:                        RoutesReady
  Latest Created Revision Name:  develop-00003
  Latest Ready Revision Name:    develop-00003
  Observed Generation:           3
  Traffic:
    Latest Revision:  true
    Percent:          100
    Revision Name:    develop-00003
  URL:                http://develop.sun-frontend.alpha.domainname.tld
Events:               <none>

xclud commented 3 years ago
E0508 11:11:28.225062      16 aws_credentials.go:77] while getting AWS credentials NoCredentialProviders: no valid providers in chain. Deprecated.
    For verbose messaging see aws.Config.CredentialsChainVerboseErrors
INFO[0006] Retrieving image manifest nginx:stable-alpine 
INFO[0006] Retrieving image nginx:stable-alpine from registry index.docker.io 
INFO[0009] Built cross stage deps: map[]                
INFO[0009] Retrieving image manifest nginx:stable-alpine 
INFO[0009] Returning cached image manifest              
INFO[0009] Executing 0 build triggers                   
INFO[0009] Unpacking rootfs as cmd COPY ./web /usr/share/nginx/html requires it. 
INFO[0013] COPY ./web /usr/share/nginx/html             
INFO[0013] Taking snapshot of files...                  
INFO[0014] Pushing image to 
INFO[0014] Pushed image to 1 destinations  

Step completed
No changes to apply to service 'develop'.
Service 'develop' with latest revision 'develop-00003' (unchanged) is available at URL:
http://develop.sun-frontend...

Step completed
julz commented 3 years ago

Ah, I see. I think this may be working as intended: the local ksvc yaml is unchanged so the client didn't do anything (it does not check the digest, it only checks if the image name changed). There's a cli issue discussing changing this in https://github.com/knative/client/issues/398.

@rhuss may be able to share the best cli workflow to handle this case

/assign @rhuss

xclud commented 3 years ago

I came up with the idea of generating a UUID as an environment variable and pass it through the steps. My setup is:

stepTemplate:
  env:
    - name: UUID
      value: $(uuidgen)

steps:
  - echo $(UUID) # this prints `$(uuidgen)` but desired output is `b4694733-1643-4967-9e0a-44dc3876d333`
  - kaniko push http://repo/image:$(UUID)
  - kn service update --image http://repo/image:$(UUID)

But as in pseudo-code above, it prints $(uuidgen) but the desired output is b4694733-1643-4967-9e0a-44dc3876d333.

Do you have any recommendation or workaround this problem? Any other solution do you suggest?

xclud commented 3 years ago

I ended up using the checkout_sha from the gitlab trigger. It is now:

steps:
  - kaniko push http://repo/image:$(tt.params.gitrevision)
  - kn service update --image http://repo/image:$(tt.params.gitrevision)

Still i would like to know if the solution in my above comment is possible and how. Thanks.

markusthoemmes commented 3 years ago

Using a stable sha is imo the definitive answer here. Is it possible for you to obtain the sha of the built image? You could then define the image as http://repo/image@sha256:$IMAGE_SHA, which would always work as expected and guarantees that you get that exact image in the deployment.

xclud commented 3 years ago

I did try the imagedigestexporter tool. But the workflow looks odd to me and not very straightforward. Is there any other tool to get the sha of the image?

markusthoemmes commented 3 years ago

I bat-signalled some Tekton folks to comment on the image SHA thing. FWIW, your current approach with the Git SHA is fine as well most likely, if you expect the image produced from a specific commit to always be the same.

xclud commented 3 years ago

Thank you.

if you expect the image produced from a specific commit to always be the same.

Yes this is the case and it works very well.

nikhil-thomas commented 3 years ago

@xclud @markusthoemmes

is Tekton Task's Results feature what you are looking for ? :) https://tekton.dev/docs/pipelines/tasks/#emitting-results

xclud commented 3 years ago

I already tried the Task results. I get the results but i don't know what to do next with the them. It seems the results are useful to pass data from Task-to-Task, not from Step-to-Step. Or am i getting it wrong?

nikhil-thomas commented 3 years ago

it should work in both cases. if we are writing sha to /tekton/results/<sha-result-name> file, from one step, then we can read the sha from another step from the same/tekton/results/<sha-result-name> file. in other words, from the context of a step (container) every result is a file in /tekton/results

infact, you can use the variable expansion, $(results.<result-name>.path).

xclud commented 3 years ago

Thank you @nikhil-thomas. This solves my problem.

rhuss commented 3 years ago

@xclud sorry for being late to the game. Your use case is very valid and we have some support from the client side to force an update of the image from the client side:

However, the real solution is probably a fix on the server-side that allows the user to specify the behaviour for images updates on an UPDATE API (aka POST) for an Knative service. Should the image be checked for a new version or not, if the image name has not changed in that update call ? I believe the proper solution would be to introduce a similar concept like for ImagePullPolicy in the container spec, or maybe even reuse this for this purpose. I don't even know if ImagePullPolicy has any effect for a container spec in a Knative context.

There is an ongoing discussion around this issue (see the latest Client WG call), so you might want to keep an eye on https://github.com/knative/client/issues/1329 and https://github.com/knative/client/issues/1318 .

xclud commented 3 years ago

I believe the proper solution would be to introduce a similar concept like for ImagePullPolicy

@rhuss I strongly agree with the concept you are explaining. But there is one little note: ImagePullPolicy is used to really re-pull the image from the repository. My suggestion is not to re-pull the image. It's behavior should be related to the creation a new revision. I would use the name RevisionCreationPolicy with possible values of CreateAnyways and SkipIfExists.