dont create new revision if image is unchanged

[guillaumeblaquiere]

In what area(s)?

Classifications:
/kind bug

What version of Knative Client?

Version: v20190828-local-f53ebd8
Build Date: 2019-08-28 09:06:01
Git Revision: f53ebd8-dirty
Dependencies:

• serving: v0.6.0
• eventing: v0.8.0

What version of Knative Serving running on your cluster?

0.6.x

Expected Behavior

When I run the command kn service update XXX --image gcr.io/project/XXX I would like that the latest version of my container is deployed. The latest version must be automatically checked against the GCR full checksum and not with the short name in the command.

Actual Behavior

Nothing is performed because the kn tool check only the name of the image and not the tag of the latest version in the GCR.

Steps to Reproduce the Problem

• Run kn service create XXX --image gcr.io/project/XXX
• Deploy a new version in GCR
• Run kn service update XXX --image gcr.io/project/XXX
  -> No new revision created. But the container are really different.

Workaround

I alternate the command

• kn service update XXX --image gcr.io/project/XXX
• kn service update XXX --image gcr.io/project/XXX:latest

[navidshaikh]

Version: v20190828-local-f53ebd8
Build Date: 2019-08-28 09:06:01
Git Revision: f53ebd8-dirty
Dependencies:

serving: v0.6.0
eventing: v0.8.0

This doesn't look like built from latest master on 2019-08-28.

You can grab the nightly kn binary here https://github.com/knative/client/blob/master/docs/README.md#installing-kn.

kn has couple of flags added recently to configure the behavior to pull the image tag afresh with each new revision. You can flag --no-lock-to-digest to pull the image tag afresh with each new revision. Following flags are available during service create and update.

      --lock-to-digest           keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) (default true)
      --no-lock-to-digest        do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)

[guillaumeblaquiere]

Better but not perfect
Here my tests:

guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service create process-message --image gcr.io/gbl-imt-homerider-basguillaueb/process-me
ssage-gke -e TOPIC_PERIODIC=message-periodic -e TOPIC_EVENT=message-event -e GCP_PROJECT=gbl-imt-homerider-basguillaueb -e ALLOWED_PATTERN_ORANGE='.*((90DFFB(81|BD58|7367){1})|(5
322)).*'
Service 'process-message' successfully created in namespace 'default'.
Waiting for service 'process-message' to become ready ... OK
Service URL:
http://process-message.default.example.com
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service list
NAME              URL                                          GENERATION   AGE     CONDITIONS   READY   REASON
process-message   http://process-message.default.example.com   1            5m54s   3 OK / 3     True
pubsub-to-bq      http://pubsub-to-bq.default.example.com      3            11m     3 OK / 3     True
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service update process-message --image gcr.io/gbl-imt-homerider-basguillaueb/process-me
ssage-gke
Waiting for service 'process-message' to become ready ... OK
Service 'process-message' updated in namespace 'default'.
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service list
NAME              URL                                          GENERATION   AGE     CONDITIONS   READY   REASON
process-message   http://process-message.default.example.com   2            6m59s   3 OK / 3     True
pubsub-to-bq      http://pubsub-to-bq.default.example.com      3            12m     3 OK / 3     True
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service update process-message --image gcr.io/gbl-imt-homerider-basguillaueb/process-me
ssage-gke
Waiting for service 'process-message' to become ready ...
timeout: service 'process-message' not ready after 60 seconds
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn service list
NAME              URL                                          GENERATION   AGE     CONDITIONS   READY   REASON
process-message   http://process-message.default.example.com   3            9m12s   3 OK / 3     True
pubsub-to-bq      http://pubsub-to-bq.default.example.com      3            14m     3 OK / 3     True
guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$

There no container update in gcr

1. Create the service -> revision 1
2. Update -> revision 2 -> container digest is the same, should do nothing.
3. Update -> failed timeout 60s -> Finally deployed revision 3 -> container digest is the same, should do nothing

for information:

guillaume_blaquiere@cloudshell:~/knative-client-pilot (gbl-imt-homerider-basguillaueb)$ kn version
Version:      v20190904-local-2985e55
Build Date:   2019-09-04 06:07:49
Git Revision: 2985e55
Support:
- Serving: v0.8.0  v0.7.1
- API(s):  v1alpha1

[rusde]

As per the code kn client currently does not query for imageDigest. It just calls KNative API's to update service.

Does the proposal here is for kn client to query imageDigest parameters and do a diff with current KNative revision deployed. Only call KNative API if diff contains some properties?

As per I see it flag --lock-to-digest does allow customers to update other parameters without deploying latest image version. I see this as no issue. Thoughts?

[navidshaikh]

As per the code kn client currently does not query for imageDigest. It just calls KNative API's to update service.

+1

Does the proposal here is for kn client to query imageDigest parameters and do a diff with current KNative revision deployed. Only call KNative API if diff contains some properties?

I'd say this is something we should probably discuss with Serving WG, IMO this check belongs Serving side.

[rusde]

Opened serving issue

[sixolet]

I believe the current client behavior on master is that:

• When you specify an --image parameter to an update, we'll generate a name for you locally, by default
• By generating a new name locally, we force a new revision
• By forcing a new revision, Serving will re-resolve the image
• When you specify --image, any locked-to-digest image will be replaced with the image-by-tag again, which the server will re-resolve. If you don't specify --image, we will lock it to the digest if we're doing that for the service.

I think this does exactly what you want.

If you don't generate the revision name locally, you can end up with a noop change on the server, which will not force a new revision, and thus not re-resolve your tag.

[navidshaikh]

While switching from client side revision-name generation to server side revision-name generation, there is one revision created though

client-side revision-name generation

13:24 ➜  client git:(master)  ./kn service create hello --image gcr.io/knative-samples/helloworld-go
Service 'hello' successfully created in namespace 'default'.
Waiting for service 'hello' to become ready ... OK

Service URL:
http://hello.default.apps-crc.testing

13:24 ➜  client git:(master)  ./kn service update hello --image gcr.io/knative-samples/helloworld-go
Waiting for service 'hello' to become ready ... OK
Service 'hello' updated in namespace 'default'.

13:25 ➜  client git:(master)  ./kn revision list -s hello
NAME            SERVICE   GENERATION   AGE   CONDITIONS   READY   REASON
hello-vxndg-2   hello     2            21s   4 OK / 4     True    
hello-lgrjp-1   hello     1            39s   4 OK / 4     True    

switching to server-side revision-name generation + no-change in image, new revision generated

13:25 ➜  client git:(master)  ./kn service update hello --image gcr.io/knative-samples/helloworld-go --revision-name=
Waiting for service 'hello' to become ready ... OK
Service 'hello' updated in namespace 'default'.

13:25 ➜  client git:(master)  ./kn revision list -s hello                                                            
NAME            SERVICE   GENERATION   AGE   CONDITIONS   READY   REASON
hello-ntjq6     hello     3            18s   4 OK / 4     True    
hello-vxndg-2   hello     2            54s   4 OK / 4     True    
hello-lgrjp-1   hello     1            72s   4 OK / 4     True    

sub-sequent update + server-side revision-name generation + no-image-change, no new revision generated

13:25 ➜  client git:(master)  ./kn service update hello --image gcr.io/knative-samples/helloworld-go --revision-name=
Waiting for service 'hello' to become ready ... OK
Service 'hello' updated in namespace 'default'.

13:25 ➜  client git:(master)  ./kn revision list -s hello                                                            
NAME            SERVICE   GENERATION   AGE   CONDITIONS   READY   REASON
hello-ntjq6     hello     3            22s   4 OK / 4     True    
hello-vxndg-2   hello     2            58s   4 OK / 4     True    
hello-lgrjp-1   hello     1            76s   3 OK / 4     True  

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[navidshaikh]

/reopen
/remove-lifecycle stale

[knative-prow-robot]

@navidshaikh: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle stale

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[rhuss]

To reiterate on this issue, I see really two possible desired behaviours here, when --image is provided on update with the same image as the original one.

• A: The image has changed in the registry (different digest) --> Then a new revision should be created
• B: The image has not changed in the registry --> No new revision should be created.

When using BYO revision as we do now, then for every update we change the name locally which triggers the image-digest resolving algorithm on the server-side. However, even when the digest is the same a new revision is created as the client is in the driver seat and decided to create a new revision because it provided a new revision name. Using this with BYO revision names can fix this only if the client has a way to detect whether an image has been changed, by contacting the registry directly. This, however, is not possible for multiple reasons (network topology, authentication mismatch).

When using server-side generated names then it looks like that no new revision is created if the same image name is provided during an update. I'm not sure if this is still the case, we should verify that. However, I believe if this issue should be resolved, then it should be done on the server-side. Not via BYO name (which we really should move into a niche and not use it as default as outlined in #1144 )

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[rhuss]

/remove-lifecycle stale

This issue is still important and connected to these issues:

• can no longer force a re-download of an image via "kn update" w/o extra stuff #1318
• Digest resolution not working? serving#11317
• lock-to-digest help text default is backwards and doesn't seem to work #1329

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[rhuss]

/remove-lifecycle stale

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[rhuss]

/remove-lifecycle stale