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

Add a new concept page for Sidecar containers #43346

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

T-Lakshmi
Copy link
Contributor

Fixes #42762

@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 Oct 6, 2023
@k8s-ci-robot k8s-ci-robot added 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. labels Oct 6, 2023
@shannonxtreme
Copy link
Contributor

Is this mostly a move of the existing content to the new concept page?

@T-Lakshmi
Copy link
Contributor Author

T-Lakshmi commented Oct 6, 2023

@shannonxtreme

Is this mostly a move of the existing content to the new concept page?

Yes, But there are few changes and addon points also.

@tamilselvan1102
Copy link

tamilselvan1102 commented Oct 6, 2023

I think below two changes are required

  1. Add label [Sidecar containers] and make a link with [sidecar-containers.md] page in the left side menu below the [Init Containers].
  2. Add [Sidecar containers] link from [whatsnext] in [Init Containers].

@T-Lakshmi
Copy link
Contributor Author

Hi @tamilselvan1102,

Add label [Sidecar containers] and make a link with [sidecar-containers.md] page in the left side menu below the [Init Containers].

Is this what you are talking about? If yes, it is already added to the PR.
image

@T-Lakshmi
Copy link
Contributor Author

@tamilselvan1102

Add [Sidecar containers] link from [whatsnext] in [Init Containers].

Updated, Thanks for remembering.

@sftim
Copy link
Contributor

sftim commented Oct 6, 2023

/sig node

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Oct 6, 2023
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. Here's some feedback.

@netlify
Copy link

netlify bot commented Oct 12, 2023

Pull request preview available for checking

Built without sensitive environment variables

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

@T-Lakshmi
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 12, 2023
@sftim
Copy link
Contributor

sftim commented Oct 20, 2023

Relevant to #43610

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 11, 2023
@k8s-ci-robot k8s-ci-robot requested a review from sftim December 11, 2023 12:09
@sftim
Copy link
Contributor

sftim commented Dec 11, 2023

LGTM for docs

@kubernetes/sig-node-pr-reviews does this look right, now?

@matthyx
Copy link
Contributor

matthyx commented Dec 11, 2023

LGTM for docs

@kubernetes/sig-node-pr-reviews does this look right, now?

Let me have a look today...

@matthyx
Copy link
Contributor

matthyx commented Dec 11, 2023

LGTM, however with 1.29 sidecars are going to beta and will be enabled by default.
@sftim do we want to already acknowledge that?

@sftim
Copy link
Contributor

sftim commented Dec 11, 2023

with 1.29 sidecars are going to beta and will be enabled by default.

I'll check with release docs. Personally, I think this is better merged than not.

@sftim
Copy link
Contributor

sftim commented Dec 11, 2023

/approve

/hold
See #43346 (comment)

@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 Dec 11, 2023
@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 Dec 11, 2023
@katcosgrove
Copy link
Contributor

I am fine with merging this into main as-is, then opening a PR against the dev-1.29 branch once I have frozen k/website.

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 11, 2023
@sftim
Copy link
Contributor

sftim commented Dec 11, 2023

LGTM from #43346 (comment)

/lgtm

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

LGTM label has been added.

Git tree hash: bd98d7ca54351db7ed66fa2617f182fa94176798

@k8s-ci-robot k8s-ci-robot merged commit 9b9d765 into kubernetes:main Dec 11, 2023
6 checks passed
@sftim sftim mentioned this pull request Dec 11, 2023
@sftim
Copy link
Contributor

sftim commented Dec 12, 2023

#44305 adapts that change for v1.29

@T-Lakshmi
Copy link
Contributor Author

#44305 adapts that change for v1.29

@sftim,
Yes, I saw the PR and merge changes in the dev1.29 branch docs.
Meanwhile, as sidecars are moving to beta state, I believe we should remove the text below from sidecar containers page.

By default, this feature is not available in Kubernetes. To avail this feature, you need to enable the feature gate named SidecarContainers.

WDYT?

@sftim
Copy link
Contributor

sftim commented Dec 12, 2023

Does that text appear on the dev-1.29 branch? See https://deploy-preview-43082--kubernetes-io-vnext-staging.netlify.app/docs/ for the preview of that branch.

@T-Lakshmi
Copy link
Contributor Author

Yes, It's there in dev-1.29 branch.
I could see this text at the end of the section in the preview page.

@sftim
Copy link
Contributor

sftim commented Dec 12, 2023

OK. Would you be willing to send in a PR to fix that bug? You can branch from main shortly after the release that is scheduled for later today.

@T-Lakshmi
Copy link
Contributor Author

Sure, I will work on it.
Thanks.

prashantrewar pushed a commit to prashantrewar/website that referenced this pull request Dec 30, 2023
Add a new concept page for Sidecar containers
@Adawntoremember
Copy link

fd224a1

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. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Development

Successfully merging this pull request may close these issues.

Unclear documentation about sidecar containers