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

update SidecarContainers to stable in v1.33 #49685

Open
wants to merge 2 commits into
base: dev-1.33
Choose a base branch
from

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Feb 8, 2025

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language labels Feb 8, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sftim for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 8, 2025
@pacoxu
Copy link
Member Author

pacoxu commented Feb 8, 2025

/cc @SergeyKanzhelev

Copy link

netlify bot commented Feb 8, 2025

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 4087aa6
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/67ac6b96e663ed0008cd5982

@pacoxu pacoxu force-pushed the sidecar-containers-stable branch from 65711f3 to 6bca998 Compare February 8, 2025 07:30
@pacoxu
Copy link
Member Author

pacoxu commented Feb 8, 2025

/cc @gjkim42

@k8s-ci-robot k8s-ci-robot requested a review from gjkim42 February 8, 2025 07:31
Copy link

netlify bot commented Feb 8, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 65711f3
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67a7076827d6c40008695c4a
😎 Deploy Preview https://deploy-preview-49685--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 configuration.

Copy link

netlify bot commented Feb 8, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 4087aa6
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/67ac6b96da2cb40008d27538
😎 Deploy Preview https://deploy-preview-49685--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 configuration.

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, but we also need to reword https://kubernetes.io/docs/concepts/workloads/pods/sidecar-containers/#pod-sidecar-containers because the feature gate will no longer control behavior.

You should change this page or ask another contributor to help with the revision.


@pacoxu if you can rally someone from SIG Node who is keen to help, https://kubernetes.io/docs/tutorials/configuration/pod-sidecar-containers/ is not yet quite at the quality level we'd like for beta. That fact doesn't block graduation but if we can improve that tutorial and polish it more, even better.

@kundan2707
Copy link
Contributor

Preview

…-gates/SidecarContainers.md

Co-authored-by: Dipesh Rawat <[email protected]>
@pacoxu
Copy link
Member Author

pacoxu commented Feb 12, 2025

I find that #46525 did some rephrase based on @matthyx 's #43471 beta update doc PR. https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#termination-with-sidecars(and some context above this )

@matthyx @SergeyKanzhelev for GA, do we want to publish a blog?

@pacoxu if you can rally someone from SIG Node who is keen to help, https://kubernetes.io/docs/tutorials/configuration/pod-sidecar-containers/ is not yet quite at the quality level we'd like for beta. That fact doesn't block graduation but if we can improve that tutorial and polish it more, even better.

I could help on improve that part based on merged KEP and merged PRs.
/hold
@sftim Do you have a clear optimization direction, such as wanting the description to be clearer, adding more technical details, providing additional examples, or including use cases? I may proposal some later for review.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2025
@matthyx
Copy link
Contributor

matthyx commented Feb 12, 2025

@matthyx @SergeyKanzhelev for GA, do we want to publish a blog?

yes and a celebration in London !

@sftim
Copy link
Contributor

sftim commented Feb 12, 2025

To improve https://kubernetes.io/docs/tutorials/configuration/pod-sidecar-containers/:

  • make it a tutorial about using sidecar containers, not a tutorial about switching to the initContainers-that-stay-running paradigm

  • actually demonstrate a sidecar; for example, have a main app container that displays a trending fruit (we use fruit in examples such as https://kubernetes.io/docs/tasks/job/parallel-processing-expansion/) on a webpage. Accompany that with a sidecar that picks a random fruit to be the trending one.

  • actually demonstrate a sidecar that starts before a later init container and also before an app container
    (maybe the init container writes a page that reads "We don't have any trending fruit yet"

  • maybe also demonstrate a sidecar container that reads from a (mounted) log file and writes entries to its stdout

@sftim
Copy link
Contributor

sftim commented Feb 12, 2025

On the tutorial, I also recommend illustrating general good practice; for example, have a Pod template with a selector on kubernetes.io/os rather than assume that the cluster is all Linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. language/en Issues or PRs related to English language size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants