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

Service Creation in Release Mode #2819

Closed
dgerd opened this issue Dec 31, 2018 · 6 comments
Closed

Service Creation in Release Mode #2819

dgerd opened this issue Dec 31, 2018 · 6 comments
Assignees
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.
Milestone

Comments

@dgerd
Copy link

dgerd commented Dec 31, 2018

Current Behavior

A Service in Release Mode currently takes revision name(s) as input to the Spec to determine where to route traffic. However, since the service is responsible for creating the configuration there is a bootstrapping problem. A revision name is not known until the service creates it, but a revision name is required to create a Service in this mode.

To get around this restriction there are two patterns that are used in 0.2:

  1. Create the Service in "runLatest", observe the revision name, and then update to "release"
  2. Know that the first revision name is "${SERVICE_NAME}-00001", and pass that as input

Neither patterns are desirable. The first pattern has the disadvantage of requiring a create and update to achieve the desired state, and requires increased knowledge of Knative move through the required steps.

The pattern of putting non-existent revisions in the list creates a race condition between the update of the route and the creation of the revision which is not handled by the Service. This can lead to unexpected failures. In addition, the second pattern requires the user to know the naming convention, and may result in hardcoding a convention that is not currently covered by the runtime spec.

Proposal

This proposal was discussed at the 2018/12/05 API Work Group meeting.

I propose the introduction of sentinel values in the "revisions" list to allow a user to reference the latest created Revision without knowing the name. Future variables could be added to extend the functionality, but a single sentinel value (i.e. "@latestRevision" ) would be all that is currently required. Sentinel values can be denoted by a prefix sigil that is outside of the Kubernetes naming convention. This allows for the namespace of sentinel values to not conflict with valid revision names.

Other considered options can be seen in the document linked below.

The biggest advantages of this approach over others is that:

  1. The action of the user is very explicit
  2. It brings up the service into a "Ready" state
  3. It is extensible and usable outside of Service creation.

Additional Info

Original Proposal Document (Shared with knative-users@ and knative-dev@)

@knative-prow-robot knative-prow-robot added area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding. labels Dec 31, 2018
@dgerd
Copy link
Author

dgerd commented Dec 31, 2018

/milestone Serving 0.4

@vagababov
Copy link
Contributor

/assign

Will follow the naming suggested here.

@vagababov
Copy link
Contributor

So "@x" is not a valid YAML token, unless quoted.

@mattmoor
Copy link
Member

What if we just assert that "latest" is an invalid Revision name and use that?

@vagababov
Copy link
Contributor

latest feels like a very generic keyword. latest-revision, perhaps/

vagababov added a commit to vagababov/serving that referenced this issue Feb 6, 2019
Followup to the main implementation of knative#2819.
- fix the test comment, to reflect the reality
- add a case where candidate is replaced with @latest
- traffic distribution is validated.
knative-prow-robot pushed a commit that referenced this issue Feb 8, 2019
* Add one more case to cover @latest as part of the traffic split.

Followup to the main implementation of #2819.
- fix the test comment, to reflect the reality
- add a case where candidate is replaced with @latest
- traffic distribution is validated.

* update the test

* intermediate checkpoint state

* checkpoint

* checkpoint

* make sure stuff works

* restore util.go

* update comment

* Update test/conformance/service_test.go

Co-Authored-By: vagababov <vagababov@users.noreply.github.com>

* Update test/conformance/util.go

Co-Authored-By: vagababov <vagababov@users.noreply.github.com>

* address comments
@mattmoor
Copy link
Member

I believe this is done, so I'm going to close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

4 participants