Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Backport 17.03] Adding ipam options to ipam driver requests #2449

Closed
wants to merge 3 commits into from
Closed

[Backport 17.03] Adding ipam options to ipam driver requests #2449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14b4d5e
Adding ipam options to ipam driver requests
Nov 16, 2017
e6d0339
Libnetwork vendoring for IPAM serialization
Nov 16, 2017
b97ab63
Docker vendoring
Nov 16, 2017
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
30 changes: 16 additions & 14 deletions agent/exec/container/api_client_test.mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (_mr *_MockAPIClientRecorder) ContainerWait(arg0, arg1 interface{}) *gomock
return _mr.mock.ctrl.RecordCall(_mr.mock, "ContainerWait", arg0, arg1)
}

func (_m *MockAPIClient) ContainersPrune(_param0 context.Context, _param1 types.ContainersPruneConfig) (types.ContainersPruneReport, error) {
func (_m *MockAPIClient) ContainersPrune(_param0 context.Context, _param1 filters.Args) (types.ContainersPruneReport, error) {
ret := _m.ctrl.Call(_m, "ContainersPrune", _param0, _param1)
ret0, _ := ret[0].(types.ContainersPruneReport)
ret1, _ := ret[1].(error)
Expand Down Expand Up @@ -576,7 +576,7 @@ func (_mr *_MockAPIClientRecorder) ImageTag(arg0, arg1, arg2 interface{}) *gomoc
return _mr.mock.ctrl.RecordCall(_mr.mock, "ImageTag", arg0, arg1, arg2)
}

func (_m *MockAPIClient) ImagesPrune(_param0 context.Context, _param1 types.ImagesPruneConfig) (types.ImagesPruneReport, error) {
func (_m *MockAPIClient) ImagesPrune(_param0 context.Context, _param1 filters.Args) (types.ImagesPruneReport, error) {
ret := _m.ctrl.Call(_m, "ImagesPrune", _param0, _param1)
ret0, _ := ret[0].(types.ImagesPruneReport)
ret1, _ := ret[1].(error)
Expand Down Expand Up @@ -673,7 +673,7 @@ func (_mr *_MockAPIClientRecorder) NetworkRemove(arg0, arg1 interface{}) *gomock
return _mr.mock.ctrl.RecordCall(_mr.mock, "NetworkRemove", arg0, arg1)
}

func (_m *MockAPIClient) NetworksPrune(_param0 context.Context, _param1 types.NetworksPruneConfig) (types.NetworksPruneReport, error) {
func (_m *MockAPIClient) NetworksPrune(_param0 context.Context, _param1 filters.Args) (types.NetworksPruneReport, error) {
ret := _m.ctrl.Call(_m, "NetworksPrune", _param0, _param1)
ret0, _ := ret[0].(types.NetworksPruneReport)
ret1, _ := ret[1].(error)
Expand Down Expand Up @@ -748,14 +748,14 @@ func (_mr *_MockAPIClientRecorder) PluginCreate(arg0, arg1, arg2 interface{}) *g
return _mr.mock.ctrl.RecordCall(_mr.mock, "PluginCreate", arg0, arg1, arg2)
}

func (_m *MockAPIClient) PluginDisable(_param0 context.Context, _param1 string) error {
ret := _m.ctrl.Call(_m, "PluginDisable", _param0, _param1)
func (_m *MockAPIClient) PluginDisable(_param0 context.Context, _param1 string, _param2 types.PluginDisableOptions) error {
ret := _m.ctrl.Call(_m, "PluginDisable", _param0, _param1, _param2)
ret0, _ := ret[0].(error)
return ret0
}

func (_mr *_MockAPIClientRecorder) PluginDisable(arg0, arg1 interface{}) *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "PluginDisable", arg0, arg1)
func (_mr *_MockAPIClientRecorder) PluginDisable(arg0, arg1, arg2 interface{}) *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "PluginDisable", arg0, arg1, arg2)
}

func (_m *MockAPIClient) PluginEnable(_param0 context.Context, _param1 string, _param2 types.PluginEnableOptions) error {
Expand All @@ -780,10 +780,11 @@ func (_mr *_MockAPIClientRecorder) PluginInspectWithRaw(arg0, arg1 interface{})
return _mr.mock.ctrl.RecordCall(_mr.mock, "PluginInspectWithRaw", arg0, arg1)
}

func (_m *MockAPIClient) PluginInstall(_param0 context.Context, _param1 string, _param2 types.PluginInstallOptions) error {
func (_m *MockAPIClient) PluginInstall(_param0 context.Context, _param1 string, _param2 types.PluginInstallOptions) (io.ReadCloser, error) {
ret := _m.ctrl.Call(_m, "PluginInstall", _param0, _param1, _param2)
ret0, _ := ret[0].(error)
return ret0
ret0, _ := ret[0].(io.ReadCloser)
ret1, _ := ret[1].(error)
return ret0, ret1
}

func (_mr *_MockAPIClientRecorder) PluginInstall(arg0, arg1, arg2 interface{}) *gomock.Call {
Expand All @@ -801,10 +802,11 @@ func (_mr *_MockAPIClientRecorder) PluginList(arg0 interface{}) *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "PluginList", arg0)
}

func (_m *MockAPIClient) PluginPush(_param0 context.Context, _param1 string, _param2 string) error {
func (_m *MockAPIClient) PluginPush(_param0 context.Context, _param1 string, _param2 string) (io.ReadCloser, error) {
ret := _m.ctrl.Call(_m, "PluginPush", _param0, _param1, _param2)
ret0, _ := ret[0].(error)
return ret0
ret0, _ := ret[0].(io.ReadCloser)
ret1, _ := ret[1].(error)
return ret0, ret1
}

func (_mr *_MockAPIClientRecorder) PluginPush(arg0, arg1, arg2 interface{}) *gomock.Call {
Expand Down Expand Up @@ -1122,7 +1124,7 @@ func (_mr *_MockAPIClientRecorder) VolumeRemove(arg0, arg1, arg2 interface{}) *g
return _mr.mock.ctrl.RecordCall(_mr.mock, "VolumeRemove", arg0, arg1, arg2)
}

func (_m *MockAPIClient) VolumesPrune(_param0 context.Context, _param1 types.VolumesPruneConfig) (types.VolumesPruneReport, error) {
func (_m *MockAPIClient) VolumesPrune(_param0 context.Context, _param1 filters.Args) (types.VolumesPruneReport, error) {
ret := _m.ctrl.Call(_m, "VolumesPrune", _param0, _param1)
ret0, _ := ret[0].(types.VolumesPruneReport)
ret1, _ := ret[1].(error)
Expand Down
6 changes: 1 addition & 5 deletions agent/exec/container/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,9 @@ func (e *executor) Describe(ctx context.Context) (*api.NodeDescription, error) {
} else if typ.Capability == "networkdriver" {
plgnTyp = "Network"
}
plgnName := plgn.Name
if plgn.Tag != "" {
plgnName += ":" + plgn.Tag
}
plugins[api.PluginDescription{
Type: plgnTyp,
Name: plgnName,
Name: plgn.Name,
}] = struct{}{}
}
}
Expand Down
47 changes: 43 additions & 4 deletions manager/allocator/networkallocator/networkallocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/docker/libnetwork/driverapi"
"github.com/docker/libnetwork/drvregistry"
"github.com/docker/libnetwork/ipamapi"
"github.com/docker/libnetwork/netlabel"
"github.com/docker/swarmkit/api"
"github.com/docker/swarmkit/log"
"github.com/pkg/errors"
Expand Down Expand Up @@ -435,6 +436,7 @@ func (na *NetworkAllocator) releaseEndpoints(networks []*api.NetworkAttachment)

// allocate virtual IP for a single endpoint attachment of the service.
func (na *NetworkAllocator) allocateVIP(vip *api.Endpoint_VirtualIP) error {
var opts map[string]string
localNet := na.getNetwork(vip.NetworkID)
if localNet == nil {
return fmt.Errorf("networkallocator: could not find local network state")
Expand All @@ -461,8 +463,13 @@ func (na *NetworkAllocator) allocateVIP(vip *api.Endpoint_VirtualIP) error {
}
}

if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil {
// set ipam allocation method to serial
opts = setIPAMSerialAlloc(localNet.nw.IPAM.Driver.Options)
}

for _, poolID := range localNet.pools {
ip, _, err := ipam.RequestAddress(poolID, addr, nil)
ip, _, err := ipam.RequestAddress(poolID, addr, opts)
if err != nil && err != ipamapi.ErrNoAvailableIPs && err != ipamapi.ErrIPOutOfRange {
return errors.Wrap(err, "could not allocate VIP from IPAM")
}
Expand Down Expand Up @@ -511,6 +518,7 @@ func (na *NetworkAllocator) deallocateVIP(vip *api.Endpoint_VirtualIP) error {

// allocate the IP addresses for a single network attachment of the task.
func (na *NetworkAllocator) allocateNetworkIPs(nAttach *api.NetworkAttachment) error {
var opts map[string]string
var ip *net.IPNet

ipam, _, _, err := na.resolveIPAM(nAttach.Network)
Expand Down Expand Up @@ -542,10 +550,16 @@ func (na *NetworkAllocator) allocateNetworkIPs(nAttach *api.NetworkAttachment) e
}
}

// Set the ipam options if the network has an ipam driver.
if localNet.nw.IPAM != nil && localNet.nw.IPAM.Driver != nil {
// set ipam allocation method to serial
opts = setIPAMSerialAlloc(localNet.nw.IPAM.Driver.Options)
}

for _, poolID := range localNet.pools {
var err error

ip, _, err = ipam.RequestAddress(poolID, addr, nil)
ip, _, err = ipam.RequestAddress(poolID, addr, opts)
if err != nil && err != ipamapi.ErrNoAvailableIPs && err != ipamapi.ErrIPOutOfRange {
return errors.Wrap(err, "could not allocate IP from IPAM")
}
Expand Down Expand Up @@ -667,7 +681,7 @@ func (na *NetworkAllocator) loadDriver(name string) error {
if pg == nil {
return fmt.Errorf("plugin store is unintialized")
}
_, err := pg.Get(name, driverapi.NetworkPluginEndpointType, plugingetter.LOOKUP)
_, err := pg.Get(name, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
return err
}

Expand Down Expand Up @@ -764,7 +778,21 @@ func (na *NetworkAllocator) allocatePools(n *api.Network) (map[string]string, er
}
pools[poolIP.String()] = poolID

gwIP, _, err := ipam.RequestAddress(poolID, net.ParseIP(ic.Gateway), nil)
// This change is a backport that leverage a new allocation policy offered by the libnetwork IPAM
// This will let the ipam driver allocate IP sequentially differently from before that was the leftmost free bit of the pool.
// The objective is to reduce the occurrence of address already in use error that is due to a non synchronous handling
// of resources made by swarmkit, where the state of container is deleted from raft and
// the resources (like IP) is made available for other tasks to use immediately while the previous owner is
// still shutting down
if dOptions == nil {
dOptions = make(map[string]string)
}
dOptions[ipamapi.RequestAddressType] = netlabel.Gateway
// set ipam allocation method to serial
dOptions = setIPAMSerialAlloc(dOptions)
defer delete(dOptions, ipamapi.RequestAddressType)

gwIP, _, err := ipam.RequestAddress(poolID, net.ParseIP(ic.Gateway), dOptions)
Copy link
Contributor

Choose a reason for hiding this comment

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

@fcrisciani in the master branch, this statement seems to be inside a conditional:

if ic.Gateway != "" || gwIP == nil {
...
}

Do we need that here?

Copy link
Author

Choose a reason for hiding this comment

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

good point, have to check

Copy link
Author

Choose a reason for hiding this comment

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

Looking at it better looks like that logic with the if seems had been introduced by another commit fb74191, that is a major restructure looks like and the logic there extract gwIP.

This change as is does not change the current behavior but simply pass the additional flag to the IPAM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks for checking!

if err != nil {
// Rollback by releasing all the resources allocated so far.
releasePools(ipam, ipamConfigs[:i], pools)
Expand Down Expand Up @@ -792,3 +820,14 @@ func initializeDrivers(reg *drvregistry.DrvRegistry) error {
}
return nil
}

// setIPAMSerialAlloc sets the ipam allocation method to serial
func setIPAMSerialAlloc(opts map[string]string) map[string]string {
if opts == nil {
opts = make(map[string]string)
}
if _, ok := opts[ipamapi.AllocSerialPrefix]; !ok {
opts[ipamapi.AllocSerialPrefix] = "true"
}
return opts
}
4 changes: 4 additions & 0 deletions manager/allocator/networkallocator/networkallocator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,10 @@ func (a *mockIpam) DiscoverDelete(dType discoverapi.DiscoveryType, data interfac
return nil
}

func (a *mockIpam) IsBuiltIn() bool {
Copy link
Author

Choose a reason for hiding this comment

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

This is coming from this PR: 8afb5ec

return true
}

func TestCorrectlyPassIPAMOptions(t *testing.T) {
var err error
expectedIpamOptions := map[string]string{"network-name": "freddie"}
Expand Down
2 changes: 1 addition & 1 deletion manager/allocator/networkallocator/portallocator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ func (ps *portSpace) allocate(p *api.PortConfig) (err error) {
}

// Check out an arbitrary port from dynamic port space.
swarmPort, err := ps.dynamicPortSpace.GetID()
swarmPort, err := ps.dynamicPortSpace.GetID(true)
if err != nil {
return
}
Expand Down
2 changes: 1 addition & 1 deletion manager/controlapi/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func validateDriver(driver *api.Driver, pg plugingetter.PluginGetter, pluginType
return nil
}

p, err := pg.Get(driver.Name, pluginType, plugingetter.LOOKUP)
p, err := pg.Get(driver.Name, pluginType, plugingetter.Lookup)
if err != nil {
return grpc.Errorf(codes.InvalidArgument, "error during lookup of plugin %s", driver.Name)
}
Expand Down
6 changes: 3 additions & 3 deletions vendor.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ github.com/prometheus/common ebdfc6da46522d58825777cf1f90490a5b1ef1d8
github.com/prometheus/procfs abf152e5f3e97f2fafac028d2cc06c1feb87ffa5

github.com/docker/distribution 7230e9def796c63a4033211dc5107742d689fc1e
github.com/docker/docker 0fb0d67008157add34f1e11685e23a691db92644
github.com/docker/docker 428600108cce0a11e65ec4ebd9e439e947b55da7
github.com/docker/go-connections 34b5052da6b11e27f5f2e357b38b571ddddd3928
github.com/docker/go-events 37d35add5005832485c0225ec870121b78fcff1c
github.com/docker/go-units 954fed01cc617c55d838fa2230073f2cb17386c8
github.com/docker/libkv 9fd56606e928ff1f309808f5d5a0b7a2ef73f9a8
github.com/docker/libnetwork 3ab699ea36573d98f481d233c30c742ade737565
github.com/docker/libnetwork 878043960238db64a7e783199f688211560dd84c https://github.com/fcrisciani/libnetwork
Copy link
Author

Choose a reason for hiding this comment

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

temporary till the libnetwork change gets merged

github.com/opencontainers/runc 8e8d01d38d7b4fb0a35bf89b72bc3e18c98882d7

github.com/davecgh/go-spew 5215b55f46b2b919f50a1df0eaa5886afe4e3b3d
github.com/Microsoft/go-winio f778f05015353be65d242f3fedc18695756153bb
github.com/Sirupsen/logrus f76d643702a30fbffecdfe50831e11881c96ceb3 https://github.com/aaronlehmann/logrus
github.com/beorn7/perks/quantile 4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9
github.com/beorn7/perks 4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9
github.com/boltdb/bolt e72f08ddb5a52992c0a44c7dda9316c7333938b2
github.com/cloudflare/cfssl 7fb22c8cba7ecaf98e4082d22d65800cf45e042a
github.com/dustin/go-humanize 8929fe90cee4b2cb9deb468b51fb34eba64d1bf0
Expand Down
Loading