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

WIP: Debug: Enable VM debug console #658

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

WIP: Debug: Enable VM debug console #658

wants to merge 1 commit into from

Conversation

mcastelino
Copy link
Contributor

Enable the VM debug console whenever the VM Kernel debug
is enabled.

Fixes: #559.

Has a dependency on having a updated agent being installed in the container rootfs to add the new systemd services.

clearcontainers/agent#122

Signed-off-by: Manohar Castelino manohar.r.castelino@intel.com

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@grahamwhaley grahamwhaley mentioned this pull request Oct 2, 2017
Open
grahamwhaley
grahamwhaley approved these changes Oct 2, 2017
View reviewed changes
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.

lgtm

@grahamwhaley
Copy link
Contributor

CI is failing though. I suspect we probably have a vendor or merge dep? In f26 we have:

../../../go/src/github.com/clearcontainers/runtime/list.go:84: undefined: defaultRootDirectory
../../../go/src/github.com/clearcontainers/runtime/main.go:91: undefined: defaultRootDirectory
../../../go/src/github.com/clearcontainers/runtime/config.go:146: too many errors

@jodh-intel
Copy link
Contributor

Hmm, actually the CI run was successful for all 3 runs. Looks like there might be an issue with the jenkins-github plugin @sboeuf ?

@jodh-intel
Copy link
Contributor

@grahamwhaley - the undefined errors are caused by the jenkins scripts incorrectly go get'ing the runtime code (it should be go get -d). Hopefully we can resolve that once clearcontainers/jenkins#10 lands.

@clearcontainersbot
Copy link

kubernetes qa-passed 👍

1 similar comment
@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@mcastelino mcastelino force-pushed the topic/debug_console branch from 44af25c to f28f5a2 Compare October 6, 2017 17:25
@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@jodh-intel
Copy link
Contributor

Hi @mcastelino - this requires an update to the tests. From the CI logs:

./create_test.go:848: cannot use func literal (type func(string) []virtcontainers.Param) as type func(string, oci.RuntimeConfig) []virtcontainers.Param in assignment

@mcastelino mcastelino force-pushed the topic/debug_console branch from f28f5a2 to 53c685c Compare October 9, 2017 18:37
@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@mcastelino
Debug: Enable VM debug console
b59bfde 
Enable the VM debug console whenever the VM Kernel debug
is enabled.

Fixes: clearcontainers#559.

Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
@mcastelino mcastelino force-pushed the topic/debug_console branch from 53c685c to b59bfde Compare October 9, 2017 19:48
@clearcontainersbot
Copy link

kubernetes qa-passed 👍

@jodh-intel
Copy link
Contributor

jodh-intel commented Oct 10, 2017 •
edited by grahamwhaley
Loading

lgtm

Approved with PullApprove Approved with PullApprove

jodh-intel
jodh-intel reviewed Oct 11, 2017
View reviewed changes
docs/debug-kernel.md
for further debug.

```
socat /run/virtcontainers/pods/<pod id>/console.sock
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a $ prompt for consistency with the rest of the doc please. Also, presumably we need a sudo in there too?

@iphutch - please can you review these doc changes?

Copy link

Choose a reason for hiding this comment

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

+1

@jodh-intel
Copy link
Contributor

Hi @mcastelino - just one final nit in the docs from me.

@iphutch - please could you review the doc changes?

iphutch
iphutch suggested changes Oct 16, 2017
View reviewed changes
docs/debug-kernel.md
@@ -44,3 +44,12 @@ to enable kernel boot logs using the configuration file.

This will result in the `cc-proxy` log being printed on the terminal. This method
can also be used to capture the logs in a specific `cc-proxy` file.

5. Connect to the Virtual machine console
Copy link

Choose a reason for hiding this comment

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

Use all lower case:
s/Virtual/virtual

docs/debug-kernel.md
for further debug.

```
socat /run/virtcontainers/pods/<pod id>/console.sock
Copy link

Choose a reason for hiding this comment

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

+1

docs/debug-kernel.md

5. Connect to the Virtual machine console

If the kernel boots successfully then the virtual machine console can be accessed
Copy link

Choose a reason for hiding this comment

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

We should avoid using an "if" clause unless we provide options for when the kernel does not boot successfully.
Let us ensure that our instructions are accurate and assume the reader has followed the instructions exactly:
After the kernel boots, the virtual machine console can be accessed for further debugging.

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel
Copy link
Contributor

Hi @mcastelino - just a reminder that this PR needs a few minor tweaks.

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

4 similar comments
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel
Copy link
Contributor

Hi @klynnrif, @rcaballeromx - I'm assuming @iphutch won't be able to complete this review so could one of you take a look and if you're happy, we'll force-merge this as we will have the requisite doc team approval.

@klynnrif
Copy link

@jodh-intel @rcaballeromx the suggestions @iphutch made lgtm. All good to me with those changes. Thanks!

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

3 similar comments
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel jodh-intel mentioned this pull request Jan 12, 2018
Closed
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

6 similar comments
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel jodh-intel mentioned this pull request Mar 14, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jodh-intel jodh-intel jodh-intel left review comments

@iphutch iphutch iphutch requested changes

@grahamwhaley grahamwhaley grahamwhaley approved these changes

Assignees
No one assigned
Labels
in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mcastelino @clearcontainersbot @grahamwhaley @jodh-intel @klynnrif @iphutch @amyleeland