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 pkg/kubelet/config to v1beta3 #5630

Closed
bgrant0607 opened this issue Mar 19, 2015 · 8 comments
Closed

Update pkg/kubelet/config to v1beta3 #5630

bgrant0607 opened this issue Mar 19, 2015 · 8 comments
Assignees
Labels
area/kubelet priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.
Milestone

Comments

@bgrant0607
Copy link
Member

This isn't strictly blocking v1beta3 enablement #5475, nor did it block elimination of BoundPods. However, at some point prior to 1.0 we're going to nuke ContainerManifest and v1beta1. Kubelet should start consuming v1beta3 sooner rather than later.

Additionally, pkg/kubelet/config/http_test.go serializes api.ContainerManifest in several places. The internal representation should never be serialized. This is blocking removal of the json tags from pkg/api/types.go #3933.

@bgrant0607 bgrant0607 added priority/backlog Higher priority than priority/awaiting-more-evidence. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Mar 19, 2015
@bgrant0607 bgrant0607 added this to the v1.0 milestone Mar 19, 2015
@yujuhong
Copy link
Contributor

There is a WIP PR for reading pods: #5598. BoundPods have been removed in kubelet already.

#5632 to fix http_test.go

@wojtek-t
Copy link
Member

Thanks for the http_test.go fix @yujuhong !

cc @vmarmol
As for this issue, I think that #3372 is a prerequisite for this. I'm currently working on adding a support for reading Pods via http source (#5598). Once done with it, I will also modify the file channel.

As soon as both are done, I think we can start thinking of getting rid of support for ContainerManifest from there. I'm also happy to work on it.
@bgrant0607 : What deprecation policy do we have? Can we just simply remove support for ContainerManifest (obviously assuming that all tests & scripts are working correctly) or should we do something before it?

@wojtek-t wojtek-t self-assigned this Mar 19, 2015
@vmarmol
Copy link
Contributor

vmarmol commented Mar 19, 2015

@wojtek-t since the Container VM relies on, and we publish examples using, the ContainerManifest I don't think we can remove our ability to take it in just yet. We should at least announce the deprecation in Container VMs and change all our examples. A tool to change manifests would be nice too, I think we can do that today easily as well.

@wojtek-t
Copy link
Member

Thanks Victor. Just few questions:

  1. How/where should we announce deprecation?
  2. Can you point me to Container VM code/examples so that I can change them?
    Once Kubelet supports reading pods via http & config file, I'm happy to start changing our examples (and also create such tool).

@bgrant0607
Copy link
Member Author

@vmarmol @wojtek-t ContainerVM is not a product. It is used by internal teams. However, we're not committed to continuing to improve and evolve ContainerVM for them. We should announce the deprecation of ContainerVM ASAP. We're definitely going to nuke it along with the rest of v1beta1 if not sooner.

@dchen1107
Copy link
Member

@bgrant0607 That is what I thought for a while until EricH pointed me this public page: https://cloud.google.com/compute/docs/containers/container_vms

Sounds like we talked about it, but no real action is performed.

@wojtek-t
Copy link
Member

As soon as #3372 is done (all necessary PRs are already under review), Kubelet will support reading both Pods and ContainerManifest. I looked through the Kubernetes codebase and it seems that there are no references to ContainerManifest anywhere.

So I think that the only remaining work here would be related to ContainerVM. However, from the internal discussions it seems that there is still no consensus what we're going to do with ContainerVM. So I'm not sure if we can do anything about it right now.

@wojtek-t
Copy link
Member

pkg/kubelet/config currently supports v1beta3 (in addition to ContainerManifest).
Since there is no clear plan what to do with ContainerManifest (and since removing is part of getting rid of v1beta1), I'm closing this one as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubelet priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

No branches or pull requests

5 participants