From 9a88511d00bbce17a0c40d0f74a276ebcfaa757d Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 3 Sep 2024 10:31:24 -0300 Subject: [PATCH] fix(docker): avoid specifying the MAC address of container for Docker API < v1.44 BE-10880 (#12179) --- api/docker/container.go | 57 ++++++++++++++++++++++++++++++++---- api/docker/container_test.go | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 api/docker/container_test.go diff --git a/api/docker/container.go b/api/docker/container.go index c1e0631b4..2d5c1e426 100644 --- a/api/docker/container.go +++ b/api/docker/container.go @@ -9,6 +9,7 @@ import ( dockerclient "github.com/portainer/portainer/api/docker/client" "github.com/portainer/portainer/api/docker/images" + "github.com/Masterminds/semver" "github.com/docker/docker/api/types" dockercontainer "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" @@ -30,6 +31,44 @@ func NewContainerService(factory *dockerclient.ClientFactory, dataStore dataserv } } +// applyVersionConstraint uses the version to apply a transformation function to +// the value when the constraint is satisfied +func applyVersionConstraint[T any](currentVersion, versionConstraint string, value T, transform func(T) T) (T, error) { + newValue := value + + constraint, err := semver.NewConstraint(versionConstraint) + if err != nil { + return newValue, errors.New("invalid version constraint specified") + } + + currentVer, err := semver.NewVersion(currentVersion) + if err != nil { + log.Warn().Err(err).Msg("Unable to parse the Docker client version") + + return newValue, nil + } + + if satisfiesConstraint, _ := constraint.Validate(currentVer); satisfiesConstraint { + newValue = transform(value) + } + + return newValue, nil +} + +func clearMacAddrs(n network.NetworkingConfig) network.NetworkingConfig { + netConfig := network.NetworkingConfig{ + EndpointsConfig: make(map[string]*network.EndpointSettings), + } + + for k := range n.EndpointsConfig { + endpointConfig := n.EndpointsConfig[k].Copy() + endpointConfig.MacAddress = "" + netConfig.EndpointsConfig[k] = endpointConfig + } + + return netConfig +} + // Recreate a container func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.Endpoint, containerId string, forcePullImage bool, imageTag, nodeName string) (*types.ContainerJSON, error) { cli, err := c.factory.CreateClient(endpoint, nodeName, nil) @@ -83,7 +122,7 @@ func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.End return nil, errors.Wrap(err, "rename container error") } - networkWithCreation := network.NetworkingConfig{ + initialNetwork := network.NetworkingConfig{ EndpointsConfig: make(map[string]*network.EndpointSettings), } @@ -95,10 +134,10 @@ func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.End } // 5. get the first network attached to the current container - if len(networkWithCreation.EndpointsConfig) == 0 { + if len(initialNetwork.EndpointsConfig) == 0 { // Retrieve the first network that is linked to the present container, which // will be utilized when creating the container. - networkWithCreation.EndpointsConfig[name] = network + initialNetwork.EndpointsConfig[name] = network } } c.sr.enable() @@ -124,7 +163,15 @@ func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.End // to retain the same network settings we have to connect on creation to one of the old // container's networks, and connect to the other networks after creation. // see: https://portainer.atlassian.net/browse/EE-5448 - create, err := cli.ContainerCreate(ctx, container.Config, container.HostConfig, &networkWithCreation, nil, container.Name) + + // Docker API < 1.44 does not support specifying MAC addresses + // https://github.com/moby/moby/blob/6aea26b431ea152a8b085e453da06ea403f89886/client/container_create.go#L44-L46 + initialNetwork, err = applyVersionConstraint(cli.ClientVersion(), "< 1.44", initialNetwork, clearMacAddrs) + if err != nil { + return nil, err + } + + create, err := cli.ContainerCreate(ctx, container.Config, container.HostConfig, &initialNetwork, nil, container.Name) c.sr.push(func() { log.Debug().Str("container_id", create.ID).Msg("removing the new container") @@ -144,7 +191,7 @@ func (c *ContainerService) Recreate(ctx context.Context, endpoint *portainer.End log.Debug().Str("container_id", newContainerId).Msg("connecting networks to container") networks := container.NetworkSettings.Networks for key, network := range networks { - if _, ok := networkWithCreation.EndpointsConfig[key]; ok { + if _, ok := initialNetwork.EndpointsConfig[key]; ok { // skip the network that is used during container creation continue } diff --git a/api/docker/container_test.go b/api/docker/container_test.go new file mode 100644 index 000000000..e3d9e0bd8 --- /dev/null +++ b/api/docker/container_test.go @@ -0,0 +1,52 @@ +package docker + +import ( + "testing" + + "github.com/docker/docker/api/types/network" + "github.com/stretchr/testify/require" +) + +func TestApplyVersionConstraint(t *testing.T) { + initialNet := network.NetworkingConfig{ + EndpointsConfig: map[string]*network.EndpointSettings{ + "key1": { + MacAddress: "mac1", + EndpointID: "endpointID1", + }, + "key2": { + MacAddress: "mac2", + EndpointID: "endpointID2", + }, + }, + } + + f := func(currentVer string, constraint string, success, emptyMac bool) { + t.Helper() + + transformedNet, err := applyVersionConstraint(currentVer, constraint, initialNet, clearMacAddrs) + if success { + require.NoError(t, err) + } else { + require.Error(t, err) + } + + require.Len(t, transformedNet.EndpointsConfig, len(initialNet.EndpointsConfig)) + + for k := range initialNet.EndpointsConfig { + if emptyMac { + require.NotEqual(t, initialNet.EndpointsConfig[k], transformedNet.EndpointsConfig[k]) + require.Empty(t, transformedNet.EndpointsConfig[k].MacAddress) + + continue + } + + require.Equal(t, initialNet.EndpointsConfig[k], transformedNet.EndpointsConfig[k]) + } + } + + f("1.45", "< 1.44", true, false) // No transformation + f("1.43", "< 1.44", true, true) // Transformation + f("a.b.", "< 1.44", true, false) // Invalid current version + f("1.45", "z 1.44", false, false) // Invalid version constraint +}