debug: Add `buildx debug` command #2006

ktock · 2023-08-17T14:07:24Z

This commit refactors the debugger flags by adding buildx debug command that can be used for launching the monitor.
This implements the following features proposed by @jedevc and @crazy-max (#1104 (comment))

buildx debug-shell would just become buildx debug shell or even just buildx debug.

buildx build would remain unchanged from the current user experience. We wouldn't have any debugging flags here, it's all about build (though we still should use the controller api).
buildx debug build would allow all the same options that are currently in build to be specified after the build component. However, we could add any generic debugging flags before the build component, such as --on, e.g.:
$ buildx debug --on=error build . --target <target-in-dockerfile>

Examples

This launches a monitor after the build. /bin/sh will be invoked in the result image.

$ BUILDX_EXPERIMENTAL=1 /tmp/out/buildx debug --invoke /bin/sh build .

This launches a monitor after the build only when the build fails. /bin/sh will be invoked in the result image.

$ BUILDX_EXPERIMENTAL=1 /tmp/out/buildx debug --on=error --invoke=/bin/sh build .

The following immediately launches the monitor.

$ BUILDX_EXPERIMENTAL=1 /tmp/out/buildx debug

Notes

As the debugger-related flags are available only under buildx debug and this commit introduces buildx debug for launching the monitor directly, this commit removed buildx build --invoke=debug-shell flag. Please let me know if we shouldn't remove this.
~~I've implemented buildx debug shell for launching monitor but not buildx debug. Please let me know if we should have both of them.~~ Implemented buildx debug based on the comment debug: Add buildx debug command #2006 (comment) .
As this PR focuses on basic features of buildx deubg, the following proposal isn't included in this PR.
- That way, we split out the debugging flags from the build flags, and also allow integrating that more neatly with bake in the future:
```
$ buildx debug --on=error bake <target>
```
- buildx debug would allow "extra" args after a -- component, which would be run directly in the monitor, before stdin is connected. For example, to break on line 10, and drop into a shell:
```
$ buildx debug build . --target <target-in-dockerfile> -- break 10\; exec sh
```

ktock · 2023-08-18T07:31:22Z

It seems that we need to fix https://github.com/docker/docs as well to pass the CI?

docs-upstream / validate / run (pull_request) : https://github.com/docker/buildx/actions/runs/5898858287/job/16000556956?pr=2006#step:9:4159

 #23 238.2   Liquid Exception: undefined method `gsub' for nil:NilClass in engine/reference/commandline/buildx_debug-shell.md
#23 238.2                     ------------------------------------------------
#23 238.2       Jekyll 4.2.2   Please append `--trace` to the `build` command 
#23 238.2                      for any additional information or backtrace. 
#23 238.2                     ------------------------------------------------

crazy-max · 2023-08-18T07:46:10Z

It seems that we need to fix https://github.com/docker/docs as well to pass the CI?

docs-upstream / validate / run (pull_request) : https://github.com/docker/buildx/actions/runs/5898858287/job/16000556956?pr=2006#step:9:4159
 #23 238.2   Liquid Exception: undefined method `gsub' for nil:NilClass in engine/reference/commandline/buildx_debug-shell.md
#23 238.2                     ------------------------------------------------
#23 238.2       Jekyll 4.2.2   Please append `--trace` to the `build` command 
#23 238.2                      for any additional information or backtrace. 
#23 238.2                     ------------------------------------------------

Ah right that's because we have a stub file upstream for debug-shell that is now removed: https://github.com/docker/docs/blob/main/engine/reference/commandline/buildx_debug-shell.md. We didn't expect commands to be removed when this was implemented tbh 😅

crazy-max · 2023-08-18T08:00:15Z

commands/root.go

@@ -87,8 +88,10 @@ func addCommands(cmd *cobra.Command, dockerCli command.Cli) {
 		imagetoolscmd.RootCmd(dockerCli, imagetoolscmd.RootOptions{Builder: &opts.builder}),
 	)
 	if isExperimental() {
+		cmd.AddCommand(debugcmd.RootCmd(dockerCli, func(dops debugcmd.DebugOptions) *cobra.Command {
+			return buildCmd(dockerCli, opts, dops)


We are just debugging the build cmd so sounds good to have buildx debug defaulting to build atm but could be any root cmd in the future such as bake with buildx debug bake so we might consider passing cmd instead and have a "debug" interface for each command supporting debug. WDYT?

Thanks for the suggestion! It sounds good to me. I've fixed the patch to pass *cobra.Command with the debugger interface to debugcmd.RootCmd. PTAL 🙏

jedevc · 2023-08-18T11:15:25Z

commands/debug/shell.go

 	var options control.ControlOptions
 	var progressMode string

 	cmd := &cobra.Command{
-		Use:   "debug-shell",
+		Use:   "shell",


IMO, we could just have shell be the top-level command, so docker buildx debug drops you directly into the shell.

Then we don't need a separate command for it, and it kind of acts as debug is your portal to all debugging stuff. The top-level debug command needs to do something anyways, and to me this makes the most sense.

Also, if we drop shell, it means that we could potentially have every command after debug can be run without the debug - debug build corresponds to build, debug bake corresponds to bake, etc.

Thanks for the suggestion! Fixed the patch to launch the debugger directly by buildx debug command.

jedevc · 2023-08-18T11:17:33Z

Aha, thanks @ktock 🎉 This looks like a perfect start, just one real comment, otherwise looks good!

jedevc · 2023-08-18T11:17:53Z

commands/debug/root.go

+
+// DebugConfig is a user-specified configuration for the debugger.
+type DebugConfig struct {
+


nit: empty line (also below)

Off-topic: is there any way we can get this linted / auto-formatted? I know that gofumpt has a rule for this, but there's some other rules in there that are a bit more controversial IMO. @crazy-max any ideas?

I looked at gofumpt and wsl linters and don't see any rules for this case 🤔

Ah they have rules like this for functions at least in "No empty lines around function bodies" but not for generic blocks

crazy-max · 2023-08-21T05:28:02Z

@ktock Following @dvdksn comment in docker/docs#17947 (comment), I have disabled the docs-upstream workflow. Can you force push to check if it works?

ktock · 2023-08-21T06:01:54Z

Thanks!

crazy-max · 2023-08-21T14:07:36Z

commands/build.go

@@ -515,8 +515,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
 	flags.StringVar(&options.provenance, "provenance", "", `Shorthand for "--attest=type=provenance"`)

 	if isExperimental() {
-		flags.StringVar(&invokeFlag, "invoke", "", "Invoke a command after the build")
-		flags.SetAnnotation("invoke", "experimentalCLI", nil)
+		// TODO: move this to debug command if needed


root, detach and server-config flags don't look necessary anymore for build command right? Looks to be already available in debug root cmd.

Maybe? Technically a non debugged build can still run on the remote controller, so maybe best to leave it here for now.

jedevc

Thanks @ktock!

This gets closer to what I was imagining 🎉

I wonder, (and maybe this is something to look into with the "extra" args after a -- component) if we should always drop into a shell when using a debug subcommand? To me, that would feel like a logical default.

PTAL @tonistiigi

tonistiigi

When I run buildx debug (no arguments) and then exec sh I get

Process "i7xkjhw104urtoth9pboe0ug6" started. Press Ctrl-a-c to switch to that process.
(buildx) Switched IO
Switched IO

But there is no process, nor build context.

I'm not sure if this is specifically in this PR but enabling debugger breaks stacktraces. Without the opt-in I get.

Dockerfile:60
--------------------
  59 |     ARG TARGETPLATFORM
  60 | >>> RUN2 --mount=type=bind,target=. \
  61 | >>>   --mount=type=cache,target=/root/.cache \
  62 | >>>   --mount=type=cache,target=/go/pkg/mod \
  63 | >>>   --mount=type=bind,from=buildx-version,source=/buildx-version,target=/buildx-version <<EOT
  64 |       set -e
--------------------
ERROR: failed to solve: dockerfile parse error on line 60: unknown instruction: RUN2 (did you mean RUN?)

but with this only

ERROR: dockerfile parse error on line 60: unknown instruction: RUN2 (did you mean RUN?)

This is without any extra flags, just with BUILDX_EXPERIMANTAL=1 docker buildx debug build .

docker buildx debug --on=error build . with Dockerfile error I don't see any error printed at all, nor stacktrace. Only after I close the monitor I see the error printed.
I think --on should default to error on debug build. Atm. I don't see any differences between buildx build and buildx debug build.
Issuing rollback command always seems to return context canceled error.

Interactive container was restarted with process "4qa81kfpppzkoawwhh052bbn4". Press Ctrl-a-c to switch to the new container
(buildx) Switched IO
ERROR: failed to exec process: context canceled
ERROR: failed to exec process: context canceled

Follow-up. --on=error on a container error gets me in the container but there is no context of what happened, what was the last command etc. I think the help command or monitor messages should give context about what is the current interactive context (build result for specific target, error result from command) so there is context of what gets run on exec/reload/rollback.
Follow-up. We need ls command that would list the files in the current dir. If I get error like runc run failed: unable to start container process: exec: "sh": executable file not found in $PATH then I have no idea what I'm missing. This could also be exec of debug image what has the current mounts mounted somewhere.

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

ktock · 2023-10-13T03:19:10Z

@tonistiigi Thanks for the comments!

there is no process, nor build context.

buildx debug launches a monitor but it's not attached to any session in buildx so no process is available. (attach command allows attaching to other sessions from the monitor)
Fixed to return explicit error in these cases (ded91da).

In the future, we should create a new session for these cases with user-configured "default" image.

enabling debugger breaks stacktraces

Fixed (8da8ee2)

I don't see any error printed at all, nor stacktrace

Fixed(0dd89f6)

I think --on should default to error on debug build. Atm.

Fixed (5a0e4c1).

Issuing rollback command always seems to return context canceled error.

This is because rollback cancels the previously running process and creates the new one.
This is actually not an error to be reported to the user so I've changed this to be a debug level log (6db8569).

The following-ups will be separated PRs.

Follow-up. --on=error on a container error gets me in the container but there is no context of what happened, what was the last command etc. I think the help command or monitor messages should give context about what is the current interactive context (build result for specific target, error result from command) so there is context of what gets run on exec/reload/rollback.
Follow-up. We need ls command that would list the files in the current dir. If I get error like runc run failed: unable to start container process: exec: "sh": executable file not found in $PATH then I have no idea what I'm missing. This could also be exec of debug image what has the current mounts mounted somewhere.

tonistiigi

Thanks. I think we can merge this now in the current state, but some more follow-ups:

Let's say I have two different types of errors. One is wrong Dockerfile command and another is process error.

Dockerfile:60
--------------------
  59 |     ARG TARGETPLATFORM
  60 | >>> RUN2 --mount=type=bind,target=. \
  61 | >>>   --mount=type=cache,target=/root/.cache \
  62 | >>>   --mount=type=cache,target=/go/pkg/mod \
  63 | >>>   --mount=type=bind,from=buildx-version,source=/buildx-version,target=/buildx-version <<EOT
  64 |       set -e
--------------------
ERROR: dockerfile parse error on line 60: unknown instruction: RUN2 (did you mean RUN?)
[+] Building 0.0s (0/0)                                                                                                                                                                                                         docker:desktop-linux
Launching interactive container. Press Ctrl-a-c to switch to monitor console
Interactive container was restarted with process "o5e8x1ty9nn2j93m62b8zmhdn". Press Ctrl-a-c to switch to the new container
Switched IO

------
Dockerfile:60
--------------------
  59 |     ARG TARGETPLATFORM
  60 | >>> RUN --mount=type=bind,target=. \
  61 | >>>   --mount=type=cache,target=/root/.cache \
  62 | >>>   --mount=type=cache,target=/go/pkg/mod \
  63 | >>>   --mount=type=bind,from=buildx-version,source=/buildx-version,target=/buildx-version <<EOT
  64 | >>>   set -e
  65 | >>>   xx-go2 --wrap
  66 | >>>   DESTDIR=/usr/bin VERSION=$(cat /buildx-version/version) REVISION=$(cat /buildx-version/revision) GO_EXTRA_LDFLAGS="-s -w" ./hack/build
  67 | >>>   xx-verify --static /usr/bin/docker-buildx
  68 | >>> EOT
  69 |
--------------------
ERROR: process "/bin/sh -c   set -e\n  xx-go2 --wrap\n  DESTDIR=/usr/bin VERSION=$(cat /buildx-version/version) REVISION=$(cat /buildx-version/revision) GO_EXTRA_LDFLAGS=\"-s -w\" ./hack/build\n  xx-verify --static /usr/bin/docker-buildx\n" did not complete successfully: exit code: 127
[+] Building 0.0s (0/0)                                                                                                                                                                                                         docker:desktop-linux
Launching interactive container. Press Ctrl-a-c to switch to monitor console
Interactive container was restarted with process "l3x31fm3i08owokoxwtazeuuw". Press Ctrl-a-c to switch to the new container
/ #

As expected only the second one is debuggable (only second one opens shell as well). But from the output they print same messages about interactive containers and switching IO. It should be more clear that these are different types of errors, why first one does not create execution context and what runs in the shell of second one.

There is an error in Dockerfile command (eg. change RUN to RUN2). I get an error. Now I run reload. It builds up to the Dockerfile error again but this time I don't see any error. It just shows:

(buildx) reload
[+] Building 0.6s (4/4) FINISHED                                                                                                                                                                                                docker:desktop-linux
 => [internal] load build definition from Dockerfile                                                                                                                                                                                            0.0s
 => => transferring dockerfile: 4.74kB                                                                                                                                                                                                          0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                               0.0s
 => => transferring context: 45B                                                                                                                                                                                                                0.0s
 => resolve image config for docker.io/docker/dockerfile:1                                                                                                                                                                                      0.5s
 => CACHED docker-image://docker.io/docker/dockerfile:1@sha256:ac85f380a63b13dfcefa89046420e1781752bab202122f8f50032edf31be0021                                                                                                                 0.0s
Interactive container was restarted with process "p1zsu784bbr3orf8t1g5by9j7". Press Ctrl-a-c to switch to the new container
Switched IO
Switched IO
(buildx)

I run debug build . and get an error. I fix the error and issue reload. Now my build succeeds. I run some more interactive processes and eventually close the debugger. Now I get an error.

(buildx)
[+] Building 0.0s (0/0)                                                                                                                                                                                                         docker:desktop-linux
Dockerfile:60
--------------------
  59 |     ARG TARGETPLATFORM
  60 | >>> RUN2 --mount=type=bind,target=. \
  61 | >>>   --mount=type=cache,target=/root/.cache \
  62 | >>>   --mount=type=cache,target=/go/pkg/mod \
  63 | >>>   --mount=type=bind,from=buildx-version,source=/buildx-version,target=/buildx-version <<EOT
  64 |       set -e
--------------------
ERROR: dockerfile parse error on line 60: unknown instruction: RUN2 (did you mean RUN?)

What I think is the error from the very first build invocation.

(We can discuss this more in the issue before any actual work). When I reach an error I don't think there is a way to switch to the clean state before the failed command to try running it again. My options are to reload to rebuild from source or rollback that resets my local changes but I still seem to have the state that the command that failed generated before it errored.

ktock · 2023-10-18T03:44:11Z

Thanks for the feedback.

The following will be fixed by #2086

There is an error in Dockerfile command (eg. change RUN to RUN2). I get an error. Now I run reload. It builds up to the Dockerfile error again but this time I don't see any error.

I run debug build . and get an error. I fix the error and issue reload. Now my build succeeds. I run some more interactive processes and eventually close the debugger. Now I get an error.

I think the following is already done by rollback --init

(We can discuss this more in the issue before any actual work). When I reach an error I don't think there is a way to switch to the clean state before the failed command to try running it again. My options are to reload to rebuild from source or rollback that resets my local changes but I still seem to have the state that the command that failed generated before it errored.

I've recorded Other following-up issues to #1104 (comment)

ktock force-pushed the debug-command branch 3 times, most recently from 80bb8e0 to 274c1ce Compare August 18, 2023 04:38

crazy-max reviewed Aug 18, 2023

View reviewed changes

ktock force-pushed the debug-command branch 2 times, most recently from 934e927 to 55691f3 Compare August 18, 2023 08:54

jedevc self-requested a review August 18, 2023 10:28

jedevc reviewed Aug 18, 2023

View reviewed changes

ktock force-pushed the debug-command branch from 55691f3 to 453fd37 Compare August 18, 2023 12:14

crazy-max mentioned this pull request Aug 18, 2023

ci(validate-upstream): allow cleanup of stub files docker/docs#17947

Closed

ktock force-pushed the debug-command branch from 453fd37 to 162b682 Compare August 21, 2023 05:52

crazy-max reviewed Aug 21, 2023

View reviewed changes

crazy-max approved these changes Aug 21, 2023

View reviewed changes

jedevc approved these changes Aug 24, 2023

View reviewed changes

crazy-max requested a review from tonistiigi August 24, 2023 10:40

ktock force-pushed the debug-command branch 2 times, most recently from e18f57d to da87cb4 Compare August 31, 2023 07:41

tonistiigi reviewed Oct 12, 2023

View reviewed changes

ktock added 6 commits October 13, 2023 12:09

debug: Add buildx debug command

508b2ef

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

exec, rollback: return error when no session found

ded91da

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

debug: set on=error by default

5a0e4c1

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

process: Do not print error log when process is canceled

6db8569

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

controller: return original error to preserve stacktrace

8da8ee2

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

monitor: print error information before launching monitor

0dd89f6

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

debug: update docs

198764f

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>

ktock force-pushed the debug-command branch from da87cb4 to 198764f Compare October 13, 2023 03:14

tonistiigi approved these changes Oct 17, 2023

View reviewed changes

crazy-max added the area/debug label Oct 17, 2023

tonistiigi merged commit ac93a7f into docker:master Oct 18, 2023
59 checks passed

ktock deleted the debug-command branch October 18, 2023 01:39

This was referenced Oct 18, 2023

debug: Following-up patches for error messages #2086

Merged

Proposal: build debugging in buildx (interactive sessions) #1104

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

debug: Add `buildx debug` command #2006

debug: Add `buildx debug` command #2006

ktock commented Aug 17, 2023 •

edited

ktock commented Aug 18, 2023 •

edited

crazy-max commented Aug 18, 2023

crazy-max Aug 18, 2023

ktock Aug 18, 2023

jedevc Aug 18, 2023

ktock Aug 18, 2023

jedevc commented Aug 18, 2023

jedevc Aug 18, 2023

jedevc Aug 18, 2023

crazy-max Aug 21, 2023

jedevc Aug 21, 2023 •

edited

crazy-max commented Aug 21, 2023

ktock commented Aug 21, 2023

crazy-max Aug 21, 2023

jedevc Aug 21, 2023

jedevc left a comment •

edited

tonistiigi left a comment

ktock commented Oct 13, 2023 •

edited

tonistiigi left a comment

ktock commented Oct 18, 2023


		// DebugConfig is a user-specified configuration for the debugger.
		type DebugConfig struct {

debug: Add buildx debug command #2006

debug: Add buildx debug command #2006

Conversation

ktock commented Aug 17, 2023 • edited

Examples

Notes

ktock commented Aug 18, 2023 • edited

crazy-max commented Aug 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jedevc commented Aug 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jedevc Aug 21, 2023 • edited

Choose a reason for hiding this comment

crazy-max commented Aug 21, 2023

ktock commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jedevc left a comment • edited

Choose a reason for hiding this comment

tonistiigi left a comment

Choose a reason for hiding this comment

ktock commented Oct 13, 2023 • edited

tonistiigi left a comment

Choose a reason for hiding this comment

ktock commented Oct 18, 2023

debug: Add `buildx debug` command #2006

debug: Add `buildx debug` command #2006

ktock commented Aug 17, 2023 •

edited

ktock commented Aug 18, 2023 •

edited

jedevc Aug 21, 2023 •

edited

jedevc left a comment •

edited

ktock commented Oct 13, 2023 •

edited