devLXD: Response format based on vsock usage #15727
Conversation
|
|
||
| "github.com/canonical/lxd/client" | ||
| "github.com/canonical/lxd/lxd/metrics" | ||
| "github.com/canonical/lxd/lxd/request" |
There was a problem hiding this comment.
I would like to draw some attention here.
Since the information whether the vsock is being used is required during response rendering, the response package is now depending on request package (to reference a context key - request.DevLXDOverVsock).
If this is not desired, we can use plain string to reference the key. Or maybe there is even better approach that I am not seeing.
There was a problem hiding this comment.
I'm not certain about the side effects of this but one option would be to define the context key elsewhere. The context key should be of a particular type and not just a plain string (you'll get a linter complaint otherwise, I think this is required for key uniqueness or something)
There was a problem hiding this comment.
Cool, so if there are any undesired side effects then we should move the key in some package that can be referenced from lxd and lxd/response package. We can use lxd/response package itself as this would remove the need for a new dependency and the key is mainly used to determine the response format.
There was a problem hiding this comment.
This might be an issue for MicroCluster, but I think @roosterfish is in the process of untangling that stuff anyway.
|
Please rebase |
|
Initial review - all looking very neat and tidy! Especially in the lxd-agent 🚀 |
MusicDin
force-pushed
the
feat/devlxd-cleanup
branch
from
57b1812 to
3f266b5
Compare
trnsprt, ok := r.http.Transport.(*http.Transport)
if !ok {
panic("error")
}
if trnsprt.TLSClientConfig != nil {
// When communicating with devLXD over vsock, the response is
// expected to be in devLXD format.
resp, etag, err := lxdParseResponse(resp)
if err != nil {
return nil, "", err
}
// Wrap into devLXD response.
return &api.DevLXDResponse{
Content: resp.Metadata,
StatusCode: resp.StatusCode,
}, etag, nil
}Suggested in 1:1 using the above instead of having an |
|
Just for context, @markylaing's suggestion is a way to get rid of setting/hardcoding |
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
MusicDin
force-pushed
the
feat/devlxd-cleanup
branch
from
3f266b5 to
303ef87
Compare
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
Signed-off-by: Din Music <[email protected]>
MusicDin
force-pushed
the
feat/devlxd-cleanup
branch
from
303ef87 to
e7c8405
Compare
| }, nil | ||
| } | ||
|
|
||
| // ConnectDevLXDHTTPWithContext lets you connect to devLXD over a VM socket. |
There was a problem hiding this comment.
VM socket? You mean Instance socket - is this for container or VMs?
There was a problem hiding this comment.
For containers we have just ConnectDevLXD[WithContext](), which connects to the provided unix path (e.g. /dev/lxd/sock).
This is for VMs, and it was meant to be ... over a vsock.
There was a problem hiding this comment.
Oh I see the important difference was "HTTP".
3 participants
This PR changes the devLXD response functions in a way that it determines the response format based on the vsock usage. If devLXD is being interacted over vsock (from LXD agent), the responses are returned as
api.Response. Otherwise, the response are formatted asapi.DevLXDResponse. This information is stored in the request context, and is retrieved from it during the response rendering.Additionally, the LXD agent now uses the devLXD client to interact with the devLXD. This means that the devLXD client is adjusted to expect
api.Responsewhen used from LXD agent.