Commit API spec and design doc by evankanderson · Pull Request #444 · knative/serving

evankanderson · 2018-03-21T18:32:08Z

This is a conversion from GDoc-to-markdown (and cleanup) of the initial API spec written at Google and shared with various partners.

The goal is for this to become the official repo of the API specification.

Add images from API draft

Still TODO: - Appendices A-E. - Resolve diffs between spec.md and concrete API definition. - Define process for updating the API specification. - Additional API specifications covering: * Logging / monitoring / observability contract * Runtime container contract / environment

mattmoor · 2018-03-21T19:43:32Z

docs/spec/overview.md

+
+* **Revisions**, which are immutable snapshots of code + config, created by a
+
+* **Configuration**, which acts as a stream of environments for Revisions.


The diagram @mchmarny has in his docs PR would be fantastic here.

You mean https://github.com/mchmarny/elafros/blob/master/docs/overview/images/api-objects.png ?

That's one of about 5-6 pictures that needs to be added to the PR.

Yep, exactly.

evankanderson · 2018-03-21T20:13:37Z

Much credit to @cooperneil for much of the original prose of the document.

sixolet · 2018-03-21T21:40:43Z

docs/spec/overview.md

+* a single resource that can be watched to see a history of all
+  revisions created
+
+* enables (but doesn’t mandate) PATCH semantics for new revisions to


parallelism fail: "provides enables PATCH semantics"

Configuration:

provides a single referenceable...

provides a single resource ...

enables ...

provides the ability

I think I can just omit "enables" here, and say "provides (but does not mandate) PATCH semantics ..."

sixolet · 2018-03-21T21:42:22Z

docs/spec/overview.md

+  change to configuration, such as updating an env variable,
+  inheriting all other configuration and source/image.
+
+When creating an initial route and performing the first deployment,


This paragraph will want to change with a resolution to #412

I think another layer of this document will be necessary after formalizing #412, but in its original form, this doc describes operating on the core elafros building blocks

sixolet · 2018-03-21T21:43:24Z

docs/spec/spec.md

+  selfLink: ...
+  ...
+spec:
+  rollout:


Current API has traffic bumped up a level, and no rollout field.

Done, thanks!

cooperneil · 2018-03-21T22:21:17Z

docs/spec/errors.md

+  - type: BuildFailed
+    status: True
+    reason: BuildStepFailed  # could also be SourceMissing, etc
+    # reason is for machine consumption, message is for human consumption


I think this is in conflict with the recommendation in #351, where:

Reason is typically defined in terms of a string and so the values this takes on (while camelcase words) should be treated as opaque. Clients shouldn't programmatically act on their values, but bias towards using Reason as a terse explanation of the state for end-users

cooperneil · 2018-03-21T22:23:05Z

docs/spec/normative_examples.md

+  - revisionName: def
+    percent: 25
+  conditions:
+  - type: RolloutInProgress


In the spirit of making terminal happy-path conditions true, perhaps we should flip this to change RolloutInProgress -> RolloutComplete

We have a Ready condition today, to reflect the time until the referenced Revisions and Configurations are themselves first ready. After it becomes Ready I wouldn't expect transitions such as Configuration rollouts to affect readiness.

RolloutComplete with status: False (or conversely RolloutInProgress) certainly communicate a notable condition, but I wonder if this is adequately covered by .status.traffic and events?

cooperneil · 2018-03-21T22:27:11Z

docs/spec/spec.md

+  # +optional. composable Build spec, if omitted provide image directly
+  build:  # This is a build.dev/v1alpha1.BuildTemplateSpec
+    source:
+      # oneof archive|manifest|repository:


this can be updated with the new proposed Build structure, featuring:
source:
git:
gcs:
custom:

@mattmoor - I can't recall the issue # that describes this form

knative/build#78

labadav · 2018-03-21T20:43:10Z

docs/product/personas.md

+
+These are generally software engineres looking to build and run a
+stateless software application, without concern about the underlying
+infrastructure.


The developer personas are software engineers, who are looking to build and run a stateless software application without having to worry about the underlying infrastructure.

I believe the comma in "software engineers, who are looking to build..." is unnecessary. I'm also unsure what a stateless non-software application would be; is it necessary to mention software? I'd rephrase this as:

The developer personas are software engineers looking to build and run a stateless application without worrying about the underlying infrastructure.

Yes, right on both counts!

typo: engineres

D'oh, I copied this from another branch. Well, I'll copy it in because it may be useful, but we should merge it with @mchmarny 's docs.

labadav · 2018-03-21T20:46:01Z

docs/product/personas.md

+
+* Hobbyist
+* "Backend" SWE
+* "Full stack" SWE


remove quotes:
Backend SWE
Full stack SWE

labadav · 2018-03-21T20:48:11Z

docs/product/personas.md

+  - Start a build
+  - Read build logs
+* Language operator
+  - Create a build path/build pack


Maybe, to be parallel with the other sections this should say:

Developer (starting a build and reading build logs)

Language operator (creating a build path/build pack)

cc @imjasonh

I updated this to add User stories in addition to the prototypical job roles that the first section had.

labadav · 2018-03-21T20:49:56Z

docs/spec/errors.md

+  of the resource.
+
+* other fields may provide status on the most-recently retrieved state
+  of the world as it relates to the resource (example: number of


most recently retrieved (no hyphen)
state of the system (not world) or state of the environment?

labadav · 2018-03-21T20:50:40Z

docs/spec/errors.md

@@ -0,0 +1,304 @@
+# Error Conditions and Reporting
+
+The standard kubernetes API pattern for reporting configuration errors


Kubernetes API (capitalized)

labadav · 2018-03-21T22:42:41Z

docs/spec/overview.md

+to any or all backing revisions.
+
+**Revision** is an immutable snapshot of code and configuration. A
+revision may have been created from a pre-built container image or


may have been => can be

labadav · 2018-03-21T22:43:42Z

docs/spec/overview.md

+built from source. While there is a history of previous revisions,
+only those currently referenced by a Route are addressable or
+routable. Older inactive revisions need not be backed by underlying
+resources, they may exist only as the revision metadata in


they exist only as revision metadata in storage.

labadav · 2018-03-21T22:44:11Z

docs/spec/overview.md

+
+A **Configuration** describes the desired latest Revision state, and
+creates and tracks the status of Revisions as the desired state is
+updated. A configuration may include instructions on how to transform


may => might

labadav · 2018-03-21T22:45:20Z

docs/spec/overview.md

+  rollouts
+
+* a single resource that can be watched to see a history of all
+  revisions created


of all the revisions created

labadav · 2018-03-21T22:47:27Z

docs/spec/overview.md

+* a single resource that can be watched to see a history of all
+  revisions created
+
+* enables (but doesn’t mandate) PATCH semantics for new revisions to


Configuration:

provides a single referenceable...

provides a single resource ...

enables ...

provides the ability

mattmoor

Super excited to see this. I read through a left comments.

When this goes in, I'll so another pass and open issues for the gaps.

mattmoor · 2018-03-22T00:27:25Z

docs/product/personas.md

+
+These are generally software engineres looking to build and run a
+stateless software application, without concern about the underlying
+infrastructure.


typo: engineres

mattmoor · 2018-03-22T00:28:11Z

docs/product/personas.md

+  - Start a build
+  - Read build logs
+* Language operator
+  - Create a build path/build pack


cc @imjasonh

mattmoor · 2018-03-22T00:30:46Z

docs/spec/errors.md

+...
+status:
+  latestReadyRevisionName: abc
+  latestCreatedRevisionName: bcd  # Hasn't become became "Ready"


comment needs an update.

mattmoor · 2018-03-22T00:33:52Z

docs/spec/errors.md

+  conditions:
+  - type: Complete
+    status: True
+  - type: BuildFailed


On the Build resource, this would just be Failed. On the Revision, it is qualified as BuildFailed.

mattmoor · 2018-03-22T00:35:11Z

docs/spec/errors.md

+  buildLogsLink: "http://logging.infra.mycompany.com/...?filter=..."
+  conditions:
+  - type: Complete
+    status: True


Complete implies success, so given the heading we should remove this in favor of just Failed.

I modeled this after Job, which does the same IIRC.

mattmoor · 2018-03-22T02:10:07Z

docs/spec/normative_examples.md

+BuildSpec of a Build resource in the Configuration. This describes:
+
+* **What** to build (`build.source`): Source can be provided as an
+  archive, manifest file, or repository.


git, gcs, or custom.

mattmoor · 2018-03-22T02:11:21Z

docs/spec/normative_examples.md

+  build:  # build.dev/v1alpha1.BuildTemplateSpec
+    source:
+      # oneof archive|manifest|repository:
+      archive:


mattmoor · 2018-03-22T02:12:33Z

docs/spec/normative_examples.md

+creation of two Revisions, one with the config change, and the other
+with the new code deployment. It is expected that Revision will wait
+for the `revisionTemplate.spec.container.image` to be live before
+marking the Revision as "ready".


Change this last sentence to waiting for buildName to complete?

mattmoor · 2018-03-22T02:15:40Z

docs/spec/normative_examples.md

+    source:
+      # oneof archive|manifest|repository:
+      archive:
+        url: https://...


nit: update

mattmoor · 2018-03-22T02:15:57Z

docs/spec/normative_examples.md

+  revisionTemplate:  # template for building Revision
+    metadata:
+      labels:
+        # One-of "function" or "app", convention for CLI/UI clients to list/select


or "container"?

"container" is a packaging format. "function" and "app" are calling styles, where a "function" typically has a single named endpoint (and possibly some supporting metadata/discovery URLs about typing etc), while an "app" has multiple HTTP endpoints.

Ok, then I think we need to update like 99% of our samples, which IIRC I switched from serviceType: container to this kind of label w/ container :)

I can see about putting together a PR to change this, but "container" doesn't really tell the UI much about what's going on inside (since both functions and apps end up being run as containers).

It's also valid to not supply the label, but then the UI will have to treat the Revision generically.

bobcatfish

So excited to see these docs in the repo! These will really help people understand the project.

Several of these pages currently have no links into them, could we add some links from DEVELOPMENT.md, maybe CONTRIBUTING.md as well?

bobcatfish · 2018-03-23T14:32:12Z

docs/spec/overview.md

+Configuration provides:
+
+* a single referenceable resource for the route to perform automated
+  rollouts


your bulleted lists have one line too much whitespace:

single referenceable resource for the route to perform automated rollouts

a single resource that can be watched to see a history of all revisions created

v.s.

single referenceable resource for the route to perform automated rollouts

a single resource that can be watched to see a history of all revisions created

The former looks kinda strange, the latter is what one would expect to see and is easier on the eye

bobcatfish · 2018-03-23T14:37:01Z

docs/spec/normative_examples.md

+
+* Creating a revision from source
+
+* Creating a function


could these be numbered and link to each section? and i think we should have 1 item for each of the 5 examples below e.g.

1. [Automatic rollout of a new Revision to existing Service - pre-built container](#1-automatic-rollout-of-a-new-revision-to-existing-service---pre-built-container) 2. [Creating Route and deploying first Revision - pre-built container](#2-creating-route-and-deploying-first-revision---pre-built-container) 3. [Manual rollout of a new Revision - config change only](#3-manual-rollout-of-a-new-revision---config-change-only) 4. [Deploy a Revision from source](#4-deploy-a-revision-from-source) 5. [Deploy a Function](#5-deploy-a-function)

Added a TOC, though I used more free text than the literal section headings. I forget that github markdown doesn't automate convenient navigation.

bobcatfish · 2018-03-23T14:43:17Z

docs/product/personas.md

+# Elafros Personas
+
+When discussing user actions, it is often helpful to [define specific
+user roles](http://www.agilemodeling.com/artifacts/personas.htm) who


can we link to a different source of information about personas? or remove the link if we can't find anything better. the example persona on this page seems like a pretty classic example of 'so easy my grandmother could so it'

GRANDMA GOT STEM

<end of rant>

Thanks, I hadn't actually read the persona so much as Googled and picked out the first link where the leading 5 sentences made sense. I switched to Wikipedia, which seems less biased. If you can find a better reference, I'd be happy to switch the link again.

bobcatfish · 2018-03-23T14:44:46Z

docs/spec/motivation.md

@@ -0,0 +1,21 @@
+The goal of the Elafros project is to provide a common toolkit and API


instead of making this a separate doc, what do you think about putting this into the main README? it seems like it could be a good intro to the repo/project

My plan is that some amount of the stuff in docs is eventually going to live at http://elafros.dev/

I'd be happy to have the README steal from some of these docs, though. (Maybe in a separate PR?)

bobcatfish · 2018-03-23T14:46:13Z

docs/spec/spec.md

+```
+
+
+## Configuration


it would be handy if these sections in the spec linked back to their explanations in the overview

This tied in well with @mattmoor's suggestion. (Done)

mattmoor · 2018-03-24T14:04:55Z

docs/spec/overview.md

+
+![Object model](images/object_model.png)
+
+## Route


This is the only heading, which stands out looking at the rendered form.

Added headers for the others, to allow reference from other docs.

mattmoor · 2018-03-24T20:14:15Z

docs/spec/errors.md

+  - type: LatestRevisionReady
+    status: False
+	reason: RevisionMissing
+	message: "The latest Revision appears to have been deleted."


New emacs -- I had indent-tabs-mode non-nil.

mattmoor · 2018-03-24T20:14:55Z

docs/spec/spec.md

-        # build.template.arguments[_IMAGE], as the "promise" of a future build
+        # build.template.arguments[_IMAGE], as the "promise" of a future build.
+		# If buildName is provided, it is expected that this image will be
+		# present when the referenced build is complete.


mattmoor · 2018-03-24T20:16:05Z

docs/spec/spec.md

+  - type: RolloutInProgress
+    status: False
+  - type: TrafficDropped
+    status: False


Doh, these comments don't create links, otherwise they'd be perfect :(

mattmoor · 2018-03-24T20:19:21Z

docs/spec/spec.md

@@ -233,7 +235,7 @@ status:
  # the provenance of the revision, from the container
  imageSource:
    archive|manifest|repository: ...


I think this could use an update (if the intention is to match Build's source 1:1), or maybe even a qualification as something that needs more detailed specification (if we want it to match Grafeas, or some external standard).

mattmoor · 2018-03-24T20:21:48Z

docs/spec/normative_examples.md

+$ elafros revisions list --service my-service
+Name    Traffic  Id   Date                Deployer     Git sha
+next     0%      v3   2018-01-19 12:16    user1        64d79ce
+current  100%    v2   2018-01-18 20:34    user1        a6f92d1


Yeah, these kind of things just seem to catch my eye for some reason. I'm reminded of the early ECR UI "screenshots" where all of the images had the same digest, but different sizes :)

mattmoor · 2018-03-24T20:25:10Z

docs/spec/normative_examples.md

+  revisionTemplate:  # template for building Revision
+    metadata:
+      labels:
+        # One-of "function" or "app", convention for CLI/UI clients to list/select


Ok, then I think we need to update like 99% of our samples, which IIRC I switched from serviceType: container to this kind of label w/ container :)

mattmoor · 2018-03-27T21:31:42Z

/lgtm

google-prow-robot · 2018-03-27T21:31:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, mattmoor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [evankanderson,mattmoor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Evan Anderson added 2 commits February 15, 2018 20:07

Define software personas for Elafros.

cb6b3f1

evankanderson requested a review from a team March 21, 2018 18:32

google-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 21, 2018

evankanderson self-assigned this Mar 21, 2018

evankanderson requested a review from carieshmarie March 21, 2018 19:03

mattmoor reviewed Mar 21, 2018

View reviewed changes

sixolet reviewed Mar 21, 2018

View reviewed changes

cooperneil reviewed Mar 21, 2018

View reviewed changes

labadav reviewed Mar 21, 2018

View reviewed changes

mattmoor reviewed Mar 22, 2018

View reviewed changes

bobcatfish reviewed Mar 23, 2018

View reviewed changes

Evan Anderson added 7 commits March 24, 2018 04:28

Merge remote-tracking branch 'upstream/master' into spec

b015f63

Add resource relationship images for overview and examples.

abed401

Fix image links to point to subdir

5f531b3

Update personas with @labadav feedback and additional words.

e4e16ea

Apply @labadav comments and other feedback on error defs, add ToC

ddd3b5f

Apply @sixolet and @cooperneil feedback on types and resource structure.

21f3d1c

Apply remaining @labadav comments.

c93a524

mattmoor reviewed Mar 24, 2018

View reviewed changes

Address @mattmoor comments.

99b541d

mattmoor reviewed Mar 24, 2018

View reviewed changes

mattmoor approved these changes Mar 24, 2018

View reviewed changes

Evan Anderson added 3 commits March 25, 2018 01:46

Fix personas link to be less implicitly biased.

c2a765e

Add section heads for Overview, to allow references later.

85458d5

Clean up tabs and other formatting in spec and errors

e41e0f6

evankanderson mentioned this pull request Mar 25, 2018

Mark samples as either "app" or "function" #500

Merged

google-prow-robot assigned mattmoor Mar 27, 2018

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2018

mattmoor mentioned this pull request Mar 27, 2018

Add TrafficDropped condition to Route #495

Closed

evankanderson merged commit 5a496e5 into knative:master Mar 27, 2018


		* Revisions, which are immutable snapshots of code + config, created by a

		* Configuration, which acts as a stream of environments for Revisions.

		@@ -0,0 +1,304 @@
		# Error Conditions and Reporting

		The standard kubernetes API pattern for reporting configuration errors

		@@ -0,0 +1,21 @@
		The goal of the Elafros project is to provide a common toolkit and API

		```


		## Configuration

Conversation

evankanderson commented Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evankanderson commented Mar 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattmoor Mar 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cooperneil Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattmoor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

evankanderson commented Mar 21, 2018 •

edited

Loading

mattmoor Mar 22, 2018 •

edited

Loading

cooperneil Mar 21, 2018 •

edited

Loading