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

Edit and redo ConfigMap docs #35160

Merged
merged 12 commits into from
Jul 26, 2022
Merged

Conversation

tomkivlin
Copy link
Contributor

@tomkivlin tomkivlin commented Jul 19, 2022

Move exiting content on optional ConfigMaps into the "Understanding ConfigMaps and Pods" section to make it more generic (was a sub-section within "Add ConfigMap data to a Volume" and re-worded it slightly.

Also added in content from #2334, which was removed in #2568.

Resolves #9829

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 19, 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 Jul 19, 2022
@netlify
Copy link

netlify bot commented Jul 19, 2022

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit ad33b0c
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62dfaa99b292d60008cf3508
😎 Deploy Preview https://deploy-preview-35160--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.

@sftim
Copy link
Contributor

sftim commented Jul 20, 2022

/retitle Edit and redo ConfigMap docs

We avoid putting PR references (eg #2334) in PR titles.

@k8s-ci-robot k8s-ci-robot changed the title small edit and add back content from PR#2334 Edit and redo ConfigMap docs Jul 20, 2022
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks. Some suggestions about the optional ConfigMaps. We shouldn't imply that the behavior is always the same - those optional ConfigMaps might actually exist.

tomkivlin and others added 6 commits July 21, 2022 07:49
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
…nfigmap.md

Co-authored-by: Tim Bannister <tim@scalefactory.com>
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 21, 2022
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 21, 2022
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@shannonxtreme would you like to comment on this one? I know you've been looking at the tech writing in the secrets-and-configmaps area recently.

Copy link
Contributor

@shannonxtreme shannonxtreme left a comment

Choose a reason for hiding this comment

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

I made some suggestions for clarity. Thank you for this PR :)

@@ -675,7 +657,7 @@ data:

### Restrictions

- You must create a ConfigMap before referencing it in a Pod specification (unless you mark the ConfigMap as "optional"). If you reference a ConfigMap that doesn't exist, the Pod won't start. Likewise, references to keys that don't exist in the ConfigMap will prevent the pod from starting.
- You must create a ConfigMap before referencing it in a Pod specification, or mark the ConfigMap as "optional" (see [Optional ConfigMaps](#optional-configmaps)). If you reference a ConfigMap that doesn't exist, or hasn't been marked as "optional" the Pod won't start. Likewise, references to keys that don't exist in the ConfigMap will prevent the pod from starting.
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like this makes the sentence harder to understand.

Suggested change
- You must create a ConfigMap before referencing it in a Pod specification, or mark the ConfigMap as "optional" (see [Optional ConfigMaps](#optional-configmaps)). If you reference a ConfigMap that doesn't exist, or hasn't been marked as "optional" the Pod won't start. Likewise, references to keys that don't exist in the ConfigMap will prevent the pod from starting.
- You must create the `ConfigMap` object before you reference it in a Pod specification. Alternatively, mark the ConfigMap reference as `optional` in the Pod spec (see [Optional ConfigMaps](#optional-configmaps)). If you reference a ConfigMap that doesn't exist and you don't mark the reference as `optional`, the Pod won't start. Similarly, references to keys that don't exist in the ConfigMap will also prevent the Pod from starting, unless you mark the key references as `optional`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in ad33b0c

tomkivlin and others added 3 commits July 26, 2022 09:19
…nfigmap.md

Co-authored-by: Shannon Kularathna <ax3shannonkularathna@gmail.com>
…nfigmap.md

Co-authored-by: Shannon Kularathna <ax3shannonkularathna@gmail.com>
@tomkivlin
Copy link
Contributor Author

Thanks, @shannonxtreme - I have made the updates as suggested.

@shannonxtreme
Copy link
Contributor

Thank you for doing this !

/lgtm

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

LGTM label has been added.

Git tree hash: 021a8f0ca4d854e3803b3907c632943b56eddfe1

@tomkivlin
Copy link
Contributor Author

Thank you for doing this !

/lgtm

No worries, thanks for the feedback and support.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks

/approve

In a Pod, or pod template, you can mark a reference to a ConfigMap as _optional_.
If the ConfigMap is non-existent, the configuration for which it provides data in the Pod (e.g. environment variable, mounted volume) will be empty.
You can mark a reference to a ConfigMap as _optional_ in a Pod specification.
If the ConfigMap doesn't exist, the configuration for which it provides data in the Pod (e.g. environment variable, mounted volume) will be empty.
Copy link
Contributor

Choose a reason for hiding this comment

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

A nit: we avoid Latin phrases and their abbreviations.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 Jul 26, 2022
@k8s-ci-robot k8s-ci-robot merged commit fb8577a into kubernetes:main Jul 26, 2022
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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConfigMap documentation does not cover optional functionality
4 participants