-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: docker working even without containerd #3796
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
base: master
Are you sure you want to change the base?
Conversation
Only initialize containderd for docker if StorageDriver is ContainerdSnapshotterStorageDriver
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@benji2k2 please file the CLA |
done |
|
There also seems to be an error in my pr - probably containerdClient have to be declared before the if-condition: Sorry, I’m not very familiar with Go... Maybe you can fix that for me. Thanks :) |
Declared Variable
container/docker/factory.go
Outdated
|
|
||
| var ( | ||
| containerdClient *containerd.ContainerdClient | ||
| ) |
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.
Move this to line 334, that's where all variables are declared :)
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.
done
moved Variable containerdClient to other variables
Changes type (no pointer)
Correct assignment of variable containerdClient Co-authored-by: Sambhav Jain <67923444+sambhav-jain-16@users.noreply.github.com>
|
please update the PR title to reflect what the PR does. Please add more details in the PR body (you just have one sentence there) |
|
I updated title and description. But i do not see the formatting issue in line 357. can someone take a look for me? I have no Development Environment for Go. |
|
@benji2k2 |
|
@sambhav-jain-16 thanks for the hint, i generally know how it works :) but as already mentioned i don't have a development environment for go installed, i just edited the code in the github web ide to fix the bug. therefore "make format" does not work for me without setting up go. can you maybe do this step for me? thanks in advance... |
|
@benji2k2 I tried pushing the format fix, but I don't have the permissions to write to your fork. Can you provide me with the same? Thanks :) |
@sambhav-jain-16 you got invitation to my fork |
In 0.54.0 containerd-snapshotter support was added. But old versions of docker do not support containerd-snapshotter. unfortunately the containerd-client is always initialized - even when ContainerdSnapshotterStorageDriver is not used by docker. this leads to error initializing docker factory for older docker versions.
With this fix containerd-client will only initiallized when ContainerdSnapshotterStorageDriver is used by docker. Therefore older docker versions will work again.