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.

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

Few refactoring around the docker provider #1440

Merged
merged 2 commits into from
Apr 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/containous/flaeg"
"github.com/containous/traefik/acme"
"github.com/containous/traefik/provider"
"github.com/containous/traefik/provider/docker"
"github.com/containous/traefik/types"
)

Expand Down Expand Up @@ -40,7 +41,7 @@ type GlobalConfiguration struct {
IdleTimeout flaeg.Duration `description:"maximum amount of time an idle (keep-alive) connection will remain idle before closing itself."`
InsecureSkipVerify bool `description:"Disable SSL certificate verification"`
Retry *Retry `description:"Enable retry sending request if network error"`
Docker *provider.Docker `description:"Enable Docker backend"`
Docker *docker.Provider `description:"Enable Docker backend"`
File *provider.File `description:"Enable File backend"`
Web *WebProvider `description:"Enable Web backend"`
Marathon *provider.Marathon `description:"Enable Marathon backend"`
Expand Down Expand Up @@ -328,7 +329,7 @@ type Retry struct {
// NewTraefikDefaultPointersConfiguration creates a TraefikConfiguration with pointers default values
func NewTraefikDefaultPointersConfiguration() *TraefikConfiguration {
//default Docker
var defaultDocker provider.Docker
var defaultDocker docker.Provider
defaultDocker.Watch = true
defaultDocker.ExposedByDefault = true
defaultDocker.Endpoint = "unix:///var/run/docker.sock"
Expand Down
4 changes: 2 additions & 2 deletions provider/consul_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (provider *ConsulCatalog) getBackendName(node *api.ServiceEntry, index int)
serviceName := strings.ToLower(node.Service.Service) + "--" + node.Service.Address + "--" + strconv.Itoa(node.Service.Port)

for _, tag := range node.Service.Tags {
serviceName += "--" + normalize(tag)
serviceName += "--" + Normalize(tag)
}

serviceName = strings.Replace(serviceName, ".", "-", -1)
Expand Down Expand Up @@ -246,7 +246,7 @@ func (provider *ConsulCatalog) buildConfig(catalog []catalogUpdate) *types.Confi
Nodes: allNodes,
}

configuration, err := provider.getConfiguration("templates/consul_catalog.tmpl", FuncMap, templateObjects)
configuration, err := provider.GetConfiguration("templates/consul_catalog.tmpl", FuncMap, templateObjects)
if err != nil {
log.WithError(err).Error("Failed to create config")
}
Expand Down
179 changes: 179 additions & 0 deletions provider/docker/builder_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package docker

import (
docker "github.com/docker/engine-api/types"
"github.com/docker/engine-api/types/container"
"github.com/docker/engine-api/types/network"
"github.com/docker/engine-api/types/swarm"
"github.com/docker/go-connections/nat"
)

func containerJSON(ops ...func(*docker.ContainerJSON)) docker.ContainerJSON {
c := &docker.ContainerJSON{
Copy link
Member

Choose a reason for hiding this comment

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

variable names with one letter, are you sure ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, but between c, ctnr or cont, I went lazy 👼 (didn't want to use container because it would override the container package)

ContainerJSONBase: &docker.ContainerJSONBase{
Name: "fake",
HostConfig: &container.HostConfig{},
},
Config: &container.Config{},
NetworkSettings: &docker.NetworkSettings{
NetworkSettingsBase: docker.NetworkSettingsBase{},
},
}

for _, op := range ops {
op(c)
}

return *c
Copy link
Contributor

Choose a reason for hiding this comment

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

How about returning a pointer since we are already applying the ops on the pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, we always use ContainerJSON as not a pointer so… I prered to have just on return *c than a lot of *containerJSON(…) 😝

Copy link
Contributor

Choose a reason for hiding this comment

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

My point was that we pass-by-pointer when we apply the ops only to do a return-by-value eventually. But I suppose it's fine this way.

}

func name(name string) func(*docker.ContainerJSON) {
return func(c *docker.ContainerJSON) {
c.ContainerJSONBase.Name = name
}
}

func networkMode(mode string) func(*docker.ContainerJSON) {
return func(c *docker.ContainerJSON) {
c.ContainerJSONBase.HostConfig.NetworkMode = container.NetworkMode(mode)
}
}

func labels(labels map[string]string) func(*docker.ContainerJSON) {
return func(c *docker.ContainerJSON) {
c.Config.Labels = labels
}
}

func ports(portMap nat.PortMap) func(*docker.ContainerJSON) {
return func(c *docker.ContainerJSON) {
c.NetworkSettings.NetworkSettingsBase.Ports = portMap
}
}

func withNetwork(name string, ops ...func(*network.EndpointSettings)) func(*docker.ContainerJSON) {
return func(c *docker.ContainerJSON) {
if c.NetworkSettings.Networks == nil {
c.NetworkSettings.Networks = map[string]*network.EndpointSettings{}
}
c.NetworkSettings.Networks[name] = &network.EndpointSettings{}
for _, op := range ops {
op(c.NetworkSettings.Networks[name])
}
}
}

func ipv4(ip string) func(*network.EndpointSettings) {
return func(s *network.EndpointSettings) {
s.IPAddress = ip
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm on the fence whether the builder deserves its on set if tests. It doesn't seem strictly required but would be helpful to have them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, so… I don't want to overdo it too. 😛


func swarmTask(id string, ops ...func(*swarm.Task)) swarm.Task {
task := &swarm.Task{
ID: id,
}

for _, op := range ops {
op(task)
}

return *task
}

func taskSlot(slot int) func(*swarm.Task) {
return func(task *swarm.Task) {
task.Slot = slot
}
}

func taskStatus(ops ...func(*swarm.TaskStatus)) func(*swarm.Task) {
return func(task *swarm.Task) {
status := &swarm.TaskStatus{}

for _, op := range ops {
op(status)
}

task.Status = *status
}
}

func taskState(state swarm.TaskState) func(*swarm.TaskStatus) {
return func(status *swarm.TaskStatus) {
status.State = state
}
}

func swarmService(ops ...func(*swarm.Service)) swarm.Service {
service := &swarm.Service{
ID: "serviceID",
Spec: swarm.ServiceSpec{
Annotations: swarm.Annotations{
Name: "defaultServiceName",
},
},
}

for _, op := range ops {
op(service)
}

return *service
}

func serviceName(name string) func(service *swarm.Service) {
return func(service *swarm.Service) {
service.Spec.Annotations.Name = name
}
}

func serviceLabels(labels map[string]string) func(service *swarm.Service) {
return func(service *swarm.Service) {
service.Spec.Annotations.Labels = labels
}
}

func withEndpoint(ops ...func(*swarm.Endpoint)) func(*swarm.Service) {
return func(service *swarm.Service) {
endpoint := &swarm.Endpoint{}

for _, op := range ops {
op(endpoint)
}

service.Endpoint = *endpoint
}
}

func virtualIP(networkID, addr string) func(*swarm.Endpoint) {
return func(endpoint *swarm.Endpoint) {
if endpoint.VirtualIPs == nil {
endpoint.VirtualIPs = []swarm.EndpointVirtualIP{}
}
endpoint.VirtualIPs = append(endpoint.VirtualIPs, swarm.EndpointVirtualIP{
NetworkID: networkID,
Addr: addr,
})
}
}

func withEndpointSpec(ops ...func(*swarm.EndpointSpec)) func(*swarm.Service) {
return func(service *swarm.Service) {
endpointSpec := &swarm.EndpointSpec{}

for _, op := range ops {
op(endpointSpec)
}

service.Spec.EndpointSpec = endpointSpec
}
}

func modeDNSSR(spec *swarm.EndpointSpec) {
spec.Mode = swarm.ResolutionModeDNSRR
}

func modeVIP(spec *swarm.EndpointSpec) {
spec.Mode = swarm.ResolutionModeVIP
}