From c1824b41f46eb4bcbc5517133c5b7e4bc3e77fe4 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 5 Nov 2018 09:09:09 -0800 Subject: [PATCH 1/4] phantom ip/mac vlan network after a powercycle --- drivers/ipvlan/ipvlan_network.go | 67 ++++++++++++++++++---------- drivers/ipvlan/ipvlan_setup.go | 17 +++++++ drivers/ipvlan/ipvlan_store.go | 11 ++++- drivers/macvlan/macvlan_network.go | 71 ++++++++++++++++++++---------- drivers/macvlan/macvlan_setup.go | 17 +++++++ drivers/macvlan/macvlan_store.go | 12 ++++- endpoint_cnt.go | 5 +++ network.go | 8 +++- 8 files changed, 156 insertions(+), 52 deletions(-) diff --git a/drivers/ipvlan/ipvlan_network.go b/drivers/ipvlan/ipvlan_network.go index 8825e1e117..1262f66897 100644 --- a/drivers/ipvlan/ipvlan_network.go +++ b/drivers/ipvlan/ipvlan_network.go @@ -60,10 +60,14 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -76,22 +80,33 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } else { + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break + } + } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err - } - config.CreatedSlaveLink = true + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } + // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -100,24 +115,28 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + if !vlanLinkExists(config.Parent) { + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork the network for the specified driver type diff --git a/drivers/ipvlan/ipvlan_setup.go b/drivers/ipvlan/ipvlan_setup.go index da8d8faeb8..77c561d472 100644 --- a/drivers/ipvlan/ipvlan_setup.go +++ b/drivers/ipvlan/ipvlan_setup.go @@ -70,6 +70,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -156,6 +164,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/ipvlan/ipvlan_store.go b/drivers/ipvlan/ipvlan_store.go index 72eb3fc4ff..5dda6adbef 100644 --- a/drivers/ipvlan/ipvlan_store.go +++ b/drivers/ipvlan/ipvlan_store.go @@ -55,7 +55,14 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("ipvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } } return nil @@ -73,7 +80,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("could not create ipvlan network for id %s from persistent state", config.ID) } } diff --git a/drivers/macvlan/macvlan_network.go b/drivers/macvlan/macvlan_network.go index beeed41638..d8f1ed13bd 100644 --- a/drivers/macvlan/macvlan_network.go +++ b/drivers/macvlan/macvlan_network.go @@ -64,10 +64,15 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } + // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -80,22 +85,33 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } else { + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break + } } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Dummy Link %s for Mac Vlan already exists", getDummyName(stringid.TruncateID(config.ID))) } - config.CreatedSlaveLink = true // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -104,24 +120,33 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + + if !vlanLinkExists(config.Parent) { + // if the subinterface parent_iface.vlan_id checks do not pass, return err. + // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Parent Sub Interface %s already Exists NetID %s", config.Parent, config.ID) + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork deletes the network for the specified driver type diff --git a/drivers/macvlan/macvlan_setup.go b/drivers/macvlan/macvlan_setup.go index fc33ebd707..de059f7b30 100644 --- a/drivers/macvlan/macvlan_setup.go +++ b/drivers/macvlan/macvlan_setup.go @@ -74,6 +74,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -160,6 +168,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/macvlan/macvlan_store.go b/drivers/macvlan/macvlan_store.go index 8683cacd02..74a1b36375 100644 --- a/drivers/macvlan/macvlan_store.go +++ b/drivers/macvlan/macvlan_store.go @@ -55,7 +55,15 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("macvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } + } return nil @@ -73,7 +81,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("Could not create macvlan network for id %s from persistent state", config.ID) } } diff --git a/endpoint_cnt.go b/endpoint_cnt.go index 7b7527426d..c2c84bd12b 100644 --- a/endpoint_cnt.go +++ b/endpoint_cnt.go @@ -180,3 +180,8 @@ func (ec *endpointCnt) IncEndpointCnt() error { func (ec *endpointCnt) DecEndpointCnt() error { return ec.atomicIncDecEpCnt(false) } + +func (ec *endpointCnt) SetEndpointCnt(cnt uint64) error { + return ec.setCnt(cnt) +} + diff --git a/network.go b/network.go index 0a4a2277b0..7bdc234426 100644 --- a/network.go +++ b/network.go @@ -1051,7 +1051,13 @@ func (n *network) delete(force bool, rmLBEndpoint bool) error { if n.ConfigFrom() != "" { if t, err := c.getConfigNetwork(n.ConfigFrom()); err == nil { - if err := t.getEpCnt().DecEndpointCnt(); err != nil { + // ConfigOnly and ConfigFrom Networks have 1 to 1 correspondence? In that case + // there is no need to decrement the ref count, simply set it to zero. + // In the case of a powercycle, the count on the configfrom network increments more than + // once and this can cause the configfrom network to linger forever and not even deleteable. + // This happens due to the additional increment after the power on, hence count can never + // become zero. + if err := t.getEpCnt().SetEndpointCnt(0); err != nil { logrus.Warnf("Failed to update reference count for configuration network %q on removal of network %q: %v", t.Name(), n.Name(), err) } From 91cab66ff00815698dd1c4ca03d12d474345584e Mon Sep 17 00:00:00 2001 From: Rajesh Nataraja Date: Mon, 5 Nov 2018 09:46:39 -0800 Subject: [PATCH 2/4] phantom ip/mac vlan network after a powercycle --- drivers/ipvlan/ipvlan_network.go | 67 ++++++++++++++++++---------- drivers/ipvlan/ipvlan_setup.go | 17 +++++++ drivers/ipvlan/ipvlan_store.go | 11 ++++- drivers/macvlan/macvlan_network.go | 71 ++++++++++++++++++++---------- drivers/macvlan/macvlan_setup.go | 17 +++++++ drivers/macvlan/macvlan_store.go | 12 ++++- endpoint_cnt.go | 5 +++ network.go | 8 +++- 8 files changed, 156 insertions(+), 52 deletions(-) diff --git a/drivers/ipvlan/ipvlan_network.go b/drivers/ipvlan/ipvlan_network.go index 8825e1e117..1262f66897 100644 --- a/drivers/ipvlan/ipvlan_network.go +++ b/drivers/ipvlan/ipvlan_network.go @@ -60,10 +60,14 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -76,22 +80,33 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } else { + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break + } + } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err - } - config.CreatedSlaveLink = true + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } + // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -100,24 +115,28 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + if !vlanLinkExists(config.Parent) { + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork the network for the specified driver type diff --git a/drivers/ipvlan/ipvlan_setup.go b/drivers/ipvlan/ipvlan_setup.go index da8d8faeb8..77c561d472 100644 --- a/drivers/ipvlan/ipvlan_setup.go +++ b/drivers/ipvlan/ipvlan_setup.go @@ -70,6 +70,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -156,6 +164,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/ipvlan/ipvlan_store.go b/drivers/ipvlan/ipvlan_store.go index 72eb3fc4ff..5dda6adbef 100644 --- a/drivers/ipvlan/ipvlan_store.go +++ b/drivers/ipvlan/ipvlan_store.go @@ -55,7 +55,14 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("ipvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } } return nil @@ -73,7 +80,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("could not create ipvlan network for id %s from persistent state", config.ID) } } diff --git a/drivers/macvlan/macvlan_network.go b/drivers/macvlan/macvlan_network.go index beeed41638..d8f1ed13bd 100644 --- a/drivers/macvlan/macvlan_network.go +++ b/drivers/macvlan/macvlan_network.go @@ -64,10 +64,15 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } + // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -80,22 +85,33 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } else { + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break + } } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Dummy Link %s for Mac Vlan already exists", getDummyName(stringid.TruncateID(config.ID))) } - config.CreatedSlaveLink = true // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -104,24 +120,33 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + + if !vlanLinkExists(config.Parent) { + // if the subinterface parent_iface.vlan_id checks do not pass, return err. + // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Parent Sub Interface %s already Exists NetID %s", config.Parent, config.ID) + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork deletes the network for the specified driver type diff --git a/drivers/macvlan/macvlan_setup.go b/drivers/macvlan/macvlan_setup.go index fc33ebd707..de059f7b30 100644 --- a/drivers/macvlan/macvlan_setup.go +++ b/drivers/macvlan/macvlan_setup.go @@ -74,6 +74,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -160,6 +168,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/macvlan/macvlan_store.go b/drivers/macvlan/macvlan_store.go index 8683cacd02..74a1b36375 100644 --- a/drivers/macvlan/macvlan_store.go +++ b/drivers/macvlan/macvlan_store.go @@ -55,7 +55,15 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("macvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } + } return nil @@ -73,7 +81,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("Could not create macvlan network for id %s from persistent state", config.ID) } } diff --git a/endpoint_cnt.go b/endpoint_cnt.go index 7b7527426d..c2c84bd12b 100644 --- a/endpoint_cnt.go +++ b/endpoint_cnt.go @@ -180,3 +180,8 @@ func (ec *endpointCnt) IncEndpointCnt() error { func (ec *endpointCnt) DecEndpointCnt() error { return ec.atomicIncDecEpCnt(false) } + +func (ec *endpointCnt) SetEndpointCnt(cnt uint64) error { + return ec.setCnt(cnt) +} + diff --git a/network.go b/network.go index 0a4a2277b0..7bdc234426 100644 --- a/network.go +++ b/network.go @@ -1051,7 +1051,13 @@ func (n *network) delete(force bool, rmLBEndpoint bool) error { if n.ConfigFrom() != "" { if t, err := c.getConfigNetwork(n.ConfigFrom()); err == nil { - if err := t.getEpCnt().DecEndpointCnt(); err != nil { + // ConfigOnly and ConfigFrom Networks have 1 to 1 correspondence? In that case + // there is no need to decrement the ref count, simply set it to zero. + // In the case of a powercycle, the count on the configfrom network increments more than + // once and this can cause the configfrom network to linger forever and not even deleteable. + // This happens due to the additional increment after the power on, hence count can never + // become zero. + if err := t.getEpCnt().SetEndpointCnt(0); err != nil { logrus.Warnf("Failed to update reference count for configuration network %q on removal of network %q: %v", t.Name(), n.Name(), err) } From 766eac3f330d28f0b6a06000df8d8eb4450f3d57 Mon Sep 17 00:00:00 2001 From: Rajesh Nataraja Date: Mon, 5 Nov 2018 18:25:45 -0800 Subject: [PATCH 3/4] lint errors --- drivers/ipvlan/ipvlan_network.go | 8 +++----- drivers/macvlan/macvlan_network.go | 9 ++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/ipvlan/ipvlan_network.go b/drivers/ipvlan/ipvlan_network.go index 1262f66897..79e7b735bd 100644 --- a/drivers/ipvlan/ipvlan_network.go +++ b/drivers/ipvlan/ipvlan_network.go @@ -88,12 +88,10 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { if config.ID != nw.config.ID { return false, fmt.Errorf("network %s is already using parent interface %s", getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) - } else { - logrus.Debugf("Create Network for the same ID %s\n", config.ID) - foundExisting = true - break } - + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break } } if !parentExists(config.Parent) { diff --git a/drivers/macvlan/macvlan_network.go b/drivers/macvlan/macvlan_network.go index d8f1ed13bd..f364eb23f6 100644 --- a/drivers/macvlan/macvlan_network.go +++ b/drivers/macvlan/macvlan_network.go @@ -93,11 +93,10 @@ func (d *driver) createNetwork(config *configuration) (bool, error) { if config.ID != nw.config.ID { return false, fmt.Errorf("network %s is already using parent interface %s", getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) - } else { - logrus.Debugf("Create Network for the same ID %s\n", config.ID) - foundExisting = true - break - } + } + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break } } if !parentExists(config.Parent) { From d8f08041fcb0a7b13584c1d0a2789669cf2fc330 Mon Sep 17 00:00:00 2001 From: Rajesh Nataraja Date: Mon, 5 Nov 2018 09:46:39 -0800 Subject: [PATCH 4/4] phantom ip/mac vlan network after a powercycle --- drivers/ipvlan/ipvlan_network.go | 65 +++++++++++++++++---------- drivers/ipvlan/ipvlan_setup.go | 17 ++++++++ drivers/ipvlan/ipvlan_store.go | 11 ++++- drivers/macvlan/macvlan_network.go | 70 ++++++++++++++++++++---------- drivers/macvlan/macvlan_setup.go | 17 ++++++++ drivers/macvlan/macvlan_store.go | 12 ++++- endpoint_cnt.go | 5 +++ network.go | 8 +++- 8 files changed, 153 insertions(+), 52 deletions(-) diff --git a/drivers/ipvlan/ipvlan_network.go b/drivers/ipvlan/ipvlan_network.go index 8825e1e117..79e7b735bd 100644 --- a/drivers/ipvlan/ipvlan_network.go +++ b/drivers/ipvlan/ipvlan_network.go @@ -60,10 +60,14 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -76,22 +80,31 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err - } - config.CreatedSlaveLink = true + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } + // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -100,24 +113,28 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + if !vlanLinkExists(config.Parent) { + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork the network for the specified driver type diff --git a/drivers/ipvlan/ipvlan_setup.go b/drivers/ipvlan/ipvlan_setup.go index da8d8faeb8..77c561d472 100644 --- a/drivers/ipvlan/ipvlan_setup.go +++ b/drivers/ipvlan/ipvlan_setup.go @@ -70,6 +70,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -156,6 +164,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/ipvlan/ipvlan_store.go b/drivers/ipvlan/ipvlan_store.go index 72eb3fc4ff..5dda6adbef 100644 --- a/drivers/ipvlan/ipvlan_store.go +++ b/drivers/ipvlan/ipvlan_store.go @@ -55,7 +55,14 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("ipvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } } return nil @@ -73,7 +80,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("could not create ipvlan network for id %s from persistent state", config.ID) } } diff --git a/drivers/macvlan/macvlan_network.go b/drivers/macvlan/macvlan_network.go index beeed41638..f364eb23f6 100644 --- a/drivers/macvlan/macvlan_network.go +++ b/drivers/macvlan/macvlan_network.go @@ -64,10 +64,15 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // empty parent and --internal are handled the same. Set here to update k/v config.Internal = true } - err = d.createNetwork(config) + foundExisting, err := d.createNetwork(config) if err != nil { return err } + + if foundExisting { + return nil + } + // update persistent db, rollback on fail err = d.storeUpdate(config) if err != nil { @@ -80,22 +85,32 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo } // createNetwork is used by new network callbacks and persistent network cache -func (d *driver) createNetwork(config *configuration) error { +func (d *driver) createNetwork(config *configuration) (bool, error) { + foundExisting := false networkList := d.getNetworks() for _, nw := range networkList { if config.Parent == nw.config.Parent { - return fmt.Errorf("network %s is already using parent interface %s", - getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + if config.ID != nw.config.ID { + return false, fmt.Errorf("network %s is already using parent interface %s", + getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent) + } + logrus.Debugf("Create Network for the same ID %s\n", config.ID) + foundExisting = true + break } } if !parentExists(config.Parent) { // if the --internal flag is set, create a dummy link if config.Internal { - err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) - if err != nil { - return err + if !dummyLinkExists(getDummyName(stringid.TruncateID(config.ID))) { + err := createDummyLink(config.Parent, getDummyName(stringid.TruncateID(config.ID))) + if err != nil { + return false, err + } + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Dummy Link %s for Mac Vlan already exists", getDummyName(stringid.TruncateID(config.ID))) } - config.CreatedSlaveLink = true // notify the user in logs they have limited communications if config.Parent == getDummyName(stringid.TruncateID(config.ID)) { logrus.Debugf("Empty -o parent= and --internal flags limit communications to other containers inside of network: %s", @@ -104,24 +119,33 @@ func (d *driver) createNetwork(config *configuration) error { } else { // if the subinterface parent_iface.vlan_id checks do not pass, return err. // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' - err := createVlanLink(config.Parent) - if err != nil { - return err - } - // if driver created the networks slave link, record it for future deletion - config.CreatedSlaveLink = true + + if !vlanLinkExists(config.Parent) { + // if the subinterface parent_iface.vlan_id checks do not pass, return err. + // a valid example is 'eth0.10' for a parent iface 'eth0' with a vlan id '10' + err := createVlanLink(config.Parent) + if err != nil { + return false, err + } + // if driver created the networks slave link, record it for future deletion + config.CreatedSlaveLink = true + } else { + logrus.Debugf("Parent Sub Interface %s already Exists NetID %s", config.Parent, config.ID) + } } } - n := &network{ - id: config.ID, - driver: d, - endpoints: endpointTable{}, - config: config, - } - // add the *network - d.addNetwork(n) + if !foundExisting { + n := &network{ + id: config.ID, + driver: d, + endpoints: endpointTable{}, + config: config, + } + // add the *network + d.addNetwork(n) + } - return nil + return foundExisting, nil } // DeleteNetwork deletes the network for the specified driver type diff --git a/drivers/macvlan/macvlan_setup.go b/drivers/macvlan/macvlan_setup.go index fc33ebd707..de059f7b30 100644 --- a/drivers/macvlan/macvlan_setup.go +++ b/drivers/macvlan/macvlan_setup.go @@ -74,6 +74,14 @@ func parentExists(ifaceStr string) bool { return true } +func vlanLinkExists(linkStr string) bool { + _, err := ns.NlHandle().LinkByName(linkStr) + if err != nil { + return false + } + return true +} + // createVlanLink parses sub-interfaces and vlan id for creation func createVlanLink(parentName string) error { if strings.Contains(parentName, ".") { @@ -160,6 +168,15 @@ func parseVlan(linkName string) (string, int, error) { return parent, vidInt, nil } +func dummyLinkExists(dummyName string) bool { + _, err := ns.NlHandle().LinkByName(dummyName) + if err != nil { + return false + } + return true +} + + // createDummyLink creates a dummy0 parent link func createDummyLink(dummyName, truncNetID string) error { // create a parent interface since one was not specified diff --git a/drivers/macvlan/macvlan_store.go b/drivers/macvlan/macvlan_store.go index 8683cacd02..74a1b36375 100644 --- a/drivers/macvlan/macvlan_store.go +++ b/drivers/macvlan/macvlan_store.go @@ -55,7 +55,15 @@ func (d *driver) initStore(option map[string]interface{}) error { return types.InternalErrorf("macvlan driver failed to initialize data store: %v", err) } - return d.populateNetworks() + err = d.populateNetworks() + if err != nil { + return err + } + err = d.populateEndpoints() + if err != nil { + return err + } + } return nil @@ -73,7 +81,7 @@ func (d *driver) populateNetworks() error { } for _, kvo := range kvol { config := kvo.(*configuration) - if err = d.createNetwork(config); err != nil { + if _, err = d.createNetwork(config); err != nil { logrus.Warnf("Could not create macvlan network for id %s from persistent state", config.ID) } } diff --git a/endpoint_cnt.go b/endpoint_cnt.go index 7b7527426d..c2c84bd12b 100644 --- a/endpoint_cnt.go +++ b/endpoint_cnt.go @@ -180,3 +180,8 @@ func (ec *endpointCnt) IncEndpointCnt() error { func (ec *endpointCnt) DecEndpointCnt() error { return ec.atomicIncDecEpCnt(false) } + +func (ec *endpointCnt) SetEndpointCnt(cnt uint64) error { + return ec.setCnt(cnt) +} + diff --git a/network.go b/network.go index 0a4a2277b0..7bdc234426 100644 --- a/network.go +++ b/network.go @@ -1051,7 +1051,13 @@ func (n *network) delete(force bool, rmLBEndpoint bool) error { if n.ConfigFrom() != "" { if t, err := c.getConfigNetwork(n.ConfigFrom()); err == nil { - if err := t.getEpCnt().DecEndpointCnt(); err != nil { + // ConfigOnly and ConfigFrom Networks have 1 to 1 correspondence? In that case + // there is no need to decrement the ref count, simply set it to zero. + // In the case of a powercycle, the count on the configfrom network increments more than + // once and this can cause the configfrom network to linger forever and not even deleteable. + // This happens due to the additional increment after the power on, hence count can never + // become zero. + if err := t.getEpCnt().SetEndpointCnt(0); err != nil { logrus.Warnf("Failed to update reference count for configuration network %q on removal of network %q: %v", t.Name(), n.Name(), err) }