Add better OS detection

Adds better detection of the OS type of the engine by getting the
supported platforms defined in the manifest and creating an ostype
constraint using them.

Signed-off-by: Drew Erny <drew.erny@docker.com>

api/mockclient/mock.go

@@ -457,3 +457,8 @@ func (client *MockClient) VolumesPrune(ctx context.Context, pruneFilter filters.
 	args := client.Mock.Called(ctx, pruneFilter)
 	return args.Get(0).(types.VolumesPruneReport), args.Error(1)
 }
+
+func (client *MockClient) DistributionInspect(ctx context.Context, image, authConfig string) (registry.DistributionInspect, error) {
+	args := client.Mock.Called(ctx, image, authConfig)
+	return args.Get(0).(registry.DistributionInspect), args.Error(1)
+}

api/nopclient/nop.go

@@ -395,3 +395,7 @@ func (client *NopClient) VolumeRemove(ctx context.Context, volumeID string, forc
 func (client *NopClient) VolumesPrune(ctx context.Context, pruneFilter filters.Args) (types.VolumesPruneReport, error) {
 	return types.VolumesPruneReport{}, errNoEngine
 }
+
+func (client *NopClient) DistributionInspect(_ context.Context, _, _ string) (registry.DistributionInspect, error) {
+	return registry.DistributionInspect{}, nil
+}

cluster/engine.go

@@ -29,6 +29,7 @@ import (
 	engineapi "github.com/docker/docker/client"
 	engineapinop "github.com/docker/swarm/api/nopclient"
 	"github.com/docker/swarm/swarmclient"
+	"github.com/opencontainers/image-spec/specs-go/v1"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -1668,3 +1669,30 @@ func (e *Engine) RefreshEngine(hostname string) error {
 	}
 	return e.RefreshContainers(true)
 }
+
+// GetImagePlatforms calls the DistributionInspect method on the underlying
+// engine and returns the valid platforms for the image.
+func (e *Engine) GetImagePlatforms(config *ContainerConfig, authConfig *types.AuthConfig) ([]v1.Platform, error) {
+	if config == nil {
+		return nil, fmt.Errorf("nil container config")
+	}
+
+	img := config.Image
+	// now, encode registry auth. if the authConfig is nil, this will just
+	// return EmptyString, which is a valid value.
+	encodedAuth, err := encodeAuthToBase64(authConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	// make the call to DistributionInspect for this image
+	result, err := e.apiClient.DistributionInspect(context.Background(), img, encodedAuth)
+
+	if err != nil {
+		return nil, err
+	}
+
+	// result is a type called "DistributionInspect", but we just need one part
+	// of it, the Platforms.
+	return result.Platforms, nil
+}

cluster/swarm/cluster.go

@@ -138,6 +138,11 @@ func (c *Cluster) StartContainer(container *cluster.Container) error {
 
 // CreateContainer aka schedule a brand new container into the cluster.
 func (c *Cluster) CreateContainer(config *cluster.ContainerConfig, name string, authConfig *types.AuthConfig) (*cluster.Container, error) {
+	// engines newer than api version 1.30 have a /distribution/{name:.*}/json
+	// endpoint, which can be used to contact a registry and determine the
+	// image platforms. before starting a container, fill in the constraint.
+	c.setOSTypeConstraint(config, authConfig)
+
 	container, err := c.createContainer(config, name, false, authConfig)
 
 	if err != nil {
@@ -177,6 +182,74 @@ func (c *Cluster) CreateContainer(config *cluster.ContainerConfig, name string,
 	return container, err
 }
 
+// setOSTypeConstraint chooses an engine and leverages its /distribution
+// endpoint to determine a list of compatible Platforms for the image. it then
+// adds an ostype constraint for the valid OS types. If an ostype constraint
+// already exists on container, then this will not overwrite it.
+func (c *Cluster) setOSTypeConstraint(config *cluster.ContainerConfig, authConfig *types.AuthConfig) error {
+	// first, check if there is an existing ostype constraint. if so, leave this
+	// alone and take no action.
+	constraints := config.Constraints()
+	for _, constraint := range constraints {
+		if strings.Contains(constraint, "ostype") {
+			return nil
+		}
+	}
+
+	// now that we know we have to set an os constraint, choose an engine at
+	// random. any engine should theoretically be able to contact the registry
+	engine, err := c.RANDOMENGINE()
+	if err != nil {
+		return err
+	}
+
+	// now call the corresponding method on this engine
+	platforms, err := engine.GetImagePlatforms(config, authConfig)
+	if err != nil {
+		return err
+	}
+
+	// now extract the OSes. use a map to deduplicate, in case the image is
+	// available on several architectures with the same platform.
+	ostypes := map[string]struct{}{}
+	for _, p := range platforms {
+		ostypes[p.OS] = struct{}{}
+	}
+
+	// if there is only one OS type, then we can just set a constraint. if the
+	// image is available on more OS types, then we need to build a regex for
+	// the constraint, which is more complicated. if for some reason there are
+	// 0 valid OS types, then just return without doing anything.
+	if len(ostypes) == 1 {
+		// iterate, which is how we get a map key that we don't already know
+		var ostype string
+		for os := range ostypes {
+			ostype = os
+		}
+		config.AddConstraint("ostype==" + ostype)
+	} else if len(ostypes) > 1 {
+		// first, turn the map into a slice of parenthesized strings. strictly
+		// speaking, we don't HAVE to put parentheses, but it's better to be
+		// explicity than to rely on regex order of operations
+		var osStrings []string
+		for os := range ostypes {
+			osStrings = append(osStrings, fmt.Sprintf("(%s)", os))
+		}
+
+		// then, string join the resulting slice with |
+		osStringsJoined := strings.Join(osStrings, "|")
+
+		// and finally, pack it in between / characters to denote that it's a
+		// regular expression
+		osRegex := fmt.Sprintf("/%s/", osStringsJoined)
+
+		// and add it as a constraint
+		config.AddConstraint("ostype==" + osRegex)
+	}
+
+	return nil
+}
+
 func (c *Cluster) createContainer(config *cluster.ContainerConfig, name string, withImageAffinity bool, authConfig *types.AuthConfig) (*cluster.Container, error) {
 	c.scheduler.Lock()
 

cluster/swarm/cluster_test.go

@@ -4,16 +4,22 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
 	networktypes "github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/volume"
 	engineapimock "github.com/docker/swarm/api/mockclient"
 	"github.com/docker/swarm/cluster"
+	"github.com/docker/swarm/scheduler"
+	"github.com/docker/swarm/scheduler/filter"
+	"github.com/docker/swarm/scheduler/strategy"
+	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
@@ -268,3 +274,162 @@ func TestTagImage(t *testing.T) {
 	assert.Nil(t, c.TagImage("busybox", "test_busybox:latest", false))
 	assert.NotNil(t, c.TagImage("busybox_not_exists", "test_busybox:latest", false))
 }
+
+func TestSetOsTypeConstraint(t *testing.T) {
+	c := &Cluster{
+		engines: make(map[string]*cluster.Engine),
+	}
+
+	// because setOSTypeConstraint uses RANDOMENGINE, we need to initialize a
+	// scheduler. it doesn't actually DO anything, but it cannot be nil. and to
+	// initialize a scheduler, we first need to initialize a strategy and a
+	// filter.
+	strat, err := strategy.New("binpack")
+	assert.Nil(t, err)
+	filters, err := filter.New([]string{})
+	assert.Nil(t, err)
+
+	sched := scheduler.New(strat, filters)
+	c.scheduler = sched
+
+	e := createEngine(t, "test-engine")
+	c.engines[e.ID] = e
+
+	containerConfig := containertypes.Config{
+		Image: "fooImage",
+	}
+
+	t.Run("NoPlatforms", func(t *testing.T) {
+		// call cluster.BuildContainer for each subtest so we don't mutate the
+		// master object
+		config := cluster.BuildContainerConfig(containerConfig, containertypes.HostConfig{}, networktypes.NetworkingConfig{})
+
+		// NoPlatforms tests that if no platforms are present, then no filter
+		// is set
+		apiClient := mockClientWithInit()
+		apiClient.On(
+			"DistributionInspect", mock.Anything, "fooImage", mock.Anything,
+		).Return(registry.DistributionInspect{Platforms: []v1.Platform{}}, nil)
+
+		// set the engine we created to use the mock client
+		e.ConnectWithClient(apiClient)
+
+		// and then try doing setOSTypeConstraint
+		err := c.setOSTypeConstraint(config, nil)
+		assert.Nil(t, err)
+
+		c, ok := getOSTypeConstraint(config)
+		assert.False(t, ok, "expected no ostype constraint but got one with value %q", c)
+	})
+
+	t.Run("OnePlatform", func(t *testing.T) {
+		config := cluster.BuildContainerConfig(containerConfig, containertypes.HostConfig{}, networktypes.NetworkingConfig{})
+
+		apiClient := mockClientWithInit()
+		apiClient.On(
+			"DistributionInspect", mock.Anything, "fooImage", mock.Anything,
+		).Return(
+			registry.DistributionInspect{
+				Platforms: []v1.Platform{
+					{OS: "windows"},
+				},
+			}, nil,
+		)
+
+		e.ConnectWithClient(apiClient)
+
+		err := c.setOSTypeConstraint(config, nil)
+		assert.Nil(t, err)
+
+		c, ok := getOSTypeConstraint(config)
+		assert.True(t, ok, "expected ostype constraint, but none present")
+		assert.Equal(t, c, "windows")
+	})
+
+	t.Run("TwoPlatforms", func(t *testing.T) {
+		config := cluster.BuildContainerConfig(containerConfig, containertypes.HostConfig{}, networktypes.NetworkingConfig{})
+
+		apiClient := mockClientWithInit()
+		apiClient.On(
+			"DistributionInspect", mock.Anything, "fooImage", mock.Anything,
+		).Return(
+			registry.DistributionInspect{
+				Platforms: []v1.Platform{
+					{OS: "linux"},
+					{OS: "windows"},
+				},
+			}, nil,
+		)
+
+		e.ConnectWithClient(apiClient)
+
+		err := c.setOSTypeConstraint(config, nil)
+		assert.Nil(t, err)
+
+		c, ok := getOSTypeConstraint(config)
+		assert.True(t, ok, "expected ostype constraint, but none present")
+		// the order will be random, but it should be one of these two
+		assert.True(t,
+			c == "/(linux)|(windows)/" || c == "/(windows)|(linux)/",
+			"expected linux and windows, but got %q", c,
+		)
+	})
+
+	t.Run("DuplicatePlatforms", func(t *testing.T) {
+		config := cluster.BuildContainerConfig(containerConfig, containertypes.HostConfig{}, networktypes.NetworkingConfig{})
+
+		apiClient := mockClientWithInit()
+		apiClient.On(
+			"DistributionInspect", mock.Anything, "fooImage", mock.Anything,
+		).Return(
+			registry.DistributionInspect{
+				Platforms: []v1.Platform{
+					{OS: "linux"},
+					{OS: "linux"},
+				},
+			}, nil,
+		)
+
+		e.ConnectWithClient(apiClient)
+
+		err := c.setOSTypeConstraint(config, nil)
+		assert.Nil(t, err)
+
+		c, ok := getOSTypeConstraint(config)
+		assert.True(t, ok, "expected ostype constraint, but none present")
+		// the order will be random, but it should be one of these two
+		assert.Equal(t, c, "linux")
+	})
+}
+
+// getOSTypeConstraint is a helper function that retrieves and returns the
+// value of the ostype constraint on the config. it additionally returns true
+// if any constraint existed, and false if none did.
+func getOSTypeConstraint(config *cluster.ContainerConfig) (string, bool) {
+	constraints := config.Constraints()
+	for _, constraint := range constraints {
+		if strings.Contains(constraint, "ostype==") {
+			return strings.TrimPrefix(constraint, "ostype=="), true
+		}
+	}
+
+	return "", false
+}
+
+// mockClientWithInit creates a mock engine API client with the necessary
+// methods for initializing the connection already filled in
+func mockClientWithInit() *engineapimock.MockClient {
+	apiClient := engineapimock.NewMockClient()
+	apiClient.On("Info", mock.Anything).Return(mockInfo, nil)
+	apiClient.On("ServerVersion", mock.Anything).Return(mockVersion, nil)
+	apiClient.On("NetworkList", mock.Anything,
+		mock.AnythingOfType("NetworkListOptions"),
+	).Return([]types.NetworkResource{}, nil)
+	apiClient.On("VolumeList", mock.Anything, mock.Anything).Return(volume.VolumeListOKBody{}, nil)
+	apiClient.On("Events", mock.Anything, mock.AnythingOfType("EventsOptions")).Return(make(chan events.Message), make(chan error))
+	apiClient.On("ImageList", mock.Anything, mock.AnythingOfType("ImageListOptions")).Return([]types.ImageSummary{}, nil)
+	apiClient.On("ContainerList", mock.Anything, types.ContainerListOptions{All: true, Size: false}).Return([]types.Container{}, nil).Once()
+	apiClient.On("NegotiateAPIVersion", mock.Anything).Return()
+
+	return apiClient
+}

swarmclient/interface.go

@@ -1,9 +1,9 @@
 package swarmclient
 
 import (
+	"context"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
-	"context"
 )
 
 // SwarmAPIClient contains the subset of the docker/api interface relevant to Docker Swarm
@@ -13,6 +13,7 @@ type SwarmAPIClient interface {
 	client.NetworkAPIClient
 	client.SystemAPIClient
 	client.VolumeAPIClient
+	client.DistributionAPIClient
 	ClientVersion() string
 	NegotiateAPIVersion(ctx context.Context)
 	ServerVersion(ctx context.Context) (types.Version, error)