Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

Debug: Provide a way to enable a debug console to the VM #122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcastelino
Copy link
Contributor

@mcastelino mcastelino commented Sep 29, 2017

Provide an additional unit file to enable a debug console.
Also provide an additional agent service file variant that
can be used to launch the VM in debug mode.

The console can be attached using

socat STATE_DIR_CONTAINER/console.sock -

Signed-off-by: Manohar Castelino [email protected]

Provide an additional unit file to enable a debug console.
Also provide an additional agent service file variant that
can be used to launch the VM in debug mode.

The console can be attached using

socat console.sock -

Fixes clearcontainers#123

Signed-off-by: Manohar Castelino <[email protected]>
@jodh-intel
Copy link
Contributor

Hi @mcastelino - this looke very useful! Could you add a section to the README explaining how to use it?

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

Excellent. Have had looking into something like this in the back of my mind for a little while - should be v.useful!
Agree with @jodh-intel - we need to add some info to the docs about how you invoke this from the host side, and that magic about socat.

@grahamwhaley
Copy link
Contributor

A side note - I have a memory of somebody asking if there were a way to get to the VM console for debug and optimisation - but, I can't quite remember who asked or where (Issue or email or ??). So,
/cc @wuzhy @dvoytik @wcwxyz I think should find the right person :-)

@grahamwhaley
Copy link
Contributor

Ah, seeing we also have clearcontainers/runtime#658, which has some docs, maybe just a small doc note in the agent pointing over to the relevant runtime docs is all that would be required.

StandardOutput=tty
PrivateDevices=yes
Type=simple
ExecStart=/usr/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be /bin/sh for portability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jodh-intel yes. I would prefer the most minimal shell that clear linux provides as a package.

@jcvenegas what are our options? I would rather not have a separate debug image just for this. Or if we end up with a separate image, we should always package and distribute it with the non debug image, so that the user has the confidence that they are using the exact same version of all the image components and be able to flip to the debug image by just modifying the toml config file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mcastelino @jodh-intel today there is not busybox in CL, will need to request a new shell .

@jodh-intel
Copy link
Contributor

@mcastelino - should this be marked do-not-merge until clearcontainers/osbuilder#36 lands (since it won't work aiui)?

@jodh-intel
Copy link
Contributor

@mcastelino - what are the permissions on console.sock? I don't see a way with qemu to specify permissions so hopefully they are "tight" from a security perspective?

@jodh-intel
Copy link
Contributor

I agree with @grahamwhaley - we probably don't want to have to maintain 2 sets of docs for the same thing (although I wonder if we want the real info in the agent repo and have the runtime ref it's README rather than the other way round?)

@jcvenegas
Copy link
Contributor

@mcastelino in the docs could you add a warning to not run the proxy in debug mode, in this case both the proxy and socat will try to read the console.socket.

@mcastelino
Copy link
Contributor Author

@jcvenegas @grahamwhaley @jodh-intel I was thinking of just extending the kernel debug document and make it a general advanced debug document. That way we do not end up with multiple debug documents. What do you think.

@grahamwhaley
Copy link
Contributor

I'd like one 'advanced debugging' document for devs, and not have to spread the info around split down by either different components/repos or other sub-divisions - I'd like one place I can go read and later find all the 'deep dive hard info' for devs - so, +1 from me.
We should then point to that doc from the other repo top levels I think though - if I go looking for how to debug the agent or proxy in their repos then I'd like a ref to where I can then find that info :-)

@mcastelino
Copy link
Contributor Author

@mcastelino - should this be marked do-not-merge until clearcontainers/osbuilder#36 lands (since it won't work aiui)?

@jodh-intel as Wants is a less strict requirement. Even if the image does not have the packaging changes
nothing will go wrong. So it can be enabled without the packaging changes as long as we agree on the unit file name for the debug target.

@jodh-intel
Copy link
Contributor

jodh-intel commented Oct 3, 2017

clear-containers-debug seems pretty clear to me ;)

lgtm

Approved with PullApprove

@sboeuf
Copy link
Contributor

sboeuf commented Oct 3, 2017

LGTM

Approved with PullApprove Approved with PullApprove

@sboeuf
Copy link
Contributor

sboeuf commented Oct 27, 2017

@mcastelino we need you to add some docs before we can merge this.

@mcastelino
Copy link
Contributor Author

mcastelino commented Oct 27, 2017

@sboeuf this cannot be merged unless we merge clearcontainers/osbuilder#37 which is caught up in a lot of discussion.

I will write up some more documentation and also extend the built in toml doc
https://github.com/clearcontainers/runtime/pull/658/files#diff-89cfafb08ba579a24cd62a9e9e42cbfcR58

I strongly prefer an unified image right now purely from a user's point of view. A deployer can always deploy a stripped down image. .

Debugging an image that is different from the one that failed can be fraught with issues from my experience.

@sboeuf
Copy link
Contributor

sboeuf commented Oct 27, 2017

@mcastelino ok thanks for the heads up ! And BTW I agree we should not have two different images, this will bring a looooot of confusion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants