Merge pull request #429 from RenaudWasTaken/generic-resource

Integrated Generic resource in service create

Author: thaJeztah

cli/command/service/create.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
+	cliopts "github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/spf13/cobra"
@@ -58,6 +59,9 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command {
 	flags.Var(&opts.hosts, flagHost, "Set one or more custom host-to-IP mappings (host:ip)")
 	flags.SetAnnotation(flagHost, "version", []string{"1.25"})
 
+	flags.Var(cliopts.NewListOptsRef(&opts.resources.resGenericResources, ValidateSingleGenericResource), "generic-resource", "User defined resources")
+	flags.SetAnnotation(flagHostAdd, "version", []string{"1.32"})
+
 	flags.SetInterspersed(false)
 	return cmd
 }

cli/command/service/generic_resource_opts.go

@@ -0,0 +1,105 @@
+package service
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
+
+	"github.com/docker/docker/api/types/swarm"
+	swarmapi "github.com/docker/swarmkit/api"
+	"github.com/docker/swarmkit/api/genericresource"
+)
+
+// GenericResource is a concept that a user can use to advertise user-defined
+// resources on a node and thus better place services based on these resources.
+// E.g: NVIDIA GPUs, Intel FPGAs, ...
+// See https://github.com/docker/swarmkit/blob/master/design/generic_resources.md
+
+// ValidateSingleGenericResource validates that a single entry in the
+// generic resource list is valid.
+// i.e 'GPU=UID1' is valid however 'GPU:UID1' or 'UID1' isn't
+func ValidateSingleGenericResource(val string) (string, error) {
+	if strings.Count(val, "=") < 1 {
+		return "", fmt.Errorf("invalid generic-resource format `%s` expected `name=value`", val)
+	}
+
+	return val, nil
+}
+
+// ParseGenericResources parses an array of Generic resourceResources
+// Requesting Named Generic Resources for a service is not supported this
+// is filtered here.
+func ParseGenericResources(value []string) ([]swarm.GenericResource, error) {
+	if len(value) == 0 {
+		return nil, nil
+	}
+
+	resources, err := genericresource.Parse(value)
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid generic resource specification")
+	}
+
+	swarmResources := genericResourcesFromGRPC(resources)
+	for _, res := range swarmResources {
+		if res.NamedResourceSpec != nil {
+			return nil, fmt.Errorf("invalid generic-resource request `%s=%s`, Named Generic Resources is not supported for service create or update", res.NamedResourceSpec.Kind, res.NamedResourceSpec.Value)
+		}
+	}
+
+	return swarmResources, nil
+}
+
+// genericResourcesFromGRPC converts a GRPC GenericResource to a GenericResource
+func genericResourcesFromGRPC(genericRes []*swarmapi.GenericResource) []swarm.GenericResource {
+	var generic []swarm.GenericResource
+	for _, res := range genericRes {
+		var current swarm.GenericResource
+
+		switch r := res.Resource.(type) {
+		case *swarmapi.GenericResource_DiscreteResourceSpec:
+			current.DiscreteResourceSpec = &swarm.DiscreteGenericResource{
+				Kind:  r.DiscreteResourceSpec.Kind,
+				Value: r.DiscreteResourceSpec.Value,
+			}
+		case *swarmapi.GenericResource_NamedResourceSpec:
+			current.NamedResourceSpec = &swarm.NamedGenericResource{
+				Kind:  r.NamedResourceSpec.Kind,
+				Value: r.NamedResourceSpec.Value,
+			}
+		}
+
+		generic = append(generic, current)
+	}
+
+	return generic
+}
+
+func buildGenericResourceMap(genericRes []swarm.GenericResource) (map[string]swarm.GenericResource, error) {
+	m := make(map[string]swarm.GenericResource)
+
+	for _, res := range genericRes {
+		if res.DiscreteResourceSpec == nil {
+			return nil, fmt.Errorf("invalid generic-resource `%+v` for service task", res)
+		}
+
+		_, ok := m[res.DiscreteResourceSpec.Kind]
+		if ok {
+			return nil, fmt.Errorf("duplicate generic-resource `%+v` for service task", res.DiscreteResourceSpec.Kind)
+		}
+
+		m[res.DiscreteResourceSpec.Kind] = res
+	}
+
+	return m, nil
+}
+
+func buildGenericResourceList(genericRes map[string]swarm.GenericResource) []swarm.GenericResource {
+	var l []swarm.GenericResource
+
+	for _, res := range genericRes {
+		l = append(l, res)
+	}
+
+	return l
+}

cli/command/service/generic_resource_opts_test.go

@@ -0,0 +1,22 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateSingleGenericResource(t *testing.T) {
+	incorrect := []string{"foo", "fooo-bar"}
+	correct := []string{"foo=bar", "bar=1", "foo=barbar"}
+
+	for _, v := range incorrect {
+		_, err := ValidateSingleGenericResource(v)
+		assert.Error(t, err)
+	}
+
+	for _, v := range correct {
+		_, err := ValidateSingleGenericResource(v)
+		assert.NoError(t, err)
+	}
+}

cli/command/service/opts.go

@@ -222,23 +222,30 @@ func (opts updateOptions) rollbackConfig(flags *pflag.FlagSet) *swarm.UpdateConf
 }
 
 type resourceOptions struct {
-	limitCPU      opts.NanoCPUs
-	limitMemBytes opts.MemBytes
-	resCPU        opts.NanoCPUs
-	resMemBytes   opts.MemBytes
+	limitCPU            opts.NanoCPUs
+	limitMemBytes       opts.MemBytes
+	resCPU              opts.NanoCPUs
+	resMemBytes         opts.MemBytes
+	resGenericResources []string
 }
 
-func (r *resourceOptions) ToResourceRequirements() *swarm.ResourceRequirements {
+func (r *resourceOptions) ToResourceRequirements() (*swarm.ResourceRequirements, error) {
+	generic, err := ParseGenericResources(r.resGenericResources)
+	if err != nil {
+		return nil, err
+	}
+
 	return &swarm.ResourceRequirements{
 		Limits: &swarm.Resources{
 			NanoCPUs:    r.limitCPU.Value(),
 			MemoryBytes: r.limitMemBytes.Value(),
 		},
 		Reservations: &swarm.Resources{
-			NanoCPUs:    r.resCPU.Value(),
-			MemoryBytes: r.resMemBytes.Value(),
+			NanoCPUs:         r.resCPU.Value(),
+			MemoryBytes:      r.resMemBytes.Value(),
+			GenericResources: generic,
 		},
-	}
+	}, nil
 }
 
 type restartPolicyOptions struct {
@@ -588,6 +595,11 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N
 		return service, err
 	}
 
+	resources, err := options.resources.ToResourceRequirements()
+	if err != nil {
+		return service, err
+	}
+
 	service = swarm.ServiceSpec{
 		Annotations: swarm.Annotations{
 			Name:   options.name,
@@ -619,7 +631,7 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N
 				Isolation:       container.Isolation(options.isolation),
 			},
 			Networks:      networks,
-			Resources:     options.resources.ToResourceRequirements(),
+			Resources:     resources,
 			RestartPolicy: options.restartPolicy.ToRestartPolicy(flags),
 			Placement: &swarm.Placement{
 				Constraints: options.constraints.GetAll(),
@@ -818,6 +830,8 @@ const (
 	flagEnvFile                 = "env-file"
 	flagEnvRemove               = "env-rm"
 	flagEnvAdd                  = "env-add"
+	flagGenericResourcesRemove  = "generic-resource-rm"
+	flagGenericResourcesAdd     = "generic-resource-add"
 	flagGroup                   = "group"
 	flagGroupAdd                = "group-add"
 	flagGroupRemove             = "group-rm"

cli/command/service/opts_test.go

@@ -85,3 +85,41 @@ func TestHealthCheckOptionsToHealthConfigConflict(t *testing.T) {
 	_, err := opt.toHealthConfig()
 	assert.EqualError(t, err, "--no-healthcheck conflicts with --health-* options")
 }
+
+func TestResourceOptionsToResourceRequirements(t *testing.T) {
+	incorrectOptions := []resourceOptions{
+		{
+			resGenericResources: []string{"foo=bar", "foo=1"},
+		},
+		{
+			resGenericResources: []string{"foo=bar", "foo=baz"},
+		},
+		{
+			resGenericResources: []string{"foo=bar"},
+		},
+		{
+			resGenericResources: []string{"foo=1", "foo=2"},
+		},
+	}
+
+	for _, opt := range incorrectOptions {
+		_, err := opt.ToResourceRequirements()
+		assert.Error(t, err)
+	}
+
+	correctOptions := []resourceOptions{
+		{
+			resGenericResources: []string{"foo=1"},
+		},
+		{
+			resGenericResources: []string{"foo=1", "bar=2"},
+		},
+	}
+
+	for _, opt := range correctOptions {
+		r, err := opt.ToResourceRequirements()
+		assert.NoError(t, err)
+		assert.Len(t, r.Reservations.GenericResources, len(opt.resGenericResources))
+	}
+
+}

cli/command/service/update.go

@@ -95,13 +95,23 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command {
 	flags.Var(&options.hosts, flagHostAdd, "Add a custom host-to-IP mapping (host:ip)")
 	flags.SetAnnotation(flagHostAdd, "version", []string{"1.25"})
 
+	// Add needs parsing, Remove only needs the key
+	flags.Var(newListOptsVar(), flagGenericResourcesRemove, "Remove a Generic resource")
+	flags.SetAnnotation(flagHostAdd, "version", []string{"1.32"})
+	flags.Var(newListOptsVarWithValidator(ValidateSingleGenericResource), flagGenericResourcesAdd, "Add a Generic resource")
+	flags.SetAnnotation(flagHostAdd, "version", []string{"1.32"})
+
 	return cmd
 }
 
 func newListOptsVar() *opts.ListOpts {
 	return opts.NewListOptsRef(&[]string{}, nil)
 }
 
+func newListOptsVarWithValidator(validator opts.ValidatorFctType) *opts.ListOpts {
+	return opts.NewListOptsRef(&[]string{}, validator)
+}
+
 // nolint: gocyclo
 func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOptions, serviceID string) error {
 	apiClient := dockerCli.Client()
@@ -314,6 +324,14 @@ func updateService(ctx context.Context, apiClient client.NetworkAPIClient, flags
 		updateInt64Value(flagReserveMemory, &task.Resources.Reservations.MemoryBytes)
 	}
 
+	if err := addGenericResources(flags, task); err != nil {
+		return err
+	}
+
+	if err := removeGenericResources(flags, task); err != nil {
+		return err
+	}
+
 	updateDurationOpt(flagStopGracePeriod, &cspec.StopGracePeriod)
 
 	if anyChanged(flags, flagRestartCondition, flagRestartDelay, flagRestartMaxAttempts, flagRestartWindow) {
@@ -470,6 +488,72 @@ func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
 	return false
 }
 
+func addGenericResources(flags *pflag.FlagSet, spec *swarm.TaskSpec) error {
+	if !flags.Changed(flagGenericResourcesAdd) {
+		return nil
+	}
+
+	if spec.Resources == nil {
+		spec.Resources = &swarm.ResourceRequirements{}
+	}
+
+	if spec.Resources.Reservations == nil {
+		spec.Resources.Reservations = &swarm.Resources{}
+	}
+
+	values := flags.Lookup(flagGenericResourcesAdd).Value.(*opts.ListOpts).GetAll()
+	generic, err := ParseGenericResources(values)
+	if err != nil {
+		return err
+	}
+
+	m, err := buildGenericResourceMap(spec.Resources.Reservations.GenericResources)
+	if err != nil {
+		return err
+	}
+
+	for _, toAddRes := range generic {
+		m[toAddRes.DiscreteResourceSpec.Kind] = toAddRes
+	}
+
+	spec.Resources.Reservations.GenericResources = buildGenericResourceList(m)
+
+	return nil
+}
+
+func removeGenericResources(flags *pflag.FlagSet, spec *swarm.TaskSpec) error {
+	// Can only be Discrete Resources
+	if !flags.Changed(flagGenericResourcesRemove) {
+		return nil
+	}
+
+	if spec.Resources == nil {
+		spec.Resources = &swarm.ResourceRequirements{}
+	}
+
+	if spec.Resources.Reservations == nil {
+		spec.Resources.Reservations = &swarm.Resources{}
+	}
+
+	values := flags.Lookup(flagGenericResourcesRemove).Value.(*opts.ListOpts).GetAll()
+
+	m, err := buildGenericResourceMap(spec.Resources.Reservations.GenericResources)
+	if err != nil {
+		return err
+	}
+
+	for _, toRemoveRes := range values {
+		if _, ok := m[toRemoveRes]; !ok {
+			return fmt.Errorf("could not find generic-resource `%s` to remove it", toRemoveRes)
+		}
+
+		delete(m, toRemoveRes)
+	}
+
+	spec.Resources.Reservations.GenericResources = buildGenericResourceList(m)
+	return nil
+}
+
 func updatePlacementConstraints(flags *pflag.FlagSet, placement *swarm.Placement) {
 	if flags.Changed(flagConstraintAdd) {
 		values := flags.Lookup(flagConstraintAdd).Value.(*opts.ListOpts).GetAll()