cmd/snap: add `debug api` command #14116

bboozzoo · 2024-06-24T10:36:12Z

Add a new snap debug api command which can be used to execute raw queries against snapd API. This is supposed to be a replacement for curl which is not part of the base snaps and thus, especially on core, needs to be installed separately using --devmode.

Trigger snap refresh:

$ echo '{"action": "refresh"}' | sudo SNAPD_DEBUG=1 snap debug api --pretty \
    -X POST -H  'Content-Type=application/json' '/v2/snaps/toolbox'
2024/06/24 12:32:39.856855 logger.go:93: DEBUG: re-exec not supported on distro "arch" yet
2024/06/24 12:32:39.857264 logger.go:93: DEBUG: url: /v2/snaps/toolbox
2024/06/24 12:32:39.857275 logger.go:93: DEBUG: query: 
2024/06/24 12:32:39.857280 logger.go:93: DEBUG: headers: map[Content-Type:application/json]
{
  "change": "2992",
  "result": null,
  "status": "Accepted",
  "status-code": 202,
  "type": "async"
}

Grab some static output:

$  snap debug api --pretty /v2/changes/2990 | tail -10                                                                          
        "spawn-time": "2024-06-24T12:28:38.606671208+02:00",
        "status": "Done",
        "summary": "Monitoring snap \"toolbox\" to determine whether extra refresh steps are required"
      }
    ]
  },
  "status": "OK",
  "status-code": 200,
  "type": "sync"
}

Or stacktraces from pprof:

$ sudo snap debug api '/v2/debug/pprof/goroutine' > stacktraces
...
$ go tool pprof -traces ./stacktraces | head -10
File: snapd
Type: goroutine
Time: Jun 24, 2024 at 12:41pm (CEST)
-----------+-------------------------------------------------------
         1   runtime.notetsleepg
             os/signal.signal_recv
             os/signal.loop
-----------+-------------------------------------------------------
         1   runtime.goroutineProfileWithLabels
             runtime/pprof.runtime_goroutineProfileWithLabels

Related: SNAPDENG-23792

zyga

LGTM with a tweak

zyga · 2024-06-24T10:47:02Z

cmd/snap/cmd_debug_api.go

+	hdrs := x.Positional.Headers
+	reqHdrs := make(map[string]string, len(hdrs))
+	for _, arg := range x.Positional.Headers {
+		split := strings.SplitN(arg, "=", 2)


Have a look at strings.Cut for a nicer alternative.

codecov-commenter · 2024-06-24T10:52:30Z

Codecov Report

Attention: Patch coverage is 12.72727% with 48 lines in your changes missing coverage. Please review.

Project coverage is 78.70%. Comparing base (c01787f) to head (3fe3e04).
Report is 9 commits behind head on master.

Files	Patch %	Lines
cmd/snap/cmd_debug_api.go	13.20%	46 Missing ⚠️
client/client.go	0.00%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14116      +/-   ##
==========================================
- Coverage   78.72%   78.70%   -0.03%     
==========================================
  Files        1053     1054       +1     
  Lines      137711   137892     +181     
==========================================
+ Hits       108419   108524     +105     
- Misses      22494    22562      +68     
- Partials     6798     6806       +8

Flag	Coverage Δ
unittests	`78.70% <12.72%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Meulengracht

lgtm, a couple of small comments

Meulengracht · 2024-06-24T10:57:22Z

cmd/snap/cmd_debug_api.go

+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2021 Canonical Ltd


Suggested change

* Copyright (C) 2021 Canonical Ltd

* Copyright (C) 2024 Canonical Ltd

Meulengracht · 2024-06-24T10:57:42Z

cmd/snap/cmd_debug_api.go

@@ -0,0 +1,108 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-


I think somewhere here it warrants a simple example just to show how it's used

If you add an example (like you have in the PR description), you could add it to the command's long description so it's easily accessible

I've added some examples in long description of the command.

MiguelPires

lgtm, thank you

MiguelPires · 2024-06-24T11:49:18Z

client/client.go

@@ -801,3 +801,9 @@ func (c *Client) MigrateSnapHome(snaps []string) (changeID string, err error) {

 	return c.doAsync("POST", "/v2/debug", nil, nil, bytes.NewReader(body))
 }
+
+// DebugRaw allows to make raw queries to the API, but the intention of using it


Suggested change

// DebugRaw allows to make raw queries to the API, but the intention of using it

// DebugRaw allows to make raw queries to the API with the intention of using it

MiguelPires · 2024-06-24T11:56:46Z

cmd/snap/cmd_debug_api.go

+	if rsp != nil {
+		defer rsp.Body.Close()
+	}
+	if err != nil {
+		return err
+	}


The error and the response are mutually exclusive, so this can be simplified slightly

Suggested change

if rsp != nil {

defer rsp.Body.Close()

}

if err != nil {

return err

}

if err != nil {

return err

}

defer rsp.Body.Close()

MiguelPires · 2024-06-24T11:58:00Z

cmd/snap/cmd_debug_api.go

@@ -0,0 +1,108 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-


If you add an example (like you have in the PR description), you could add it to the command's long description so it's easily accessible

pedronis

couple comments

pedronis · 2024-06-25T11:49:03Z

cmd/snap/cmd_debug_api.go

+
+	Positional struct {
+		Method  string   `positional-arg-name:"<method>"`
+		Query   string   `positional-arg-name:"<query>"`


this is a bit of a strange as it contains also the path?

Renamed the argument. I didn't want to split it into path and query params, such that snap debug api GET /v2/snaps works and you need to pass additional query args it's simply snap debug api GET '/v2/find?name=foo'

I've tweaked the arguments now so that we're closer to what curl expects, i.e.:

Usage: test-snap debug api [api-OPTIONS] <path-and-query> ... [api command options] --pretty Pretty print if response is JSON -H, --header= Set header (can be repeated multiple times) -X, --request= HTTP method to use (defaults to GET) --fail Fail on request errors

pedronis · 2024-06-25T11:51:55Z

cmd/snap/cmd_debug_api.go

+		return err
+	}
+
+	if x.Pretty {


are we consistent enough with the content type of returns in the API? that we can simply do this any time (or at least when the output is a terminal) it's JSON without needing to have the option?

pedronis · 2024-06-25T11:52:52Z

cmd/snap/cmd_debug_api.go

+	Positional struct {
+		Method  string   `positional-arg-name:"<method>"`
+		Query   string   `positional-arg-name:"<query>"`
+		Headers []string `positional-arg-name:"<headers>"`


the order query/path then headers is a bit confusing tbh

Signed-off-by: Maciej Borzecki <[email protected]>

pedronis · 2024-07-02T08:45:22Z

cmd/snap/cmd_debug_api.go

+	}
+	defer rsp.Body.Close()
+
+	if x.Pretty && rsp.Header.Get("Content-Type") == "application/json" {


my idea was not to have --pretty/Pretty at all and just check for the type?

aah I see, ok let me update the code then

Add a debug command to execute raw queries to the snapd API. This is useful when trying to query snapd endpoints directly, but curl is other similar tool is not available. Signed-off-by: Maciej Borzecki <[email protected]>

MiguelPires

lgtm

pedronis

thank you

bboozzoo added ⛔ Blocked Needs Samuele review Needs a review from Samuele before it can land Skip spread Indicate that spread job should not run labels Jun 24, 2024

bboozzoo requested review from pedronis, MiguelPires and Meulengracht June 24, 2024 10:43

bboozzoo changed the title ~~cmd/snap: add debug api command~~ RFC cmd/snap: add debug api command Jun 24, 2024

zyga approved these changes Jun 24, 2024

View reviewed changes

Meulengracht approved these changes Jun 24, 2024

View reviewed changes

MiguelPires approved these changes Jun 24, 2024

View reviewed changes

pedronis reviewed Jun 25, 2024

View reviewed changes

bboozzoo changed the title ~~RFC cmd/snap: add debug api command~~ cmd/snap: add debug api command Jul 2, 2024

client: expose a helper for executing raw queries to the snapd API

23d91e3

Signed-off-by: Maciej Borzecki <[email protected]>

bboozzoo force-pushed the bboozzoo/snap-debug-api branch from 3fe3e04 to e96bb1d Compare July 2, 2024 08:27

bboozzoo marked this pull request as ready for review July 2, 2024 08:27

bboozzoo requested review from pedronis, MiguelPires, Meulengracht and zyga July 2, 2024 08:29

bboozzoo removed the ⛔ Blocked label Jul 2, 2024

pedronis reviewed Jul 2, 2024

View reviewed changes

bboozzoo force-pushed the bboozzoo/snap-debug-api branch 2 times, most recently from a26f967 to d92e8d6 Compare July 2, 2024 08:53

cmd/snap: add debug api command

a033a40

Add a debug command to execute raw queries to the snapd API. This is useful when trying to query snapd endpoints directly, but curl is other similar tool is not available. Signed-off-by: Maciej Borzecki <[email protected]>

bboozzoo force-pushed the bboozzoo/snap-debug-api branch from d92e8d6 to a033a40 Compare July 2, 2024 09:16

bboozzoo requested a review from pedronis July 2, 2024 09:16

bboozzoo mentioned this pull request Jul 2, 2024

tests: drop uses of test-snapd-curl where possible, replace with snap debug api #14152

Open

MiguelPires approved these changes Jul 3, 2024

View reviewed changes

Meulengracht approved these changes Jul 3, 2024

View reviewed changes

pedronis approved these changes Jul 3, 2024

View reviewed changes

bboozzoo merged commit 5b632be into canonical:master Jul 4, 2024
51 checks passed

bboozzoo deleted the bboozzoo/snap-debug-api branch July 4, 2024 05:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/snap: add `debug api` command #14116

cmd/snap: add `debug api` command #14116

bboozzoo commented Jun 24, 2024 •

edited

Loading

zyga left a comment

zyga Jun 24, 2024

codecov-commenter commented Jun 24, 2024

Meulengracht left a comment

Meulengracht Jun 24, 2024

Meulengracht Jun 24, 2024

MiguelPires Jun 24, 2024

bboozzoo Jul 2, 2024

MiguelPires left a comment

MiguelPires Jun 24, 2024

MiguelPires Jun 24, 2024

MiguelPires Jun 24, 2024

pedronis left a comment

pedronis Jun 25, 2024

bboozzoo Jul 2, 2024

bboozzoo Jul 2, 2024

pedronis Jun 25, 2024

pedronis Jun 25, 2024

pedronis Jul 2, 2024

bboozzoo Jul 2, 2024

MiguelPires left a comment

pedronis left a comment

	* Copyright (C) 2021 Canonical Ltd
	* Copyright (C) 2024 Canonical Ltd

	// DebugRaw allows to make raw queries to the API, but the intention of using it
	// DebugRaw allows to make raw queries to the API with the intention of using it

cmd/snap: add debug api command #14116

cmd/snap: add debug api command #14116

Conversation

bboozzoo commented Jun 24, 2024 • edited Loading

zyga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 24, 2024

Codecov Report

Meulengracht left a comment

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

Choose a reason for hiding this comment

MiguelPires left a comment

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

pedronis left a comment

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

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

MiguelPires left a comment

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

cmd/snap: add `debug api` command #14116

cmd/snap: add `debug api` command #14116

bboozzoo commented Jun 24, 2024 •

edited

Loading