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

kata-manager: Add support for Docker CLI installation #8376

Conversation

fidencio
Copy link
Member

@fidencio fidencio commented Nov 3, 2023

Add support for also installing the Docker CLI, giving users the chance
to try Kata Containers with docker in the same way we provide users the
chance to try Kata Containers with ctr.

Fixes: #8357

@katacontainersbot katacontainersbot added the size/medium Average sized task label Nov 3, 2023
Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fidencio - A few comments...

utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
@fidencio fidencio force-pushed the topic/kata-manager-add-support-for-docker-installation branch 2 times, most recently from 156d817 to ed62d66 Compare November 6, 2023 09:18
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
@fidencio fidencio force-pushed the topic/kata-manager-add-support-for-docker-installation branch from 86a7676 to c2cc918 Compare November 6, 2023 13:59
@katacontainersbot katacontainersbot added size/large Task of significant size and removed size/medium Average sized task labels Nov 6, 2023
@fidencio
Copy link
Member Author

fidencio commented Nov 6, 2023

@jodh-intel, addressed all the comments, and have successfully tested it locally.

@fidencio
Copy link
Member Author

fidencio commented Nov 6, 2023

/test

@fidencio
Copy link
Member Author

fidencio commented Nov 6, 2023

/retest-arm-unit

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fidencio - Tests ok but a few final comments...

utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
utils/kata-manager.sh Outdated Show resolved Hide resolved
Right now we're only testing with `ctr` and there's no change in
behaviour with this commit.  However, allowing to pass a tool to run the
tests with gives us an easier time when expanding kata-manager to
support, for instance, docker and nerdctl.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio fidencio force-pushed the topic/kata-manager-add-support-for-docker-installation branch from 37fca1f to 67e9685 Compare November 9, 2023 10:33
@fidencio
Copy link
Member Author

fidencio commented Nov 9, 2023

@jodh-intel, this is, yet again, ready for your review.

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @fidencio - just one non-blocking nit.

utils/kata-manager.sh Outdated Show resolved Hide resolved
@jodh-intel
Copy link
Contributor

@fidencio - Related: could you tal @ #8374 and #8383 when you get a chance?

Add support for also installing the Docker CLI, giving users the chance
to try Kata Containers with docker in the same way we provide users the
chance to try Kata Containers with `ctr`.

Fixes: kata-containers#8357

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
As "/etc/containerd/config.toml" is used from more than one place, let's
just make it a global var.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio fidencio force-pushed the topic/kata-manager-add-support-for-docker-installation branch from 1b03052 to 5d10aed Compare November 9, 2023 12:48
@fidencio
Copy link
Member Author

fidencio commented Nov 9, 2023

@fidencio - Related: could you tal @ #8374 and #8383 when you get a chance?

I will as soon as this one gets merged.

@fidencio
Copy link
Member Author

fidencio commented Nov 9, 2023

/test

Copy link
Contributor

@cmaf cmaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @fidencio

@amshinde amshinde merged commit 21e45be into kata-containers:main Nov 10, 2023
140 of 142 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kata-manager | Add support for installing a version of docker that does support kata-containers
5 participants