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

Check for revision existance when updating traffic #445

Closed
rhuss opened this issue Oct 14, 2019 · 4 comments
Closed

Check for revision existance when updating traffic #445

rhuss opened this issue Oct 14, 2019 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@rhuss
Copy link
Contributor

rhuss commented Oct 14, 2019

When the traffic distribution is updated e.g. via

kn service update random --traffic v1=10 --traffic latest=90

and if the revision does not exist (note the typo here with latest instead of @latest, then the update hangs because the service waits for this revision to exists before becoming Ready (but this will be never the case). So either the server should already go into an error state or as this doesn't seem to be the case, the client should refuse to create traffic targets to revisions that do not exist.

@rhuss rhuss added the kind/bug Categorizes issue or PR as related to a bug. label Oct 14, 2019
@navidshaikh
Copy link
Collaborator

Revision existence check was originally planned with traffic splitting implementation (first check list item for #345), but then IIUC we took off that being in scope of server side validation for resource existence (knative/serving#5094). The issue is tagged for serving v0.10.x due Oct 29.

the client should refuse to create traffic targets to revisions that do not exist.

We can introduce this (and deprecate when we've server side validation for this).

@rhuss
Copy link
Contributor Author

rhuss commented Oct 15, 2019

The issue currently is that the long timeout of 10 mins introduced in kn service create and kn service update relies on the assumption that a service goes into False or True for the Ready condition after some time < 10 mins. If it stays in Unknown (as it is now for an unknown revision), it takes really that 10 minutes to timeout.

@akerekes
Copy link
Contributor

Looks like this can be closed as the server-side bug has been fixed?

$ ./kn service update hello --traffic hello-pyzfc-1=30% --traffic latest=70%
Updating Service 'hello' in namespace 'default':

  0.054s The Route is still working to reflect the latest desired specification.
RevisionMissing: Revision "latest" referenced in traffic not found.

@rhuss
Copy link
Contributor Author

rhuss commented Feb 15, 2020

Good point, thanks for the heads up.

@rhuss rhuss closed this as completed Feb 15, 2020
dsimansk pushed a commit to dsimansk/client that referenced this issue Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants