VNC subcommand in virtletctl

[jellonek]

This command needs to be rebased on top of master after merging #598 on which it depends.

Todo:

• docs,
• test output of command and created port forwarding.

---

This change is 

[ivan4th]

Review status: 0 of 5 files reviewed at latest revision, 12 unresolved discussions.

---

pkg/tools/vnc.go, line 40 at r1 (raw file):

)

// vncCommand gives an access to VNC console of VM pod.

// vncCommand provides access to the VNC console of a VM pod

---

pkg/tools/vnc.go, line 47 at r1 (raw file):

}

// NewVNCCmd returns a cobra.Command that gives an access to VNC console of VM pod.

... that provides access to the VNC console of a VM pod

---

pkg/tools/vnc.go, line 52 at r1 (raw file):

	cmd := &cobra.Command{
		Use:   "vnc pod [port]",
		Short: "Provide an access to a VM pod VNC console",

Provide access to the VNC console of a VM pod

---

pkg/tools/vnc.go, line 57 at r1 (raw file):

			provided port forwarding for VNC will try to use that one.
			Otherwise the kernel will chose a random one which will
			be displayed as a part of command output.

I'd put it this way:

This command forwards a local port to the VNC port used by the specified VM pod. If no local port number is provided, a random available port is picked instead. The port number is displayed after the forwarding is set up, after which the commands enters an endless loop until it's interrupted with Ctrl-C.

---

pkg/tools/vnc.go, line 107 at r1 (raw file):

	switch {
	case len(parts) != 3:
		return fmt.Errorf("virsh returned %q, while expected to return something like %q", buffer.String(), "vnc://127.0.0.1:0")

while expected to return a value of a form ...

---

pkg/tools/vnc.go, line 109 at r1 (raw file):

		return fmt.Errorf("virsh returned %q, while expected to return something like %q", buffer.String(), "vnc://127.0.0.1:0")
	case parts[0] != vncProtocolName:
		return fmt.Errorf("virsh returned %q as a display protocol while expected one was %q", parts[0], vncProtocolName)

instea of expected %q

---

pkg/tools/vnc.go, line 111 at r1 (raw file):

		return fmt.Errorf("virsh returned %q as a display protocol while expected one was %q", parts[0], vncProtocolName)
	case parts[1][:2] != "//":
		return fmt.Errorf("virsh returned %q after first ':' while expected was %q", parts[1][:2], "//")

instead of expected %q

---

pkg/tools/vnc.go, line 113 at r1 (raw file):

		return fmt.Errorf("virsh returned %q after first ':' while expected was %q", parts[1][:2], "//")
	case parts[1][2:] != expectedHost:
		return fmt.Errorf("virsh returned %q as a display host while expected one was %q", parts[1], expectedHost)

instead of expected %q

---

pkg/tools/vnc.go, line 116 at r1 (raw file):

	}

	displayPortOffset, err := strconv.Atoi(parts[2])

I think it's called "display number" not "display port offset" in VNC.

---

pkg/tools/vnc.go, line 117 at r1 (raw file):

	displayPortOffset, err := strconv.Atoi(parts[2])
	if err != nil || displayPortOffset < 0 || displayPortOffset > maximalDisplayPortOffset {

besides "display port offset" -> "display number" remark above, s/maximal/max/

---

pkg/tools/vnc.go, line 118 at r1 (raw file):

	displayPortOffset, err := strconv.Atoi(parts[2])
	if err != nil || displayPortOffset < 0 || displayPortOffset > maximalDisplayPortOffset {
		return fmt.Errorf("virsh returned %q as a display port offset while expected one was a non negative integer less than %d", parts[2], maximalDisplayPortOffset)

virsh returned %a a a display number which is expected to be in range 0 .. %d

---

pkg/tools/vnc.go, line 132 at r1 (raw file):

	fmt.Printf("VNC console for pod %q is available on local port %d\n", v.podName, pf.LocalPort)
	fmt.Printf("Press ctrl-c (or send a terminate singal) to finish that.\n")

Press Ctrl-C or kill the process to stop.

---

Comments from Reviewable

[jellonek]

Review status: 0 of 5 files reviewed at latest revision, 12 unresolved discussions.

---

pkg/tools/vnc.go, line 40 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

// vncCommand provides access to the VNC console of a VM pod

Done.

---

pkg/tools/vnc.go, line 47 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

... that provides access to the VNC console of a VM pod

Done.

---

pkg/tools/vnc.go, line 52 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Provide access to the VNC console of a VM pod

Done.

---

pkg/tools/vnc.go, line 57 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

I'd put it this way:

This command forwards a local port to the VNC port used by the specified VM pod. If no local port number is provided, a random available port is picked instead. The port number is displayed after the forwarding is set up, after which the commands enters an endless loop until it's interrupted with Ctrl-C.

Done.

---

pkg/tools/vnc.go, line 107 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

while expected to return a value of a form ...

Done.

---

pkg/tools/vnc.go, line 109 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

instea of expected %q

Done.

---

pkg/tools/vnc.go, line 111 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

instead of expected %q

Done.

---

pkg/tools/vnc.go, line 113 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

instead of expected %q

Done.

---

pkg/tools/vnc.go, line 116 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

I think it's called "display number" not "display port offset" in VNC.

Done.

---

pkg/tools/vnc.go, line 117 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

besides "display port offset" -> "display number" remark above, s/maximal/max/

Done.

---

pkg/tools/vnc.go, line 118 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

virsh returned %a a a display number which is expected to be in range 0 .. %d

Done.

---

pkg/tools/vnc.go, line 132 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Press Ctrl-C or kill the process to stop.

Done.

---

Comments from Reviewable

[ivan4th]

---

Reviewed 2 of 2 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[ivan4th]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

pkg/tools/vnc_test.go, line 72 at r2 (raw file):

			switch err := cmd.Execute(); {
			case err != nil && tc.errSubstring == "":
				t.Errorf("virsh command returned an unexpected error: %v", err)

vnc command

---

Comments from Reviewable

[jellonek]

Review status: 4 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

pkg/tools/vnc_test.go, line 72 at r2 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

vnc command

Done.

---

Comments from Reviewable

[ivan4th]

---

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[pigmej]

---

Reviewed 4 of 5 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable