Borrow api module from containerd (instead of vendoring it) #3145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dims
force-pushed
the
borrow-api-module-from-containerd
branch
5 times, most recently
from
12fe9fd
to
f82338c
Compare
|
experiment was ✅ Looks like the presubmit(s) are broken because of golang changes. Let's formalize this next week. |
|
related to containerd/containerd#7231 |
|
@dims Is this a workaround/hack that we are going to follow for this release? |
|
@akhilerm yes, that would be my proposal. |
dims
force-pushed
the
borrow-api-module-from-containerd
branch
3 times, most recently
from
42dcb61
to
45d1234
Compare
dims
changed the title
dims
changed the title
|
Hmm, I'm hitting some issues running the update script myself: |
|
Thank you Dims for working on this, I think this will be a great workaround until containerd will have a proper API go module. |
bobbypage
reviewed
Aug 1, 2022
|
@bobbypage what version of |
This is on my linux box ~ Debian Testing |
|
fixed that wildcards problem @bobbypage (looks like i did this work on my macos which had the older version of tar) |
dims
force-pushed
the
borrow-api-module-from-containerd
branch
2 times, most recently
from
009beda
to
c508503
Compare
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
dims
force-pushed
the
borrow-api-module-from-containerd
branch
from
c508503
to
6b2d4b3
Compare
|
LGTM! |
|
FYI, this has completely broken projects like K3s that try to import both Containerd and Kubelet code into the same project. Containerd and the kubelet's cadvisor end up both registering the same types as part of package init. This was not a problem prior to this commit. |
|
@brandond just to make sure whoever is looking at this. The Kubernetes team does not support the use case of kubelet code imported into other projects/products as mentioned in our README - https://github.com/kubernetes/kubernetes/#to-start-using-k8s pasting the snippet here: |
|
Yes, we're aware that we are doing naughty things by mashing everything all together and mixing it up with a spoon. It mostly works, except when it doesn't. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We only need the
apifolder from https://github.com/containerd/containerd project. So we have a script that fetches a zip for a specific version (latest as of now) and sticks it into athird_party/containerddirectory. We can run this script as and when needed and check in the code. We do this until containerd/containerd#7231 is fixed on containerd-side.Note that we patch the imports as well as we are creating a new directory/package structure here as well.