Address comments from review

pkg/tools/vnc.go

@@ -33,12 +33,12 @@ import (
 )
 
 const (
- vncProtocolName  = "vnc"
- expectedHost  = "127.0.0.1"
- maximalDisplayPortOffset = 0xffff - 5900
+ vncProtocolName = "vnc"
+ expectedHost = "127.0.0.1"
+ maxDisplayNumber = 0xffff - 5900
 )
 
-// vncCommand gives an access to VNC console of VM pod.
+// vncCommand provides access to the VNC console of a VM pod
 type vncCommand struct {
  client KubeClient
  podName string
@@ -47,17 +47,18 @@ type vncCommand struct {
  waitForInterrupt bool
 }
 
-// NewVNCCmd returns a cobra.Command that gives an access to VNC console of VM pod.
+// NewVNCCmd returns a cobra.Command that provides access to the VNC console of a VM pod.
 func NewVNCCmd(client KubeClient, output io.Writer, waitForInterrupt bool) *cobra.Command {
  vnc := &vncCommand{client: client, output: output, waitForInterrupt: waitForInterrupt}
  cmd := &cobra.Command{
  Use: "vnc pod [port]",
- Short: "Provide an access to a VM pod VNC console",
+ Short: "Provide access to the VNC console of a VM pod",
  Long: dedent.Dedent(`
- This command prepares an access to a VM pod. If port is
- 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.
+ 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.
  `),
  RunE: func(cmd *cobra.Command, args []string) error {
  if len(args) < 1 {
@@ -108,22 +109,22 @@ func (v *vncCommand) Run() error {
  parts := strings.Split(strings.Trim(buffer.String(), "\n"), ":")
  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")
+ return fmt.Errorf("virsh returned %q, while expected to return a value of a form %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)
+ return fmt.Errorf("virsh returned %q as a display protocol instead of expected %q", parts[0], vncProtocolName)
  case parts[1][:2] != "//":
- return fmt.Errorf("virsh returned %q after first ':' while expected was %q", parts[1][:2], "//")
+ return fmt.Errorf("virsh returned %q after first ':' instead of expected %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)
+ return fmt.Errorf("virsh returned %q as a display host instead of expected %q", parts[1], expectedHost)
  }
 
- 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)
+ displayNumber, err := strconv.Atoi(parts[2])
+ if err != nil || displayNumber < 0 || displayNumber > maxDisplayNumber {
+ return fmt.Errorf("virsh returned %q as a display number which is expected to be in range 0..%d", parts[2], maximalDisplayPortOffset)
  }
 
  pf := &ForwardedPort{
- RemotePort: 5900 + uint16(displayPortOffset),
+ RemotePort: 5900 + uint16(displayNumber),
  LocalPort: v.port,
  }
  stopCh, err := v.client.ForwardPorts(vmPodInfo.VirtletPodName, "kube-system", []*ForwardedPort{pf})
@@ -133,7 +134,7 @@ func (v *vncCommand) Run() error {
  defer close(stopCh)
 
  fmt.Fprintf(v.output, "VNC console for pod %q is available on local port %d\n", v.podName, pf.LocalPort)
- fmt.Fprintf(v.output, "Press ctrl-c (or send a terminate singal) to finish that.\n")
+ fmt.Fprintf(v.output, "Press ctrl-c or kill the process stop.\n")
 
  // if waitForInterrupt is set to false do not wait for interrupt (e.x. in tests).
  if v.waitForInterrupt {