Skip to content

Support dockerd and system restarts for ipvlan and macvlan networks #2415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,10 @@ const overlayDSROptionString = "dsr"
// are network specific and modeled in a generic way.
func (c *controller) NewNetwork(networkType, name string, id string, options ...NetworkOption) (Network, error) {
var (
cap *driverapi.Capability
err error
t *network
cap *driverapi.Capability
err error
t *network
skipCfgEpCount bool
)

if id != "" {
Expand Down Expand Up @@ -802,7 +803,7 @@ func (c *controller) NewNetwork(networkType, name string, id string, options ...
return nil, types.InternalErrorf("Failed to apply configuration: %v", err)
}
defer func() {
if err == nil {
if err == nil && !skipCfgEpCount {
if err := t.getEpCnt().IncEndpointCnt(); err != nil {
logrus.Warnf("Failed to update reference count for configuration network %q on creation of network %q: %v",
t.Name(), network.Name(), err)
Expand All @@ -823,7 +824,13 @@ func (c *controller) NewNetwork(networkType, name string, id string, options ...

err = c.addNetwork(network)
if err != nil {
return nil, err
if strings.Contains(err.Error(), "restoring existing network") {
// This error can be ignored and set this boolean
// value to skip a refcount increment for configOnly networks
skipCfgEpCount = true
} else {
return nil, err
}
}
defer func() {
if err != nil {
Expand Down
65 changes: 44 additions & 21 deletions drivers/ipvlan/ipvlan_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 types.InternalMaskableErrorf("restoring existing network %s", config.ID)
}
// update persistent db, rollback on fail
err = d.storeUpdate(config)
if err != nil {
Expand All @@ -76,22 +80,34 @@ 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 ipvlan 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",
Expand All @@ -100,24 +116,31 @@ 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 !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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am hoping CreateSlaveLink will be already set to true vlanLinkExists returns True.
If not, can you pls move this line out of if condition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ip mac vlan code I see there is else condition with debug log "vlan already exists" . should we add similar log here for consistent purpose ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the missing logs, also config.CreatedSlaveLink is already set to true, that is why we don't set it for the restart case

} else {
logrus.Debugf("Parent Sub Interface %s already Exists NetID %s", config.Parent, config.ID)
}
// 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,
if !foundExisting {
n := &network{
id: config.ID,
driver: d,
endpoints: endpointTable{},
config: config,
}
// add the network
d.addNetwork(n)
}
// add the *network
d.addNetwork(n)

return nil
return foundExisting, nil
}

// DeleteNetwork the network for the specified driver type
Expand Down
18 changes: 18 additions & 0 deletions drivers/ipvlan/ipvlan_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ func parentExists(ifaceStr string) bool {
return true
}

// vlanLinkExists checks if specified vlan link exists in the default namespace
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, ".") {
Expand Down Expand Up @@ -156,6 +165,15 @@ func parseVlan(linkName string) (string, int, error) {
return parent, vidInt, nil
}

// dummyLinkExists checks if dummylink exists in the default namespace
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
Expand Down
11 changes: 9 additions & 2 deletions drivers/ipvlan/ipvlan_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
Expand Down
66 changes: 45 additions & 21 deletions drivers/macvlan/macvlan_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 types.InternalMaskableErrorf("restoring existing network %s", config.ID)
}

// update persistent db, rollback on fail
err = d.storeUpdate(config)
if err != nil {
Expand All @@ -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)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have . config.CreatedSlaveLink = true in this else loop too? ( same as ipvlan code )? just to make sure "config.CreatedSlaveLink is set to True.

}
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",
Expand All @@ -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 !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)
}
// 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,
if !foundExisting {
n := &network{
id: config.ID,
driver: d,
endpoints: endpointTable{},
config: config,
}
// add the network
d.addNetwork(n)
}
// add the *network
d.addNetwork(n)

return nil
return foundExisting, nil
}

// DeleteNetwork deletes the network for the specified driver type
Expand Down
18 changes: 18 additions & 0 deletions drivers/macvlan/macvlan_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ func parentExists(ifaceStr string) bool {
return true
}

// vlanLinkExists checks if specified vlan link exists in the default namespace
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, ".") {
Expand Down Expand Up @@ -160,6 +169,15 @@ func parseVlan(linkName string) (string, int, error) {
return parent, vidInt, nil
}

// dummyLinkExists checks if dummylink exists in the default namespace
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
Expand Down
12 changes: 10 additions & 2 deletions drivers/macvlan/macvlan_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
Expand Down