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

Sidecar Containers #753

Open
13 of 15 tasks
Joseph-Irving opened this issue Jan 29, 2019 · 228 comments
Open
13 of 15 tasks

Sidecar Containers #753

Joseph-Irving opened this issue Jan 29, 2019 · 228 comments
Assignees
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lead-opted-in Denotes that an issue has been opted in to a release sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team
Milestone

Comments

@Joseph-Irving
Copy link
Member

Joseph-Irving commented Jan 29, 2019

Enhancement Description

/sig node

Please keep this description up to date. This will help the Enhancement Team to track the evolution of the enhancement efficiently.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jan 29, 2019
@Joseph-Irving
Copy link
Member Author

@enisoc @dchen1107 @fejta @thockin @kow3ns @derekwaynecarr, opened this tracking issue so that we can discuss.

@kow3ns
Copy link
Member

kow3ns commented Jan 31, 2019

/assign

@Joseph-Irving
Copy link
Member Author

@derekwaynecarr I've done some scoping out of the kubelet changes required for next week's sig-node meeting, I believe that changes are only needed in the kuberuntime package, specifically kuberuntime_manager.go in and kuberuntime_container.go.

In kuberuntime_manager.go you could modify computePodActions to implement the shutdown triggering (kill sidecars when all non-sidecars have permanently exited), and starting up the sidecars first.

In kuberuntime_container.go you could modify killContainersWithSyncResult for terminating the sidecars last and sending the preStop hooks (the preStop hooks bit was a bit debatable, it wasn't settled whether that should be done or not. @thockin had a good point about why you might not want to encourage that behaviour, see comment).

Let me know if you want me to investigate any further.

@resouer
Copy link

resouer commented Feb 1, 2019

@kow3ns The discussion makes more sense to me if maybe we can define a full description of containers sequence in Pod spec (sig-app), and how to handle the sequence in kubelet for start, restart and cascading consideration (sig-node). Let's catch the Feb 5 sig-node meeting to give more inputs.

cc @Joseph-Irving

@luksa
Copy link

luksa commented Feb 7, 2019

The proposal says that sidecars only run after the init containers run. But what if the use-case requires the sidecar to run while/before the init containers run. For example, if you'd like route the pod's traffic through a proxy running as a sidecar (as in Istio), you probably want that proxy to be in place while the init containers run in case the init container itself does network calls.

@Joseph-Irving
Copy link
Member Author

@luksa I think there's the possibility of looking at having sidecars that run in init phase at some point but currently the proposal is not going to cover that use case. There is currently no way to have concurrent containers running in the init phase so that would be potentially a much larger/messier change than what is being suggested here.

@Joseph-Irving
Copy link
Member Author

Update on this KEP:
I've spoken to both @derekwaynecarr and @dchen1107 from sig-node about this and they did not express any major concerns about the proposal. I will raise a PR to the KEP adding some initial notes around implementation details and clarifying a few points that came up during the discussion.

We still need to agree on the API, it seems there is consensus that a simple way of marking containers as sidecars is prefered over more in depth ordering flags. Having a bool is somewhat limiting though so perhaps something more along the lines of containerLifecycle: Sidecar would be preferable so that we have the option of expanding in the future.

@luksa
Copy link

luksa commented Feb 14, 2019

@Joseph-Irving Actually, neither the boolean nor the containerLifecycle: Sidecar are appropriate for proper future extensibility. Instead, containerLifecycle should be an object, just like deployment.spec.strategy, with type: Sidecar. This would allow us to then introduce additional fields. For the "sidecar for the whole lifetime of the pod" solution, it would be expressed along these lines:

containerLifecycle: 
  type: Sidecar
  sidecar:
    scope: CompletePodLifetime

as opposed to

containerLifecycle: 
  type: Sidecar
  sidecar:
    scope: AfterInit

Please forgive my bad naming - I hope the names convey the idea.

But there is one problem with the approach where we introduce containerLifecycle to pod.spec.containers. Namely, it's wrong to have sidecars that run parallel to init containers specified under pod.spec.containers. So if you really want to be able to extend this to init containers eventually, you should find an alternative solution - one that would allow you to mark containers as sidecars at a higher level - i.e. not under pod.spec.containers or pod.spec.initContainers, but something like pod.spec.sidecarContainers, which I believe you already discussed, but dismissed. The init containers problem definitely calls for a solution along these lines.

@Joseph-Irving
Copy link
Member Author

@luksa You could also solve the init problem by just allowing an init container to be marked as a sidecar and have that run alongside the init containers. As I understand it, the problem is that init containers sometimes need sidecars, which is different from needing a container that runs for the entire lifetime of the pod.

The problem with pod.spec.sidecarContainers is that it's a far more complex change, tooling would need to updated and the kubelet would require a lot of modifying to support another set of containers. The current proposal is far more modest, it's only building on what's already there.

@luksa
Copy link

luksa commented Feb 14, 2019

@Joseph-Irving We could work with that yes. It's not ideal for the sidecar to shut down after the init containers run and then have the same sidecar start up again, but it's better than not having that option. The bigger problem is that older Kubelets wouldn't handle init-sidecar containers properly (as is the case with main-sidecar containers).

I'd just like you to keep init-sidecars in mind when finalizing the proposal. In essence, you're introducing the concept of "sidecar" into k8s (previously, we basically only had a set of containers that were all equal). Now you're introducing actual sidecars, so IMHO, you really should think this out thoroughly and not dismiss a very important sidecar use-case.

I'd be happy to help with implementing this. Without it, Istio can't provide its features to init containers (actually, in a properly secured Kubernetes cluster running Istio, init containers completely lose the ability to talk to any service).

@Joseph-Irving
Copy link
Member Author

In relation to the implementation discussion in #841, I've opened a WIP PR containing a basic PoC for this proposal kubernetes/kubernetes#75099. It's just a first draft and obviously not perfect but the basic functionality works and gives you an idea of the amount of change required.

cc @enisoc

@Joseph-Irving
Copy link
Member Author

I put together a short video just showing how the PoC currently behaves https://youtu.be/4hC8t6_8bTs. Seeing it in action can be better than reading about it.
Disclaimer: I'm not a pro youtuber.

@Joseph-Irving
Copy link
Member Author

I've opened two new PRs:

Any thoughts or suggestions will be much appreciated.

@SergeyKanzhelev
Copy link
Member

/milestone v1.33
/label lead-opted-in

As discussed, this KEP is going GA in 1.33

@k8s-ci-robot k8s-ci-robot added this to the v1.33 milestone Jan 23, 2025
@k8s-ci-robot k8s-ci-robot added the lead-opted-in Denotes that an issue has been opted in to a release label Jan 23, 2025
@haircommander haircommander moved this from Triage to Proposed for consideration in SIG Node 1.33 KEPs planning Jan 31, 2025
@lzung
Copy link

lzung commented Feb 2, 2025

Hello @SergeyKanzhelev @Joseph-Irving 👋, v1.33 Enhancements team here.

Just checking in as we approach enhancements freeze on 02:00 UTC Friday 14th February 2025 / 19:00 PDT Thursday 13th February 2025.

This enhancement is targeting stage stable for v1.33 (correct me, if otherwise)
/stage stable

Here's where this enhancement currently stands:

  • KEP readme using the latest template has been merged into the k/enhancements repo.
  • KEP status is marked as implementable for latest-milestone: v1.33.
  • KEP readme has up-to-date graduation criteria
  • KEP has a production readiness review that has been completed and merged into k/enhancements. (For more information on the PRR process, check here). If your production readiness review is not completed yet, please make sure to fill the production readiness questionnaire in your KEP by the PRR Freeze deadline on Thursday 6th February 2025 so that the PRR team has enough time to review your KEP.

For this KEP, we would just need to update the following:

The status of this enhancement is marked as At risk for enhancements freeze. Please keep the issue description up-to-date with appropriate stages as well.

If you anticipate missing enhancements freeze, you can file an exception request in advance. Thank you!

@k8s-ci-robot k8s-ci-robot added stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status and removed stage/beta Denotes an issue tracking an enhancement targeted for Beta status labels Feb 2, 2025
@lzung lzung moved this to At risk for enhancements freeze in 1.33 Enhancements Tracking Feb 2, 2025
@SergeyKanzhelev
Copy link
Member

@lzung I believe all criteria are met now

@lzung
Copy link

lzung commented Feb 5, 2025

Hello @SergeyKanzhelev 👋,

Now that PR #5081 has been merged, all the KEP requirements are in place and merged into k/enhancements. This enhancement is all good for the upcoming enhancements freeze. 🚀

The status of this enhancement is now marked as tracked for enhancement freeze. Thank you!

@lzung lzung moved this from At risk for enhancements freeze to Tracked for enhancements freeze in 1.33 Enhancements Tracking Feb 5, 2025
@lzung lzung added the tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team label Feb 5, 2025
@SergeyKanzhelev SergeyKanzhelev moved this from Proposed for consideration to Tracked in SIG Node 1.33 KEPs planning Feb 7, 2025
@SergeyKanzhelev SergeyKanzhelev moved this from Tracked to Implemented in SIG Node 1.33 KEPs planning Feb 7, 2025
@rayandas
Copy link
Member

Hello @SergeyKanzhelev @Joseph-Irving 👋, v1.33 Docs Lead here.

Does this enhancement work planned for v1.33 require any new docs or modification to existing docs?

If so, please follow the steps here to open a PR against dev-1.33 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday 27th February 2025 18:00 PDT.

Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.

Thank you!

@Udi-Hofesh
Copy link
Member

Hey @SergeyKanzhelev 👋 -- this is Udi (@Udi-Hofesh) from the 1.33 Communications Team!

For the 1.33 release, we are currently in the process of collecting and curating a list of potential feature blogs, and we'd love for you to consider writing one for your enhancement!

As you may be aware, feature blogs are a great way to communicate to users about features that fall into (but are not limited to) the following categories:

  • This introduces some breaking change(s)
  • This has significant impacts and/or implications to users
  • ...Or this is a long-awaited feature, which would go a long way to cover the journey more in detail 🎉

To opt in to write a feature blog, could you please let us know and open a "Feature Blog placeholder PR" (which can be only a skeleton at first) against the website repository by Wednesday, 5th March, 2025? For more information about writing a blog, please find the blog contribution guidelines 📚

Tip

Some timelines to keep in mind:

  • 02:00 UTC Wednesday, 5th March, 2025: Feature blog PR freeze
  • Monday, 7th April, 2025: Feature blogs ready for review
  • You can find more in the release document

Note

In your placeholder PR, use XX characters for the blog date in the front matter and file name. We will work with you on updating the PR with the publication date once we have a final number of feature blogs for this release.

@lzung
Copy link

lzung commented Mar 3, 2025

Hey again @SergeyKanzhelev @Joseph-Irving 👋, 1.33 Enhancements team here,

Just checking in as we approach code freeze at 02:00 UTC Friday 21st March 2025 / 19:00 PDT Thursday 20th March 2025.

Here's where this enhancement currently stands:

  • All PRs to the Kubernetes repo that are related to your enhancement are linked in the above issue description (for tracking purposes).
  • All PR/s are ready to be merged (they have approved and lgtm labels applied) by the code freeze deadline. This includes tests.

With all the implementation(code related) PRs merged as per the issue description:

Additionally, please let me know if there are any other PRs in k/k not listed in the description that we should track for this KEP, so that we can maintain accurate status.

This enhancement is now marked as tracked for code freeze for the 1.33 Code Freeze!

@lzung lzung moved this from Tracked for enhancements freeze to Tracked for code freeze in 1.33 Enhancements Tracking Mar 3, 2025
@Udi-Hofesh
Copy link
Member

Hi @SergeyKanzhelev 👋, 1.33 Communications Team here again!

This is a gentle reminder for the feature blog deadline mentioned above, which is 02:00 UTC Wednesday, 5th March, 2025. This is in just several hours! To opt in, please let us know and open a Feature Blog placeholder PR against k/website by the deadline. If you have any questions, please feel free to reach out to us!

Tip

Some timeline to keep in mind:

  • 02:00 UTC Wednesday, 5th March, 2025: Feature blog PR freeze
  • Monday, 7th April, 2025: Feature blogs ready for review
  • You can find more in the release document

Note

In your placeholder PR, use XX characters for the blog date in the front matter and file name. We will work with you on updating the PR with the publication date once we have a final number of feature blogs for this release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lead-opted-in Denotes that an issue has been opted in to a release sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/node Categorizes an issue or PR as relevant to SIG Node. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team
Projects
Status: Removed From Milestone
Status: Tracked
Status: Tracked for Code Freeze
Status: Tracked for code freeze
Status: Backlog
Status: Implemented
Status: Not for release
Development

No branches or pull requests