Skip to content

Commit

Permalink
api: Centralize defaults and add InterpolatedSpec
Browse files Browse the repository at this point in the history
Currently, there is a problem with visibility of defaults. The best way
to create a service is to only specify fields that you care about,
because then the manager can apply appropriate defaults for the other
fields. However, there's no way to know which values are actually being
used for those fields. The spec is under the user's control, so if they
don't specify a value (for example, `Spec.Task.Restart.Delay`), this
field will show up as blank when the service is retrieved.

We don't want to break the spec ownership model, but we do want a way to
communicate actual values in use back to the client. For this, we add an
`InterpolatedSpec` field which is inserted into the service object by
the ControlAPI. `InterpolatedSpec` is a version of `Spec` that has
unspecified fields filled in with default values, so the client can
observe what values are actually in use.

This also has the benefit of centralizing defaults. Default values for
these fields that were defined in various places around the code are now
centralized in `github.com/docker/swarmkit/api/defaults`.

Right now InterpolatedSpec is only added to `Service`, but if we move
forward with this, it would make sense to add it to other objects as
well.

The main question about this approach is whether we want to solve the
defaults visibility problem by using this approach where defaults are
substituted in, or by encouraging the client to set all fields instead
of leaving some empty. There are advantages and disadvantages to each
approach. The benefit of doing this client-side is that changes to
default values won't affect existing services retroactively. However,
expecting the client to fill in all fields with good defaults is
expecting a lot, and would be a barrier to centralizing defaults. I
think the best approach is to apply defaults on the manager side, but be
conservative about changing defaults, knowing that it may have
unexpected side effects.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
  • Loading branch information
aaronlehmann committed Feb 24, 2017
1 parent 3f22562 commit e5274d2
Show file tree
Hide file tree
Showing 9 changed files with 292 additions and 132 deletions.
81 changes: 81 additions & 0 deletions api/defaults/service.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package defaults

import (
"time"

"github.com/docker/swarmkit/api"
"github.com/docker/swarmkit/api/deepcopy"
gogotypes "github.com/gogo/protobuf/types"
)

// Service is a ServiceSpec object with all fields filled in using default
// values.
var Service = api.ServiceSpec{
Task: api.TaskSpec{
Runtime: &api.TaskSpec_Container{
Container: &api.ContainerSpec{
StopGracePeriod: gogotypes.DurationProto(10 * time.Second),
PullOptions: &api.ContainerSpec_PullOptions{},
DNSConfig: &api.ContainerSpec_DNSConfig{},
},
},
Resources: &api.ResourceRequirements{},
Restart: &api.RestartPolicy{
Delay: gogotypes.DurationProto(5 * time.Second),
},
Placement: &api.Placement{},
},
Update: &api.UpdateConfig{
FailureAction: api.UpdateConfig_PAUSE,
Monitor: gogotypes.DurationProto(5 * time.Second),
},
}

// InterpolateService returns a ServiceSpec based on the provided spec, which
// has all unspecified values filled in with default values.
func InterpolateService(origSpec *api.ServiceSpec) *api.ServiceSpec {
spec := origSpec.Copy()

container := spec.Task.GetContainer()
defaultContainer := Service.Task.GetContainer()
if container != nil {
if container.StopGracePeriod == nil {
container.StopGracePeriod = &gogotypes.Duration{}
deepcopy.Copy(container.StopGracePeriod, defaultContainer.StopGracePeriod)
}
if container.PullOptions == nil {
container.PullOptions = defaultContainer.PullOptions.Copy()
}
if container.DNSConfig == nil {
container.DNSConfig = defaultContainer.DNSConfig.Copy()
}
}

if spec.Task.Resources == nil {
spec.Task.Resources = Service.Task.Resources.Copy()
}

if spec.Task.Restart == nil {
spec.Task.Restart = Service.Task.Restart.Copy()
} else {
if spec.Task.Restart.Delay == nil {
spec.Task.Restart.Delay = &gogotypes.Duration{}
deepcopy.Copy(spec.Task.Restart.Delay, Service.Task.Restart.Delay)
}
}

if spec.Task.Placement == nil {
spec.Task.Placement = Service.Task.Placement.Copy()
}

if spec.Update == nil {
spec.Update = Service.Update.Copy()
} else {
if spec.Update.Monitor == nil {
spec.Update.Monitor = &gogotypes.Duration{}
deepcopy.Copy(spec.Update.Monitor, Service.Update.Monitor)
}
}

return spec
}
Loading

0 comments on commit e5274d2

Please sign in to comment.