Skip to content

nerdctl-stub: Allow positional arguments mixed with flags#8320

Merged
jandubois merged 3 commits into
rancher-sandbox:mainfrom
mook-as:nerdctl-stub/arg-order
Apr 17, 2025
Merged

nerdctl-stub: Allow positional arguments mixed with flags#8320
jandubois merged 3 commits into
rancher-sandbox:mainfrom
mook-as:nerdctl-stub/arg-order

Conversation

@mook-as

@mook-as mook-as commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Sometimes it would be convenient for the user to mix positional arguments with flags, e.g. nerdctl build --tag foo . --file Containerfile.custom. Support this case by parsing all flags, then dumping all positional arguments later. This can only work if no command contains both positional arguments as well as subcommands (so that we can reliably detect them); add a case in the help output parser to check this.

This also adds a yarn generate:nerdctl-stub --verbose that gets passed to the generator so it is easier to see what it is doing.

Fixes #8190

Comment thread src/go/nerdctl-stub/parse_args_test.go Fixed
@mook-as mook-as force-pushed the nerdctl-stub/arg-order branch from 12c7693 to 7fea042 Compare March 4, 2025 00:17
@mook-as mook-as requested a review from jandubois March 4, 2025 17:50
@jandubois

Copy link
Copy Markdown
Member

It looks like this is more complicated than I expected:

C:\Users\SUSE\build>nerdctl run busybox sh -c "echo Hello world"
2025/03/05 15:51:32 Error parsing arguments: command "container run" does not support option -c

The command runs, but the incorrect error message is not really acceptable.

Maybe there is a rule where you can't have additional [flags] if the command ends with optional arguments:

Usage: nerdctl run [flags] IMAGE [COMMAND] [ARG...]

But if you have only mandatory positional arguments, then they can be followed by more flags:

Usage: nerdctl build [flags] PATH

I haven't checked if that rule would hold everywhere, and it seems kind of too magical anyways. Unfortunately argument handling in nerdctl seems rather irregular.

@mook-as

mook-as commented Mar 6, 2025

Copy link
Copy Markdown
Contributor Author

I think we need to look for COMMAND followed by (possibly after some distance) .... This includes:

time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[compose exec]" type=arguments usage=" NERDCTL COMPOSE EXEC [FLAGS] SERVICE COMMAND [ARGS...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[compose run]" type=arguments usage=" NERDCTL COMPOSE RUN [FLAGS] SERVICE [COMMAND] [ARGS...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[container create]" type=positional usage=" NERDCTL CONTAINER CREATE [FLAGS] IMAGE [COMMAND] [ARG...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[container exec]" type=positional usage=" NERDCTL CONTAINER EXEC [FLAGS] CONTAINER COMMAND [ARG...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[container run]" type=positional usage=" NERDCTL CONTAINER RUN [FLAGS] IMAGE [COMMAND] [ARG...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[create]" type=positional usage=" NERDCTL CREATE [FLAGS] IMAGE [COMMAND] [ARG...]"
time="2025-03-05T16:36:26-08:00" level=trace msg="building subcommand" args="[exec]" type=positional usage=" NERDCTL EXEC [FLAGS] CONTAINER COMMAND [ARG...]"
time="2025-03-05T16:36:27-08:00" level=trace msg="building subcommand" args="[run]" type=positional usage=" NERDCTL RUN [FLAGS] IMAGE [COMMAND] [ARG...]"

Note the annoying [args...] vs [arg...].

Just looking for ... finds you things like

time="2025-03-05T16:34:51-08:00" level=trace msg="building subcommand" args="[compose build]" type=positional usage=" NERDCTL COMPOSE BUILD [FLAGS] [SERVICE...]"
time="2025-03-05T16:34:52-08:00" level=trace msg="building subcommand" args="[container inspect]" type=positional usage=" NERDCTL CONTAINER INSPECT [FLAGS] CONTAINER [CONTAINER, ...]"
time="2025-03-05T16:38:05-08:00" level=trace msg="building subcommand" args="[image convert]" type=positional usage=" NERDCTL IMAGE CONVERT [FLAGS] <SOURCE_REF> <TARGET_REF>..."
time="2025-03-05T16:38:05-08:00" level=trace msg="building subcommand" args="[image inspect]" type=positional usage=" NERDCTL IMAGE INSPECT [FLAGS] IMAGE [IMAGE...]"
time="2025-03-05T16:38:05-08:00" level=trace msg="building subcommand" args="[kill]" type=positional usage=" NERDCTL KILL [FLAGS] CONTAINER [CONTAINER, ...]"
time="2025-03-05T16:38:05-08:00" level=trace msg="building subcommand" args="[namespace remove]" type=positional usage=" NERDCTL NAMESPACE REMOVE [FLAGS] NAMESPACE [NAMESPACE...]"
time="2025-03-05T16:38:06-08:00" level=trace msg="building subcommand" args="[volume inspect]" type=positional usage=" NERDCTL VOLUME INSPECT [FLAGS] VOLUME [VOLUME...]"

mook-as added 2 commits April 11, 2025 16:43
Sometimes it would be convienent for the user to mix positional arguments
with flags, e.g. `nerdctl build --tag foo . --file Containerfile.custom`.
Support this case by parsing all flags, then dumping all positional
arguments later.  This can only work if no command contains both positional
arguments as well as subcommands (so that we can reliably detect them); add
a case in the help output parser to check this.

This also adds a `yarn generate:nerdctl-stub --verbose` that gets passed to
the generator so it is easier to see what it is doing.

Signed-off-by: Mark Yen <mark.yen@suse.com>
This fixes `nerdctl run ... -flag` to not warn on flags that are not to be
parsed by nerdctl.

Signed-off-by: Mark Yen <mark.yen@suse.com>
@jandubois jandubois force-pushed the nerdctl-stub/arg-order branch from ece9a9e to 420f92d Compare April 11, 2025 23:43
@jandubois

Copy link
Copy Markdown
Member

I've rebased this PR on latest main. I was testing the CI build, including running BATS, and essentially all tests involving RDX were failing. I want to have a recent CI build before digging in deeper.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>

@jandubois jandubois left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, LGTM

@jandubois jandubois merged commit 31bd624 into rancher-sandbox:main Apr 17, 2025
@mook-as mook-as deleted the nerdctl-stub/arg-order branch April 22, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nerdctl build on Windows cannot use alternative Dockerfile with absolute path

3 participants