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.

Sign up for GitHub

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

Jump to bottom

Use cobra #373

Merged

AkihiroSuda merged 1 commit into containerd:master from robberphex:release/use-cobra

Oct 14, 2021

Contributor

robberphex commented Sep 18, 2021 •

edited

switch to spf13/cobra
- support nerdctl build . -t a:b
- support completion for fish/powershell/zsh
run/exec support --
add test for exec command
add test for envvar CONTAINERD_NAMESPACE support.
- testutil.go support overrice envvar
for image without repository/tag, show <none>, keep compatible with docker.(images command)
support logout completion

robberphex force-pushed the release/use-cobra branch from 1b8371b to d1dcfad Compare

September 18, 2021 19:18

AkihiroSuda added this to the v0.12.0 milestone

AkihiroSuda added impact/major kind/refactor labels

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/build.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/build.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/build.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/commit.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/completion.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/completion.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/compose.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/container_inspect.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/events.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/exec.go Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/image_convert.go Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/main_linux.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/logout.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/rm.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/run.go Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/run.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/run.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/run.go Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

pkg/imgutil/imgutil.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

pkg/netutil/nettype/nettype.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/main.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/main.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/main.go Show resolved Hide resolved

AkihiroSuda requested changes

View reviewed changes

Member

AkihiroSuda left a comment

Thanks, but please make sure env vars such as CONTAINERD_NAMESPACE are still supported.
And please clean up debug codes.

AkihiroSuda marked this pull request as draft

September 21, 2021 05:50

robberphex force-pushed the release/use-cobra branch from d1dcfad to 04ed299 Compare

September 21, 2021 13:08

robberphex force-pushed the release/use-cobra branch from 0b7c12d to b35c71c Compare

October 13, 2021 07:06

ktock reviewed

View reviewed changes

cmd/nerdctl/images.go Outdated Show resolved Hide resolved

Member

ktock commented Oct 13, 2021

As mentioned in #373 (comment), nerdctl image rm doesn't work (and fails silently)

# nerdctl images
REPOSITORY                           TAG           IMAGE ID        CREATED           SIZE
ghcr.io/stargz-containers/busybox    1.32.0-org    bde48e175117    14 seconds ago    1.3 MiB
# nerdctl image rm bde48e175117
# echo $?
0
# nerdctl images
REPOSITORY                           TAG           IMAGE ID        CREATED           SIZE
ghcr.io/stargz-containers/busybox    1.32.0-org    bde48e175117    24 seconds ago    1.3 MiB

robberphex force-pushed the release/use-cobra branch 3 times, most recently from eb9bf57 to 67868e7 Compare

October 13, 2021 08:07

Contributor Author

robberphex commented Oct 13, 2021

@ktock I reverted related change for nerdctl image rm doesn't work.

Member

AkihiroSuda commented Oct 13, 2021 •

edited

nerdctl completion bash does not seem to work when the daemon is not running

$ nerdctl completion bash
FATA[0000] rootless containerd not running? (hint: use `containerd-rootless-setuptool.sh install` to start rootless containerd): stat /run/user/1001/containerd-rootless: no such file or directory

appNeedsRootlessParentMain() in main_linux.go seems receiving empty args []string.

robberphex force-pushed the release/use-cobra branch from 67868e7 to b05b752 Compare

October 14, 2021 03:18

Contributor Author

robberphex commented Oct 14, 2021

@AkihiroSuda
"nerdctl completion bash does not seem to work when the daemon is not running" is fixed now.

robberphex force-pushed the release/use-cobra branch from b05b752 to bd3c561 Compare

October 14, 2021 03:22

robberphex requested a review from AkihiroSuda

October 14, 2021 03:23

robberphex force-pushed the release/use-cobra branch from bd3c561 to 4854e28 Compare

October 14, 2021 03:30

Member

AkihiroSuda commented Oct 14, 2021

FAIL: TestCompletion (0.05s)
https://github.com/containerd/nerdctl/pull/373/checks?check_run_id=3890382139

robberphex force-pushed the release/use-cobra branch 2 times, most recently from 1774582 to 81c3122 Compare

October 14, 2021 11:00

AkihiroSuda reviewed

View reviewed changes

cmd/nerdctl/login.go Outdated Show resolved Hide resolved


          use cobra

fae1696

repalce spf13/cobra and spf13/pfla

Signed-off-by: luyanbo <robert.lyb@alibaba-inc.com>

robberphex force-pushed the release/use-cobra branch from 81c3122 to fae1696 Compare

October 14, 2021 11:22

robberphex requested a review from AkihiroSuda

October 14, 2021 11:45

AkihiroSuda approved these changes

View reviewed changes

Member

AkihiroSuda left a comment

Thanks, this was huge! ❤️

@ktock @fahedouch @fuweid
I'm merging this, but if you find regressions, please open issues (or PRs)

AkihiroSuda merged commit 29629b0 into containerd:master

AkihiroSuda mentioned this pull request

Cobra fixes #431

Merged

Member

AkihiroSuda commented Oct 14, 2021

follow-up PR #431

AkihiroSuda mentioned this pull request

migrate to cobra #354

Closed

robberphex deleted the release/use-cobra branch

October 17, 2021 05:49

Contributor

ningmingxiao commented Oct 20, 2021

why use cobra instead of cli?

Contributor Author

robberphex commented Oct 20, 2021

@ningmingxiao
With cobra, nerdctl can:

support nerdctl build . -t a:b. cli can only support nerdctl build -t a:b .
support completion for fish/powershell/zsh without any extra config.

This was referenced Oct 20, 2021

add restart container function #445

Closed

run: fix --rootfs (cobra regression) #447

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment