Skip to content

Commit

Permalink
Merge pull request #3148 from etherparty/fix-issue-2513
Browse files Browse the repository at this point in the history 
Fix issue 2513
  • Loading branch information
@tstromberg
tstromberg authored Sep 27, 2018
2 parents e49971a + 5d0e123 commit 74f8e49
Show file tree
Hide file tree
Showing 3 changed files with 208 additions and 43 deletions.
31 changes: 25 additions & 6 deletions pkg/drivers/kvm/domain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const domainTmpl = `
</interface>
<interface type='network'>
<source network='{{.PrivateNetwork}}'/>
<mac address='{{.MAC}}'/>
<mac address='{{.PrivateMAC}}'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
Expand Down Expand Up @@ -98,7 +98,7 @@ $ sudo usermod -a -G libvirt $(whoami)
# NOTE: For older Debian/Ubuntu versions change the group to [libvirtd]
$ newgrp libvirt
Visit https://github.com/kubernetes/minikube/blob/master/docs/drivers.md#kvm-driver for more information.
Visit https://github.com/kubernetes/minikube/blob/master/docs/drivers.md#kvm2-driver for more information.
`

func randomMAC() (net.HardwareAddr, error) {
Expand Down Expand Up @@ -152,9 +152,27 @@ func closeDomain(dom *libvirt.Domain, conn *libvirt.Connect) error {
}

func (d *Driver) createDomain() (*libvirt.Domain, error) {
// create random MAC addresses first for our NICs
if d.MAC == "" {
mac, err := randomMAC()
if err != nil {
return nil, errors.Wrap(err, "generating mac address")
}
d.MAC = mac.String()
}

if d.PrivateMAC == "" {
mac, err := randomMAC()
if err != nil {
return nil, errors.Wrap(err, "generating mac address")
}
d.PrivateMAC = mac.String()
}

// create the XML for the domain using our domainTmpl template
tmpl := template.Must(template.New("domain").Parse(domainTmpl))
var domainXml bytes.Buffer
if err := tmpl.Execute(&domainXml, d); err != nil {
var domainXML bytes.Buffer
if err := tmpl.Execute(&domainXML, d); err != nil {
return nil, errors.Wrap(err, "executing domain xml")
}

Expand All @@ -164,9 +182,10 @@ func (d *Driver) createDomain() (*libvirt.Domain, error) {
}
defer conn.Close()

dom, err := conn.DomainDefineXML(domainXml.String())
// define the domain in libvirt using the generated XML
dom, err := conn.DomainDefineXML(domainXML.String())
if err != nil {
return nil, errors.Wrapf(err, "Error defining domain xml: %s", domainXml.String())
return nil, errors.Wrapf(err, "Error defining domain xml: %s", domainXML.String())
}

return dom, nil
Expand Down
36 changes: 26 additions & 10 deletions pkg/drivers/kvm/kvm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ type Driver struct {
// If empty, a random MAC will be generated.
MAC string

// The randomly generated MAC Address for the NIC attached to the private network
// If empty, a random MAC will be generated.
PrivateMAC string

// Whether to passthrough GPU devices from the host to the VM.
GPU bool

Expand Down Expand Up @@ -211,6 +215,21 @@ func (d *Driver) Restart() error {
}

func (d *Driver) Start() error {
// if somebody/something deleted the network in the meantime,
// we might need to recreate it. It's (nearly) a noop if the network exists.
log.Info("Creating network...")
err := d.createNetwork()
if err != nil {
return errors.Wrap(err, "creating network")
}

// this call ensures that all networks are active
log.Info("Ensuring networks are active...")
err = d.ensureNetwork()
if err != nil {
return errors.Wrap(err, "ensuring active networks")
}

log.Info("Getting domain xml...")
dom, conn, err := d.getDomain()
if err != nil {
Expand Down Expand Up @@ -348,18 +367,15 @@ func (d *Driver) Remove() error {
}
defer conn.Close()

//Tear down network and disk if they exist
log.Debug("Checking if the network needs to be deleted")
network, err := conn.LookupNetworkByName(d.PrivateNetwork)
if err != nil {
log.Warn("Network %s does not exist, nothing to clean up...", d.PrivateNetwork)
}
if network != nil {
log.Infof("Network %s exists, removing...", d.PrivateNetwork)
network.Destroy()
network.Undefine()
// Tear down network if it exists and is not in use by another minikube instance
log.Debug("Trying to delete the networks (if possible)")
if err := d.deleteNetwork(); err != nil {
log.Warnf("Deleting of networks failed: %s", err.Error())
} else {
log.Info("Successfully deleted networks")
}

// Tear down the domain now
log.Debug("Checking if the domain needs to be deleted")
dom, err := conn.LookupDomainByName(d.MachineName)
if err != nil {
Expand Down
184 changes: 157 additions & 27 deletions pkg/drivers/kvm/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package kvm
import (
"bytes"
"encoding/json"
"encoding/xml"
"fmt"
"io/ioutil"
"strings"
Expand All @@ -43,11 +44,15 @@ const networkTmpl = `
</network>
`

// setupNetwork ensures that the network with `name` is started (active)
// and has the autostart feature set.
func setupNetwork(conn *libvirt.Connect, name string) error {
n, err := conn.LookupNetworkByName(defaultNetworkName)
n, err := conn.LookupNetworkByName(name)
if err != nil {
return errors.Wrapf(err, "checking network %s", name)
}

// always ensure autostart is set on the network
autostart, err := n.GetAutostart()
if err != nil {
return errors.Wrapf(err, "checking network %s autostart", name)
Expand All @@ -58,6 +63,7 @@ func setupNetwork(conn *libvirt.Connect, name string) error {
}
}

// always ensure the network is started (active)
active, err := n.IsActive()
if err != nil {
return errors.Wrapf(err, "checking network status for %s", name)
Expand All @@ -67,52 +73,175 @@ func setupNetwork(conn *libvirt.Connect, name string) error {
return errors.Wrapf(err, "starting network %s", name)
}
}

return nil
}

// ensureNetwork is called on start of the VM
func (d *Driver) ensureNetwork() error {
conn, err := getConnection()
if err != nil {
return errors.Wrap(err, "getting libvirt connection")
}
defer conn.Close()

// network: default

// Start the default network
// It is assumed that the libvirt/kvm installation has already created this network
log.Infof("Ensuring network %s is active", defaultNetworkName)
if err := setupNetwork(conn, defaultNetworkName); err != nil {
return err
}

// network: private

// Start the private network
log.Infof("Ensuring network %s is active", d.PrivateNetwork)
if err := setupNetwork(conn, d.PrivateNetwork); err != nil {
return err
}

return nil
}

// createNetwork is called during creation of the VM only (and not on start)
func (d *Driver) createNetwork() error {
if d.MAC == "" {
mac, err := randomMAC()
conn, err := getConnection()
if err != nil {
return errors.Wrap(err, "getting libvirt connection")
}
defer conn.Close()

// network: default
// It is assumed that the libvirt/kvm installation has already created this network

// network: private

// Only create the private network if it does not already exist
if _, err := conn.LookupNetworkByName(d.PrivateNetwork); err != nil {
// create the XML for the private network from our networkTmpl
tmpl := template.Must(template.New("network").Parse(networkTmpl))
var networkXML bytes.Buffer
if err := tmpl.Execute(&networkXML, d); err != nil {
return errors.Wrap(err, "executing network template")
}

// define the network using our template
network, err := conn.NetworkDefineXML(networkXML.String())
if err != nil {
return errors.Wrap(err, "generating mac address")
return errors.Wrapf(err, "defining network from xml: %s", networkXML.String())
}

// and finally create it
if err := network.Create(); err != nil {
return errors.Wrapf(err, "creating network %s", d.PrivateNetwork)
}
d.MAC = mac.String()
}

return nil
}

func (d *Driver) deleteNetwork() error {
type source struct {
//XMLName xml.Name `xml:"source"`
Network string `xml:"network,attr"`
}
type iface struct {
//XMLName xml.Name `xml:"interface"`
Source source `xml:"source"`
}
type result struct {
//XMLName xml.Name `xml:"domain"`
Name string `xml:"name"`
Interfaces []iface `xml:"devices>interface"`
}

conn, err := getConnection()
if err != nil {
return errors.Wrap(err, "getting libvirt connection")
}
defer conn.Close()

tmpl := template.Must(template.New("network").Parse(networkTmpl))
var networkXML bytes.Buffer
if err := tmpl.Execute(&networkXML, d); err != nil {
return errors.Wrap(err, "executing network template")
}
// network: default
// It is assumed that the OS manages this network

// Start the default network
log.Infof("Setting up network %s", defaultNetworkName)
if err := setupNetwork(conn, defaultNetworkName); err != nil {
return err
// network: private
log.Debugf("Checking if network %s exists...", d.PrivateNetwork)
network, err := conn.LookupNetworkByName(d.PrivateNetwork)
if err != nil {
// TODO: decide if we really wanna throw an error?
return errors.Wrap(err, "network %s does not exist")
}
log.Debugf("Network %s exists", d.PrivateNetwork)

//Check if network already exists
if _, err := conn.LookupNetworkByName(d.PrivateNetwork); err == nil {
return nil
// iterate over every (also turned off) domains, and check if it
// is using the private network. Do *not* delete the network if
// that is the case
log.Debug("Trying to list all domains...")
doms, err := conn.ListAllDomains(0)
if err != nil {
return errors.Wrap(err, "list all domains")
}
log.Debugf("Listed all domains: total of %d domains", len(doms))

network, err := conn.NetworkDefineXML(networkXML.String())
if err != nil {
return errors.Wrapf(err, "defining network from xml: %s", networkXML.String())
// fail if there are 0 domains
if len(doms) == 0 {
log.Warn("list of domains is 0 length")
}
if err := network.Create(); err != nil {
return errors.Wrapf(err, "creating network %s", d.PrivateNetwork)

for _, dom := range doms {
// get the name of the domain we iterate over
log.Debug("Trying to get name of domain...")
name, err := dom.GetName()
if err != nil {
return errors.Wrap(err, "failed to get name of a domain")
}
log.Debugf("Got domain name: %s", name)

// skip the domain if it is our own machine
if name == d.MachineName {
log.Debug("Skipping domain as it is us...")
continue
}

// unfortunately, there is no better way to retrieve a list of all defined interfaces
// in domains than getting it from the defined XML of all domains
// NOTE: conn.ListAllInterfaces does not help in this case
log.Debugf("Getting XML for domain %s...", name)
xmlString, err := dom.GetXMLDesc(libvirt.DOMAIN_XML_INACTIVE)
if err != nil {
return errors.Wrapf(err, "failed to get XML of domain '%s'", name)
}
log.Debugf("Got XML for domain %s", name)

v := result{}
err = xml.Unmarshal([]byte(xmlString), &v)
if err != nil {
return errors.Wrapf(err, "failed to unmarshal XML of domain '%s", name)
}
log.Debugf("Unmarshaled XML for domain %s: %#v", name, v)

// iterate over the found interfaces
for _, i := range v.Interfaces {
if i.Source.Network == d.PrivateNetwork {
log.Debugf("domain %s DOES use network %s, aborting...", name, d.PrivateNetwork)
return fmt.Errorf("network still in use at least by domain '%s',", name)
}
log.Debugf("domain %s does not use network %s", name, d.PrivateNetwork)
}
}

log.Infof("Setting up network %s", d.PrivateNetwork)
if err := setupNetwork(conn, d.PrivateNetwork); err != nil {
return err
// when we reach this point, it means it is safe to delete the network
log.Debugf("Trying to destroy network %s...", d.PrivateNetwork)
err = network.Destroy()
if err != nil {
return errors.Wrap(err, "network destroy")
}
log.Debugf("Trying to undefine network %s...", d.PrivateNetwork)
err = network.Undefine()
if err != nil {
return errors.Wrap(err, "network undefine")
}

return nil
Expand All @@ -136,6 +265,7 @@ func (d *Driver) lookupIP() (string, error) {
return d.lookupIPFromLeasesFile()
}

// TODO: for everything > 1002006, there is direct support in the libvirt-go for handling this
return d.lookupIPFromStatusFile(conn)
}

Expand Down Expand Up @@ -167,7 +297,7 @@ func (d *Driver) lookupIPFromStatusFile(conn *libvirt.Connect) (string, error) {

ipAddress := ""
for _, status := range statusEntries {
if status.MacAddress == d.MAC {
if status.MacAddress == d.PrivateMAC {
ipAddress = status.IPAddress
}
}
Expand All @@ -192,7 +322,7 @@ func (d *Driver) lookupIPFromLeasesFile() (string, error) {
if len(entry) != 5 {
return "", fmt.Errorf("Malformed leases entry: %s", entry)
}
if entry[1] == d.MAC {
if entry[1] == d.PrivateMAC {
ipAddress = entry[2]
}
}
Expand Down

0 comments on commit 74f8e49

Please sign in to comment.