Skip to content

Clean-up the use of RFC2119 keywords #4550

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.

Sign up for GitHub

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

Merged
knative-prow-robot merged 3 commits into from
Jul 13, 2019
Merged

Clean-up the use of RFC2119 keywords #4550

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46b7e95
Clean-up the use of RFC2119 keywords
Jun 27, 2019
11449e9
more edits from evan and dan
Jul 2, 2019
bfe62b0
more tweaks
Jul 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 43 additions & 51 deletions docs/runtime-contract.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ workloads. In contrast to general-purpose containers, stateless
request-triggered (i.e. on-demand) autoscaled containers have the following
properties:

- Little or no long-term runtime state (especially in cases where code may be
- Little or no long-term runtime state (especially in cases where code might be
scaled to zero in the absence of request traffic).
- Logging and monitoring aggregation (telemetry) is important for understanding
and debugging the system, as containers may be created or deleted at any time
and debugging the system, as containers might be created or deleted at any time
in response to autoscaling.
- Multitenancy is highly desirable to allow cost sharing for bursty applications
on relatively stable underlying hardware resources.
Expand All @@ -35,16 +35,16 @@ The
([v1.0.1](https://github.com/opencontainers/runtime-spec/blob/v1.0.1/spec.md))
is the basis for this document. When this document and the OCI specification
conflict, this document is assumed to override the general OCI recommendations.
Where this document does not specify behavior, developers should assume an OCI
compliant underlying implementation. Additionally, the core Knative definition
Where this document does not specify behavior, runtime implementations SHOULD
be OCI compliant with respect to those features. Additionally, the core Knative definition
Comment thread
evankanderson marked this conversation as resolved.
assumes the
[Linux Container Configuration](https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md).

In particular, the default Knative implementation relies on Kubernetes behavior
to implement container operation. In some cases, current Kubernetes behavior in
2018 is not as performant as recommended in this documentation. The goal of the
Knative authors is to push as much of the required functionality into Kubernetes
and/or Istio as possible, rather than implementing reach-around layers.
2018 is not as performant as envisioned in this documentation. The goal of the
Knative authors is to push as much of the needed functionality into Kubernetes
and/or HTTP routers as possible, rather than implementing reach-around layers.

This document considers two users of a given Knative environment, and is
particularly concerned with the expectations of _developers_ (and _language and
Expand All @@ -62,7 +62,7 @@ tooling developers_, by extension) running code in the environment.
## Runtime and Lifecycle

Knative aims to minimize the amount of tuning and production configuration
required to run a service. Some of these production-friendly features include:
needed to run a service. Some of these production-friendly features include:

1. Stateless computation at request-scale or event-scale granularity.
1. Automatic scaling between 0 and many instances (the process scale-out
Expand All @@ -71,7 +71,7 @@ required to run a service. Some of these production-friendly features include:
where possible.

In order to achieve these properties, containers which are operated as part of a
serverless platform SHOULD observe the following properties:
serverless platform are expected to observe the following properties:

- Fast startup time (<1s until a request or event can be processed, given
container image layer caching),
Expand All @@ -82,9 +82,7 @@ serverless platform SHOULD observe the following properties:

Comment thread
duglin marked this conversation as resolved.
### State

The general OCI state may not be available for introspection within the
container, and may only be visible to the system operator or platform provider.
In a highly-shared environment, containers may experience the following:
In a highly-shared environment, containers might experience the following:

- Containers with `status` of `stopped` MAY be immediately reclaimed by the
system.
Expand All @@ -103,12 +101,12 @@ In a highly-shared environment, containers may experience the following:
- The environment MAY restrict the use of `prestart`, `poststart`, and
`poststop` hooks to platform operators rather than developers. All of these
hooks are defined in the context of the runtime namespace, rather than the
container namespace, and may expose system-level information (and are
container namespace, and might expose system-level information (and are
non-portable).
- Failures of the developer-specified process MUST be logged to a
developer-visible logging system.

In addition, some serverless environments may use an execution model other than
In addition, some serverless environments MAY use an execution model other than
docker in linux (for example, [runv](https://github.com/hyperhq/runv) or
[Kata Containers](https://katacontainers.io/)). Implementations using an
execution model beyond docker in linux MAY alter the lifecycle contract beyond
Expand All @@ -126,11 +124,6 @@ the OCI specification as long as:
contents from a particular execution. Because containers (particularly failing
containers) can experience frequent starts, operators or platform providers
SHOULD limit the total space consumed by these failures.
- A container should write its own termination message to `/dev/termination-log`
by default. If no message is written by the container, the last few lines of
log output should be reported as the execution error (i.e. by
[setting the `terminationMessagePolicy` to `FallbackToLogsOnError`](https://kubernetes.io/docs/tasks/debug-application-cluster/determine-reason-pod-failure/#customizing-the-termination-message))
on Kubernetes.

### Warnings

Expand Down Expand Up @@ -165,7 +158,7 @@ always result in `EOF`. The `stdout` and `stderr` file descriptors on the
container SHOULD be collected and retained in a developer-accessible logging
repository. (TODO:[docs#902](https://github.com/knative/docs/issues/902)).

Within the container, pipes and file descriptors may be used to communicate
Within the container, pipes and file descriptors can be used to communicate
between processes running in the same container.

#### Dev symbolic links
Expand Down Expand Up @@ -204,7 +197,7 @@ available in the `PORT` environment variable.

The platform provider SHOULD configure the platform to perform HTTPS termination
and protocol transformation e.g. between QUIC or HTTP/2 and HTTP/1.1. Developers
should not need to implement multiple transports between the platform and their
ought not need to implement multiple transports between the platform and their
code. Unless overridden by setting the
[`name`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#containerport-v1-core)
field on the inbound port, the platform will perform automatic detection as
Expand All @@ -217,10 +210,10 @@ following protocol will be used:
- `h2c`: HTTP/2 transport, as described in
[section 3.4 of the HTTP2 spec (Starting HTTP/2 with Prior Knowledge)](https://http2.github.io/http2-spec/#known-http)

Developers SHOULD prefer to use automatic content negotiation where available,
Developers ought to use automatic content negotiation where available,
and MUST NOT set the `name` field to arbitrary values, as additional transports
Copy link
Copy Markdown
Author

@duglin duglin Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgerd is the "MUST NOT" here overstepping too? Seems like it given we're removing the other keywords from this paragraph.

Copy link
Copy Markdown

@dgerd dgerd Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "MUST NOT" is important to allow us to reserve the 'name' field for future usage. It could be argued that this is more of an API constraint than a runtime constraint, but it is important and I am okay with duplicating it here for clarity.

For example, if HTTP/3.0 comes along and we want to allow people to specify "http3" we can only enable this we know that no existing Knative Configurations allow "http3"; otherwise this will be a breaking change.

I am okay rephrasing this if you think it is unclear as is.

Copy link
Copy Markdown
Member

@evankanderson evankanderson Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "MUST NOT" suggests that validation during admission control is allowed here, and containers with other port names may be rejected.

Copy link
Copy Markdown
Author

@duglin duglin Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I'll leave it as-is.

may be defined in the future. Developers MUST assume all traffic is
intermediated by an L7 proxy. Developers MUST NOT assume a direct network
might be defined in the future. Developers can assume all traffic is
Copy link
Copy Markdown
Member

@evankanderson evankanderson Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we remove MUST here? I think #4035 moves these to a "advice to the developer" section, but the requirement here is intended to include:

  1. Don't assume that the other end of the TCP connection is the client
  2. Don't need to implement SSL in the container

Copy link
Copy Markdown
Author

@duglin duglin Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the MUST because we can't really control what the developers "assume". If we want to add something normative here then it needs to be a constraint on the Kn impl - so something like: Implementations MUST send all traffic through L7 proxies. Do you want that instead?

Copy link
Copy Markdown

@dgerd dgerd Jul 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with this change.

This runtime contract is still pretty long and I don't expect developers to read through it. The language is meant for operators and contributors.

I would eventually like to move these suggestions to a shorter user-focused document. I intend to break this file out into its own PR ( https://github.com/knative/serving/pull/4035/files#diff-2b3ccedac0c1b327ad7d674942f6cf56 ).

intermediated by an L7 proxy. Developers can not assume a direct network
connection between their server process and client processes.

#### Headers
Expand Down Expand Up @@ -282,7 +275,7 @@ target the inbound container port; platform providers SHOULD disallow other
probe methods.

Because serverless platforms automatically scale instances based on inbound
requests, and because noncompliant (or even failing) containers may be provided
requests, and because noncompliant (or even failing) containers might be provided
by developers, the following defaults SHOULD be applied by the platform provider
if not set by the developer. The probes are intended to be trivially supportable
by naive conforming containers while preventing interference with developer
Expand All @@ -292,9 +285,9 @@ code. These settings apply to both `livenessProbe` and `readinessProbe`:
- `initialDelaySeconds` set to 0
- `periodSeconds` set to platform-specific value

In order to enable scaling in response to load, developers SHOULD NOT set the
`initialDelaySeconds` to a value greater than 0, and should aim to minimize
container startup time (aka cold start time).
Setting `initialDelaySeconds` to a value greater than 0 impacts container
startup time (aka cold start time) as a container will not serve traffic until
the probe succeeds.

##### Deployment probe

Expand All @@ -321,7 +314,7 @@ be run at least once despite scale-to-zero guarantees.

OCI does not specify any properties of the network environment in which a
container runs. The following items are OPTIONAL additions to the runtime
contract which describe services which may be of particular value to platform
contract which describe services which might be of particular value to platform
providers.

#### DNS
Expand Down Expand Up @@ -355,9 +348,9 @@ serverless workloads. Containers MUST use the provided temporary storage areas

### Mounts

In general, stateless applications should package their dependencies within the
container and not rely on mutable external state for templates, logging
configuration, etc. In some cases, it may be necessary for certain application
In general, stateless applications package their dependencies within the
container and do not rely on mutable external state for templates, logging
configuration, etc. In some cases, it might be necessary for certain application
settings to be overridden at deploy time (for example, database backends or
authentication credentials). When these settings need to be loaded via a file,
read-only mounts of application configuration and secrets are supported by
Expand Down Expand Up @@ -406,7 +399,7 @@ such variables will follow demonstrated usage and utility.

### User

Developers MAY specify that containers should be run as a specific user or group
Developers MAY specify that containers be run as a specific user or group
ID using the `runAsUser` container property. If specified, the runtime
[MUST](https://github.com/knative/serving/blob/master/test/conformance/runtime/user_test.go)
run the container as the specified user ID if allowed by the platform (see
Expand All @@ -421,12 +414,12 @@ be failed.

### Default Filesystems

The OCI specification describes a default container environment which may be
The OCI specification describes a default container environment which can be
used for many different purposes, including containerization of existing legacy
or stateful processes which may store substantial amounts of on-disk state. In a
or stateful processes which might store substantial amounts of on-disk state. In a
scaled-out, stateless environment, container startup and teardown is accelerated
when on-disk resources are kept to a minimum. Additionally, developers may not
have access to the container filesystems (or the containers may be rapidly
when on-disk resources are kept to a minimum. Additionally, developers might not
have access to the container's filesystems (or the containers might be rapidly
recycled), so log aggregation SHOULD be provided.

In addition to the filesystems recommended in the OCI, the following filesystems
Expand All @@ -438,13 +431,12 @@ be provided:
| `/tmp` | MUST be Read-write.<p>SHOULD be backed by tmpfs if disk load is a concern. |
| `/var/log` | MUST be a directory with write permissions for logs storage. Implementations MAY permit the creation of additional subdirectories and log rotation and renaming. |

In addition, the following files may be overridden by the runtime environment to
enable DNS resolution:
To enable DNS resolution, the following files might be overwritten at runtime:

| File | Description |
| ------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `/etc/hosts` | MAY be overridden to provide host mappings for well-known or provider-specific resources. |
| `/etc/hostname` | OPTIONAL: some environments may set this to a different value for each container, but other environments may use the same value for all containers. |
| `/etc/hostname` | some environments MAY set this to a different value for each container, but other environments might use the same value for all containers. |
| `/etc/resolv.conf` | SHOULD be set to a valid cluster-specific recursive resolver. Providers MAY provide additional default search domains to improve customer experience in the cluster. |

Platform providers MAY provide additional platform-specific mount points
Expand Down Expand Up @@ -503,9 +495,9 @@ Seccomp provides a mechanism for further restricting the set of linux syscalls
permitted to the processes running inside the container environment. A seccomp
sandbox MAY be enforced by the platform operator; any such application profiles
SHOULD be configured and applied in a consistent mechanism outside of the
container specification. As the seccomp policy may be part of the platform
security hardening, operators MAY tune this over time as the threat environment
changes.
container specification. A seccomp policy MAY be part of the platform
security configuration that operators can tune over time as the
threat environment changes.

### Rootfs Mount Propagation

Expand All @@ -517,17 +509,17 @@ From the OCI spec:
> article in the kernel documentation has more information about mount
> propagation.

This option should only be set by the operator or platform provider, and MUST
NOT be configurable by the developer. As mount propagation may be part of the
platform security hardening, operators MAY tune this over time as the threat
environment changes.
This option MAY be set by the operator or platform provider, and MUST
NOT be configurable by the developer. Mount propagation MAY be part of the
platform security configuration that operators can tune over time
as the threat environment changes.

### Masked Paths

This option MAY only be set by the operator or platform provider, and MUST NOT
be configurable by the developer. As masked paths may be part of the platform
security hardening, operators may tune this from time to time as the threat
environment changes.
This option MAY be set by the operator or platform provider, and MUST NOT
be configurable by the developer. Masked paths MAY be part of the platform
security configuration that operators can tune over time as the
threat environment changes.

### Readonly Paths

Expand Down