-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Build Kubelet without Docker #1546
Build Kubelet without Docker #1546
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dims there were a couple of open questions I had left over from our conversation that I tagged you in - thanks :)
@dims and I had some chats about the benefits of this workstream. Would love to know y'alls thoughts on next steps? cc @BenTheElder would be curious to get your opinion/expertise given your work on this w/ sig-cloud-provider. |
/retest |
4b387b8
to
47b3939
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quick skim 👀
An alternative to consider:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove unused optional headings; remove [optional]
mark from optional headings that are selected.
47b3939
to
5a60767
Compare
Thanks - fixed these :) |
5a60767
to
ba620d6
Compare
Thanks for your responses @dims :) Updated my pr with your answers. |
/assign @dchen1107 @derekwaynecarr @Random-Liu From conversations with @dims, I think this KEP is ready for review when y'all get the chance. I will be working on getting the POC PR in a mergeable state in parallel. Thanks :) |
ba620d6
to
3586aa0
Compare
/milestone v1.19 |
/cc |
Thanks for that additional data! Definite +1 from me that we want to be very careful about the path forward to actually remove Docker support entirely from the Kubelet. It requires more discussion and thought. Hopefully this KEP makes testing out different paths, and ultimately pursuing one, easier. |
thank you for helping with this effort @mattjmcnaughton |
3586aa0
to
a85ae0e
Compare
My pleasure, thanks for adding your insights :) |
a85ae0e
to
8547df3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make the following updates:
- drop story 2
- add under test-criteria "creation of a pre-submit job to ensure we dont regress on dockerless build"
- add under test-criteria "creation of a node e2e job that passes dockerless"
the node e2e suite itself has docker deps, and so it will also want to get updated to support building a "dockerless" version so that it doesn't inadvertently regress into including them.
components. This desire leads us to our second question: is supporting | ||
compiling a dockerless Kubelet an appropriate first step? | ||
|
||
We argue yes. First, the work to support compiling a dockerless Kubelet will |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with everyone enumerated here.
|
||
## Proposal | ||
|
||
We will undertake the following steps to obtain our goals. First, we will ensure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe this impacts sig-windows, but everything here makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth adding them as a participating sig? Or just something for folks to remain cognizant of moving forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add it as a participating sig unless they have fully transitioned to an alternative.
Golang library, we will add automated tooling enforcing that only the | ||
`dockershim` can depend on `docker/docker`. | ||
|
||
One quick additional note - currently `cadvisor` also depends on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree this is orthogonal as there is also work to eliminate/reduce cadvisor dep from kubelet itself.
Thanks for the feedback @derekwaynecarr - will address within the next day or two, and will ping you when I do. |
A KEP proposing supporting building a Kubelet without docker, via the use of build tags.
8547df3
to
60b59a5
Compare
@derekwaynecarr I updated the testing section with the tests you requested. Please let me know if there's anything I misinterpreted or missed. I also deleted the second story. W.r.t. kubernetes/kubernetes#87746 (the implementation PR), it currently includes the first test type, but not the second, third, and forth. Do we want to block merging the PR on having all the new tests in place, or merge the PR as is and add them later? I'm open to either path, with a slight preference for keeping PRs as small as possible :) cc @dims |
@mattjmcnaughton my preference is to merge this as-is and file subsequent PRs. there is no requirement that one KEP == one PR :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for enumerating your implementation plan.
I look forward to assisting you in making this succesful.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, mattjmcnaughton 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 |
w00t!!! thanks @derekwaynecarr |
Wonderful news - big thanks @derekwaynecarr ! |
this is pretty cool :-) |
cc @dims what, if anything, do we need to do to get this KEP included in 1.19? Re https://groups.google.com/forum/#!topic/kubernetes-dev/G5iIu3FrRTY Or is nothing necessary because its not user facing? |
i think this KEP can move to |
@mattjmcnaughton like @neolit123 mentioned, please flip to implementable ( example https://github.com/kubernetes/enhancements/pull/1715/files#diff-215637d1a7a561ceea8855c304720e82R10 ) |
…ng-cpu-limits OCPEDGE-808: feat: add ep for cpu limits with workload partitioning
A KEP proposing supporting building a Kubelet without any "Docker-specific" code or dependency on the
docker/docker
Golang library, via the use of build tags.