Review feedback

kubernetes · Aug 23, 2017 · 1b716db · 1b716db
1 parent 819a81e
commit 1b716db
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 31 deletions.
diff --git a/cmd/drivers/kvm/main.go b/cmd/drivers/kvm/main.go
@@ -1,3 +1,5 @@
+// +build linux
+
 /*
 Copyright 2016 The Kubernetes Authors All rights reserved.
 

diff --git a/pkg/drivers/kvm/domain.go b/pkg/drivers/kvm/domain.go
@@ -70,6 +70,30 @@ const domainTmpl = `
 </domain>
 `
 
+const connectionErrorText = `
+Error connecting to libvirt socket.  Have you set up libvirt correctly?
+
+# Install libvirt and qemu-kvm on your system, e.g.
+# Debian/Ubuntu
+$ sudo apt install libvirt-bin qemu-kvm
+# Fedora/CentOS/RHEL
+$ sudo yum install libvirt-daemon-kvm qemu-kvm
+
+# Add yourself to the libvirtd group (use libvirt group for rpm based distros) so you don't need to sudo
+# Debian/Ubuntu (NOTE: For Ubuntu 17.04 change the group to libvirt)
+$ sudo usermod -a -G libvirtd $(whoami)
+# Fedora/CentOS/RHEL
+$ sudo usermod -a -G libvirt $(whoami)
+
+# Update your current session for the group change to take effect
+# Debian/Ubuntu (NOTE: For Ubuntu 17.04 change the group to libvirt)
+$ newgrp libvirtd
+# Fedora/CentOS/RHEL
+$ newgrp libvirt
+
+Visit https://github.com/kubernetes/minikube/blob/master/docs/drivers.md#kvm-driver for more information.
+`
+
 func (d *Driver) getDomain() (*libvirt.Domain, *libvirt.Connect, error) {
 	conn, err := getConnection()
 	if err != nil {
@@ -87,7 +111,7 @@ func (d *Driver) getDomain() (*libvirt.Domain, *libvirt.Connect, error) {
 func getConnection() (*libvirt.Connect, error) {
 	conn, err := libvirt.NewConnect(qemusystem)
 	if err != nil {
-		return nil, errors.Wrap(err, "Error connecting to libvirt socket")
+		return nil, errors.Wrap(err, connectionErrorText)
 	}
 
 	return conn, nil
@@ -104,8 +128,7 @@ func closeDomain(dom *libvirt.Domain, conn *libvirt.Connect) error {
 func (d *Driver) createDomain() (*libvirt.Domain, error) {
 	tmpl := template.Must(template.New("domain").Parse(domainTmpl))
 	var domainXml bytes.Buffer
-	err := tmpl.Execute(&domainXml, d)
-	if err != nil {
+	if err := tmpl.Execute(&domainXml, d); err != nil {
 		return nil, errors.Wrap(err, "executing domain xml")
 	}
 

diff --git a/pkg/drivers/kvm/kvm.go b/pkg/drivers/kvm/kvm.go
@@ -122,16 +122,6 @@ func (d *Driver) GetURL() (string, error) {
 		return "", nil
 	}
 
-	for {
-		err := drivers.WaitForSSH(d)
-		if err != nil {
-			d.IPAddress = ""
-			time.Sleep(1 * time.Second)
-		} else {
-			break
-		}
-	}
-
 	return fmt.Sprintf("tcp://%s:2376", ip), nil
 }
 
@@ -147,24 +137,26 @@ func (d *Driver) GetState() (state.State, error) {
 		return state.None, errors.Wrap(err, "getting domain state")
 	}
 
-	stateMap := map[libvirt.DomainState]state.State{
-		libvirt.DOMAIN_NOSTATE:     state.None,
-		libvirt.DOMAIN_RUNNING:     state.Running,
-		libvirt.DOMAIN_BLOCKED:     state.Error,
-		libvirt.DOMAIN_PAUSED:      state.Paused,
-		libvirt.DOMAIN_SHUTDOWN:    state.Stopped,
-		libvirt.DOMAIN_CRASHED:     state.Error,
-		libvirt.DOMAIN_PMSUSPENDED: state.Saved,
-		libvirt.DOMAIN_SHUTOFF:     state.Stopped,
-	}
-
-	val, ok := stateMap[libvirtState]
-
-	if !ok {
+	switch libvirtState {
+	case libvirt.DOMAIN_NOSTATE:
+		return state.None, nil
+	case libvirt.DOMAIN_RUNNING:
+		return state.Running, nil
+	case libvirt.DOMAIN_BLOCKED:
+		return state.Error, nil
+	case libvirt.DOMAIN_PAUSED:
+		return state.Paused, nil
+	case libvirt.DOMAIN_SHUTDOWN:
+		return state.Stopped, nil
+	case libvirt.DOMAIN_CRASHED:
+		return state.Error, nil
+	case libvirt.DOMAIN_PMSUSPENDED:
+		return state.Saved, nil
+	case libvirt.DOMAIN_SHUTOFF:
+		return state.Stopped, nil
+	default:
 		return state.None, nil
 	}
-
-	return val, nil
 }
 
 func (d *Driver) GetIP() (string, error) {
@@ -248,7 +240,6 @@ func (d *Driver) Start() error {
 	}
 
 	log.Info("Waiting to get IP...")
-	time.Sleep(5 * time.Second)
 	for i := 0; i <= 40; i++ {
 		ip, err := d.GetIP()
 		if err != nil {

diff --git a/pkg/drivers/kvm/network.go b/pkg/drivers/kvm/network.go
@@ -125,7 +125,9 @@ func (d *Driver) lookupIPFromStatusFile(conn *libvirt.Connect) (string, error) {
 
 	var statusEntries []StatusEntry
 
-	err = json.Unmarshal(statuses, &statusEntries)
+	if err := json.Unmarshal(statuses, &statusEntries); err != nil {
+		return "", errors.Wrap(err, "unmarshal status json")
+	}
 	ipAddress := ""
 	for _, status := range statusEntries {
 		if status.Hostname == d.MachineName {