From c058cb1a398911d6363e3a421fb1cd67df2458ce Mon Sep 17 00:00:00 2001 From: Ivan Mikushin Date: Fri, 15 May 2015 12:16:15 +0500 Subject: [PATCH 1/4] container_test.go: fix compile errors --- docker/container_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/container_test.go b/docker/container_test.go index 71c3fcfc..c75705be 100644 --- a/docker/container_test.go +++ b/docker/container_test.go @@ -57,9 +57,9 @@ func TestParse(t *testing.T) { assert.Equal(c.Name, "c1", "Name doesn't match") assert.True(c.remove, "Remove doesn't match") assert.True(c.detach, "Detach doesn't match") - assert.Equal(len(c.Config.Cmd), 2, "Args doesn't match") - assert.Equal(c.Config.Cmd[0], "arg1", "Arg1 doesn't match") - assert.Equal(c.Config.Cmd[1], "arg2", "Arg2 doesn't match") + assert.Equal(c.Config.Cmd.Len(), 2, "Args doesn't match") + assert.Equal(c.Config.Cmd.Slice()[0], "arg1", "Arg1 doesn't match") + assert.Equal(c.Config.Cmd.Slice()[1], "arg2", "Arg2 doesn't match") assert.True(c.HostConfig.Privileged, "Privileged doesn't match") } From 2547db84e511eb71c0a78b7bb6d41aee9fc8d4b8 Mon Sep 17 00:00:00 2001 From: Ivan Mikushin Date: Sun, 17 May 2015 17:49:04 +0500 Subject: [PATCH 2/4] test getHash() consistency --- docker/container.go | 31 ++++++++++--------------------- docker/container_test.go | 30 ++++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/docker/container.go b/docker/container.go index 2636cb01..ae405a43 100644 --- a/docker/container.go +++ b/docker/container.go @@ -41,12 +41,11 @@ func (c ByCreated) Len() int { return len(c) } func (c ByCreated) Swap(i, j int) { c[i], c[j] = c[j], c[i] } func (c ByCreated) Less(i, j int) bool { return c[j].Created < c[i].Created } -func getHash(containerCfg *config.ContainerConfig) (string, error) { +func getHash(containerCfg *config.ContainerConfig) string { hash := sha1.New() - w := util.NewErrorWriter(hash) - w.Write([]byte(containerCfg.Id)) - w.Write([]byte(containerCfg.Cmd)) + hash.Write([]byte(containerCfg.Id)) + hash.Write([]byte(containerCfg.Cmd)) if containerCfg.Service != nil { //Get values of Service through reflection val := reflect.ValueOf(containerCfg.Service).Elem() @@ -76,7 +75,7 @@ func getHash(containerCfg *config.ContainerConfig) (string, error) { switch s := serviceValue.(type) { default: - w.Write([]byte(fmt.Sprintf("%v", serviceValue))) + hash.Write([]byte(fmt.Sprintf("%v", serviceValue))) case *project.SliceorMap: for lkey := range s.MapParts() { if lkey != "io.rancher.os.hash" { @@ -86,31 +85,27 @@ func getHash(containerCfg *config.ContainerConfig) (string, error) { sort.Strings(sliceKeys) for j := 0; j < len(sliceKeys); j++ { - w.Write([]byte(fmt.Sprintf("%s=%v", sliceKeys[j], s.MapParts()[sliceKeys[j]]))) + hash.Write([]byte(fmt.Sprintf("%s=%v", sliceKeys[j], s.MapParts()[sliceKeys[j]]))) } case *project.Stringorslice: sliceKeys = s.Slice() sort.Strings(sliceKeys) for j := 0; j < len(sliceKeys); j++ { - w.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) + hash.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) } case []string: sliceKeys = s sort.Strings(sliceKeys) for j := 0; j < len(sliceKeys); j++ { - w.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) + hash.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) } } } } - if w.Err != nil { - return "", w.Err - } - - return hex.EncodeToString(hash.Sum([]byte{})), nil + return hex.EncodeToString(hash.Sum([]byte{})) } func StartAndWait(dockerHost string, containerCfg *config.ContainerConfig) error { @@ -171,10 +166,7 @@ func (c *Container) Lookup() *Container { return c } - hash, err := getHash(c.ContainerCfg) - if err != nil { - return c.returnErr(err) - } + hash := getHash(c.ContainerCfg) client, err := NewClient(c.dockerHost) if err != nil { @@ -486,10 +478,7 @@ func (c *Container) getCreateOpts(client *dockerClient.Client) (*dockerClient.Cr opts.Config.Labels = make(map[string]string) } - hash, err := getHash(c.ContainerCfg) - if err != nil { - return nil, err - } + hash := getHash(c.ContainerCfg) opts.Config.Labels[config.HASH] = hash opts.Config.Labels[config.ID] = c.ContainerCfg.Id diff --git a/docker/container_test.go b/docker/container_test.go index c75705be..313115ad 100644 --- a/docker/container_test.go +++ b/docker/container_test.go @@ -5,31 +5,30 @@ import ( "testing" "github.com/rancherio/os/config" + "github.com/rancherio/rancher-compose/librcompose/project" "github.com/stretchr/testify/require" dockerClient "github.com/fsouza/go-dockerclient" + "github.com/Sirupsen/logrus" ) func TestHash(t *testing.T) { assert := require.New(t) - hash, err := getHash(&config.ContainerConfig{ + hash := getHash(&config.ContainerConfig{ Id: "id", Cmd: "1 2 3", }) - assert.NoError(err, "") - hash2, err := getHash(&config.ContainerConfig{ + hash2 := getHash(&config.ContainerConfig{ Id: "id2", Cmd: "1 2 3", }) - assert.NoError(err, "") - hash3, err := getHash(&config.ContainerConfig{ + hash3 := getHash(&config.ContainerConfig{ Id: "id3", Cmd: "1 2 3 4", }) - assert.NoError(err, "") assert.Equal("510b68938cba936876588b0143093a5850d4a142", hash, "") assert.NotEqual(hash, hash2, "") @@ -37,6 +36,25 @@ func TestHash(t *testing.T) { assert.NotEqual(hash, hash3, "") } +func TestHash2(t *testing.T) { + assert := require.New(t) + + cfg := &config.ContainerConfig{ + Id: "docker-volumes", + Cmd: "", + MigrateVolumes: false, + ReloadConfig: false, + CreateOnly: true, + Service: &project.ServiceConfig{CapAdd:[]string(nil), CapDrop:[]string(nil), CpuShares:0, Command:"", Detach:"", Dns:project.NewStringorslice(), DnsSearch:project.NewStringorslice(), DomainName:"", Entrypoint:"", EnvFile:"", Environment:project.NewMaporslice([]string{}), Hostname:"", Image:"state", Labels:project.NewSliceorMap(map[string]string{"io.rancher.os.createonly":"true", "io.rancher.os.scope":"system"}), Links:[]string(nil), LogDriver:"json-file", MemLimit:0, Name:"", Net:"none", Pid:"", Ipc:"", Ports:[]string(nil), Privileged:true, Restart:"", ReadOnly:true, StdinOpen:false, Tty:false, User:"", Volumes:[]string{"/var/lib/docker:/var/lib/docker", "/var/lib/rancher/conf:/var/lib/rancher/conf", "/var/lib/system-docker:/var/lib/system-docker"}, VolumesFrom:[]string(nil), WorkingDir:"", Expose:[]string(nil), ExternalLinks:[]string(nil)}, + } + + for i := 0; i < 10000; i++ { + logrus.Infoln(i) + assert.Equal(getHash(cfg), getHash(cfg), "") + } +} + + func TestParse(t *testing.T) { assert := require.New(t) From 59029a49a80785d78d6790254365da7728607fe4 Mon Sep 17 00:00:00 2001 From: Ivan Mikushin Date: Sun, 17 May 2015 20:39:31 +0500 Subject: [PATCH 3/4] fix getHash() --- docker/container.go | 52 +++++++++++++++++++++++++--------------- docker/container_test.go | 16 ++++++++----- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/docker/container.go b/docker/container.go index ae405a43..8ded7992 100644 --- a/docker/container.go +++ b/docker/container.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "reflect" "sort" @@ -44,14 +45,18 @@ func (c ByCreated) Less(i, j int) bool { return c[j].Created < c[i].Created } func getHash(containerCfg *config.ContainerConfig) string { hash := sha1.New() - hash.Write([]byte(containerCfg.Id)) - hash.Write([]byte(containerCfg.Cmd)) + io.WriteString(hash, fmt.Sprintln(containerCfg.Id)) + io.WriteString(hash, fmt.Sprintln(containerCfg.Cmd)) + io.WriteString(hash, fmt.Sprintln(containerCfg.MigrateVolumes)) + io.WriteString(hash, fmt.Sprintln(containerCfg.ReloadConfig)) + io.WriteString(hash, fmt.Sprintln(containerCfg.CreateOnly)) + if containerCfg.Service != nil { //Get values of Service through reflection val := reflect.ValueOf(containerCfg.Service).Elem() //Create slice to sort the keys in Service Config, which allow constant hash ordering - var serviceKeys []string + serviceKeys := []string{} //Create a data structure of map of values keyed by a string unsortedKeyValue := make(map[string]interface{}) @@ -69,14 +74,14 @@ func getHash(containerCfg *config.ContainerConfig) string { sort.Strings(serviceKeys) //Go through keys and write hash - for i := 0; i < len(serviceKeys); i++ { - serviceValue := unsortedKeyValue[serviceKeys[i]] - sliceKeys := []string{} + for _, serviceKey := range serviceKeys { + serviceValue := unsortedKeyValue[serviceKey] + + io.WriteString(hash, fmt.Sprintf("\n %v: ", serviceKey)) switch s := serviceValue.(type) { - default: - hash.Write([]byte(fmt.Sprintf("%v", serviceValue))) - case *project.SliceorMap: + case project.SliceorMap: + sliceKeys := []string{} for lkey := range s.MapParts() { if lkey != "io.rancher.os.hash" { sliceKeys = append(sliceKeys, lkey) @@ -84,28 +89,37 @@ func getHash(containerCfg *config.ContainerConfig) string { } sort.Strings(sliceKeys) - for j := 0; j < len(sliceKeys); j++ { - hash.Write([]byte(fmt.Sprintf("%s=%v", sliceKeys[j], s.MapParts()[sliceKeys[j]]))) + for _, sliceKey := range sliceKeys { + io.WriteString(hash, fmt.Sprintf("%s=%v, ", sliceKey, s.MapParts()[sliceKey])) } - case *project.Stringorslice: - sliceKeys = s.Slice() + case project.Maporslice: + sliceKeys := s.Slice() sort.Strings(sliceKeys) - for j := 0; j < len(sliceKeys); j++ { - hash.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) + for _, sliceKey := range sliceKeys { + io.WriteString(hash, fmt.Sprintf("%s, ", sliceKey)) + } + case project.Stringorslice: + sliceKeys := s.Slice() + sort.Strings(sliceKeys) + + for _, sliceKey := range sliceKeys { + io.WriteString(hash, fmt.Sprintf("%s, ", sliceKey)) } case []string: - sliceKeys = s + sliceKeys := s sort.Strings(sliceKeys) - for j := 0; j < len(sliceKeys); j++ { - hash.Write([]byte(fmt.Sprintf("%s", sliceKeys[j]))) + for _, sliceKey := range sliceKeys { + io.WriteString(hash, fmt.Sprintf("%s, ", sliceKey)) } + default: + io.WriteString(hash, fmt.Sprintf("%v", serviceValue)) } } } - return hex.EncodeToString(hash.Sum([]byte{})) + return hex.EncodeToString(hash.Sum(nil)) } func StartAndWait(dockerHost string, containerCfg *config.ContainerConfig) error { diff --git a/docker/container_test.go b/docker/container_test.go index 313115ad..382d8340 100644 --- a/docker/container_test.go +++ b/docker/container_test.go @@ -1,6 +1,7 @@ package docker import ( + "fmt" "strings" "testing" @@ -9,7 +10,6 @@ import ( "github.com/stretchr/testify/require" dockerClient "github.com/fsouza/go-dockerclient" - "github.com/Sirupsen/logrus" ) func TestHash(t *testing.T) { @@ -30,7 +30,7 @@ func TestHash(t *testing.T) { Cmd: "1 2 3 4", }) - assert.Equal("510b68938cba936876588b0143093a5850d4a142", hash, "") + assert.Equal("d601444333c7fb4cb955bcca36c5ed59b6fa8c3f", hash, "") assert.NotEqual(hash, hash2, "") assert.NotEqual(hash2, hash3, "") assert.NotEqual(hash, hash3, "") @@ -45,15 +45,19 @@ func TestHash2(t *testing.T) { MigrateVolumes: false, ReloadConfig: false, CreateOnly: true, - Service: &project.ServiceConfig{CapAdd:[]string(nil), CapDrop:[]string(nil), CpuShares:0, Command:"", Detach:"", Dns:project.NewStringorslice(), DnsSearch:project.NewStringorslice(), DomainName:"", Entrypoint:"", EnvFile:"", Environment:project.NewMaporslice([]string{}), Hostname:"", Image:"state", Labels:project.NewSliceorMap(map[string]string{"io.rancher.os.createonly":"true", "io.rancher.os.scope":"system"}), Links:[]string(nil), LogDriver:"json-file", MemLimit:0, Name:"", Net:"none", Pid:"", Ipc:"", Ports:[]string(nil), Privileged:true, Restart:"", ReadOnly:true, StdinOpen:false, Tty:false, User:"", Volumes:[]string{"/var/lib/docker:/var/lib/docker", "/var/lib/rancher/conf:/var/lib/rancher/conf", "/var/lib/system-docker:/var/lib/system-docker"}, VolumesFrom:[]string(nil), WorkingDir:"", Expose:[]string(nil), ExternalLinks:[]string(nil)}, + Service: &project.ServiceConfig{CapAdd:nil, CapDrop:nil, CpuShares:0, Command:"", Detach:"", Dns:project.NewStringorslice(), DnsSearch:project.NewStringorslice(), DomainName:"", Entrypoint:"", EnvFile:"", Environment:project.NewMaporslice([]string{}), Hostname:"", Image:"state", Labels:project.NewSliceorMap(map[string]string{"io.rancher.os.createonly":"true", "io.rancher.os.scope":"system"}), Links:nil, LogDriver:"json-file", MemLimit:0, Name:"", Net:"none", Pid:"", Ipc:"", Ports:nil, Privileged:true, Restart:"", ReadOnly:true, StdinOpen:false, Tty:false, User:"", Volumes:[]string{"/var/lib/docker:/var/lib/docker", "/var/lib/rancher/conf:/var/lib/rancher/conf", "/var/lib/system-docker:/var/lib/system-docker"}, VolumesFrom:nil, WorkingDir:"", Expose:nil, ExternalLinks:nil}, } - for i := 0; i < 10000; i++ { - logrus.Infoln(i) - assert.Equal(getHash(cfg), getHash(cfg), "") + for i := 0; i < 1000; i++ { + assert.Equal(getHash(cfg), getHash(cfg), fmt.Sprintf("Failed at iteration: %v", i)) } } +func TestBool2String(t *testing.T) { + assert := require.New(t) + assert.Equal("true", fmt.Sprint(true), "") +} + func TestParse(t *testing.T) { assert := require.New(t) From d0476e1308ad13359c3bd57965d42e462eee31eb Mon Sep 17 00:00:00 2001 From: Ivan Mikushin Date: Mon, 18 May 2015 22:10:38 +0500 Subject: [PATCH 4/4] do not sort Maporslice keys --- docker/container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/container.go b/docker/container.go index 8ded7992..1813b3fd 100644 --- a/docker/container.go +++ b/docker/container.go @@ -94,7 +94,7 @@ func getHash(containerCfg *config.ContainerConfig) string { } case project.Maporslice: sliceKeys := s.Slice() - sort.Strings(sliceKeys) + // do not sort environment keys as the order matters for _, sliceKey := range sliceKeys { io.WriteString(hash, fmt.Sprintf("%s, ", sliceKey))