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

Revise downward API documentation #32400

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Mar 21, 2022

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. refactor Indicates a PR with large refactoring changes e.g. removes files or moves content cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 21, 2022
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Mar 21, 2022
@netlify
Copy link

netlify bot commented Mar 21, 2022

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 7f3604a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/628f7212b89481000787f577
😎 Deploy Preview https://deploy-preview-32400--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

title: Downward API
content_type: concept
description: >
There are two ways to expose Pod and container fields to a running container:
Copy link
Contributor

Choose a reason for hiding this comment

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

In this context, I'd write the first 'container' as 'Container'.
A Container has fields, but a container doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the API, Container with a capital C doesn't appear. We do see that in the Golang implementation, but not in the API (where it's actually containers).

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to look for a way to word this so that people don't start trying to kubectl get container and expect it to work, but also that readers end up clear on the two kinds of thing they can reference using the downward API.


### Fallback information for resource limits

{{< note >}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is a good practice to render a whole subsection into a note.

The Kubernetes downward API allows containers to consume information about themselves
or their context in a Kubernetes cluster. Applications in containers can have
access to that information, without the application needing to act as a client of
the Kubernetes HTTP API.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the Kubernetes HTTP API.
the Kubernetes API.

A `DownwardAPIVolumeFile` can expose Pod fields and Container fields.
[`downwardAPI` volume](/docs/concepts/storage/volumes/#downwardapi),
to expose information about itself to containers running in the Pod.
A `downwardAPI` volume can expose Pod fields and container fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A `downwardAPI` volume can expose Pod fields and container fields.
A `downwardAPI` volume can expose Pod fields and Container fields.


## {{% heading "prerequisites" %}}
In Kubernetes, there are two ways to expose Pod and container fields to a running container:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In Kubernetes, there are two ways to expose Pod and container fields to a running container:
In Kubernetes, there are two ways to expose Pod and Container fields to a running container:

The first element specifies that the value of the Pod's
`metadata.labels` field should be stored in a file named `labels`.
The second element specifies that the value of the Pod's `annotations`
field should be stored in a file named `annotations`.

{{< note >}}
The fields in this example are Pod fields. They are not
fields of the Container in the Pod.
fields of the container in the Pod.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fields of the container in the Pod.
fields of the Container in the Pod.

content/en/docs/concepts/workloads/pods/downward-api.md Outdated Show resolved Hide resolved
or API server.
* Read the [`spec`](/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec)
API definition for Pod.
In Kubernetes, you use a `spec` to define the desired state of an object.
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is useless, IMO.

* Read the [`spec`](/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec)
API definition for Pod.
In Kubernetes, you use a `spec` to define the desired state of an object.
* Read some [examples](/docs/concepts/workloads/pods/downward-api/#data-examples) of fields that you
Copy link
Contributor

Choose a reason for hiding this comment

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

No. They are not examples. The list contains all the fields that can be exposed.

@@ -155,18 +148,34 @@ The output shows the values of selected environment variables:
67108864
```

## Motivation for the Downward API
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this section belongs here, at the end of a task.

@sftim sftim force-pushed the 20220321_document_downward_api_concept branch from d8779d9 to 62fd82d Compare March 23, 2022 18:02
@sftim sftim marked this pull request as draft March 23, 2022 18:02
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2022
@sftim sftim force-pushed the 20220321_document_downward_api_concept branch 3 times, most recently from 113d17f to 85b6d85 Compare March 25, 2022 00:52
@sftim sftim marked this pull request as ready for review March 25, 2022 00:53
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 25, 2022
@k8s-ci-robot k8s-ci-robot requested a review from saad-ali March 25, 2022 00:54
@sftim
Copy link
Contributor Author

sftim commented Mar 25, 2022

How's this? I added text to explicitly explain that a Pod must include one Container.

@sftim sftim force-pushed the 20220321_document_downward_api_concept branch from 85b6d85 to 154ac66 Compare April 10, 2022 22:56
@sftim
Copy link
Contributor Author

sftim commented Apr 20, 2022

@tengqm if you have time, please take another look here.

@tengqm
Copy link
Contributor

tengqm commented Apr 20, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 20, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: f2fb40db71465fb5ec16d461bf7ab90c517d6cf3

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 25, 2022
- add a glossary entry
- add a concept page
- revise other documentation in light of above changes
@sftim sftim force-pushed the 20220321_document_downward_api_concept branch from 154ac66 to 7f3604a Compare May 26, 2022 12:26
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2022
@k8s-ci-robot k8s-ci-robot requested a review from tengqm May 26, 2022 12:26
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2022
@sftim
Copy link
Contributor Author

sftim commented May 26, 2022

Hi folks, this is ready for another review (I rebased).

Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@sftim Nonblocking nits, otherwise LGTM

Comment on lines +122 to +123
(based on the [node allocatable](/docs/tasks/administer-cluster/reserve-compute-resources/#node-allocatable)
calculation).
Copy link
Contributor

Choose a reason for hiding this comment

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

No parentheses needed

Suggested change
(based on the [node allocatable](/docs/tasks/administer-cluster/reserve-compute-resources/#node-allocatable)
calculation).
based on the [node allocatable](/docs/tasks/administer-cluster/reserve-compute-resources/#node-allocatable)
calculation.


An example is an existing application that assumes a particular well-known
environment variable holds a unique identifier. One possibility is to wrap the
application, but that is tedious and error prone, and it violates the goal of low
Copy link
Contributor

Choose a reason for hiding this comment

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

Normally I am treasurer for life of the I Hate Hyphens club, but for the sake of easier localization--

Suggested change
application, but that is tedious and error prone, and it violates the goal of low
application, but that is tedious and error-prone, and it violates the goal of low

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zacharysarah

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 17, 2022
@zacharysarah
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 17, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: e712e1299000b2685d87d695b396a9b1ff4b38cd

@k8s-ci-robot k8s-ci-robot merged commit 3a0d817 into kubernetes:main Jun 17, 2022
@sftim sftim deleted the 20220321_document_downward_api_concept branch June 20, 2022 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. refactor Indicates a PR with large refactoring changes e.g. removes files or moves content sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants