Modify hash to be computed using spew library so that nested object v…

…alues are used instead of pointer
duda · Jan 5, 2015 · f92f830 · f92f830
1 parent 943ed6d
commit f92f830
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 3 deletions.
diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go
@@ -540,7 +540,7 @@ const containerNamePrefix = "k8s"
 
 func HashContainer(container *api.Container) uint64 {
 	hash := adler32.New()
-	fmt.Fprintf(hash, "%#v", *container)
+	util.DeepHashObject(hash, *container)
 	return uint64(hash.Sum32())
 }
 

diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/credentialprovider"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	docker "github.com/fsouza/go-dockerclient"
 )
 
@@ -87,8 +88,7 @@ func TestGetContainerID(t *testing.T) {
 func verifyPackUnpack(t *testing.T, podNamespace, manifestUUID, podName, containerName string) {
 	container := &api.Container{Name: containerName}
 	hasher := adler32.New()
-	data := fmt.Sprintf("%#v", *container)
-	hasher.Write([]byte(data))
+	util.DeepHashObject(hasher, *container)
 	computedHash := uint64(hasher.Sum32())
 	podFullName := fmt.Sprintf("%s.%s", podName, podNamespace)
 	name := BuildDockerName(manifestUUID, podFullName, container)

diff --git a/pkg/util/hash.go b/pkg/util/hash.go
@@ -0,0 +1,30 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"hash"
+
+	"github.com/davecgh/go-spew/spew"
+)
+
+// DeepHashObject writes specified object to hash using the spew library
+// which follows pointers and prints actual values of the nested objects
+// ensuring the hash does not change when a pointer changes.
+func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
+	spew.Fprintf(hasher, "%#v", objectToWrite)
+}
diff --git a/pkg/util/hash_test.go b/pkg/util/hash_test.go
@@ -0,0 +1,63 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"hash/adler32"
+	"testing"
+)
+
+type wheel struct {
+	radius uint32
+}
+
+type unicycle struct {
+	primaryWheel   *wheel
+	licencePlateID string
+}
+
+func TestDeepObjectPointer(t *testing.T) {
+	// Arrange
+	wheel1 := wheel{radius: 17}
+	wheel2 := wheel{radius: 22}
+	wheel3 := wheel{radius: 17}
+
+	myUni1 := unicycle{licencePlateID: "blah", primaryWheel: &wheel1}
+	myUni2 := unicycle{licencePlateID: "blah", primaryWheel: &wheel2}
+	myUni3 := unicycle{licencePlateID: "blah", primaryWheel: &wheel3}
+
+	hasher1 := adler32.New()
+	hasher2 := adler32.New()
+	hasher3 := adler32.New()
+
+	// Act
+	DeepHashObject(hasher1, myUni1)
+	hash1 := hasher1.Sum32()
+	DeepHashObject(hasher2, myUni2)
+	hash2 := hasher2.Sum32()
+	DeepHashObject(hasher3, myUni3)
+	hash3 := hasher3.Sum32()
+
+	// Assert
+	if hash1 == hash2 {
+		t.Errorf("hash1 (%d) and hash2(%d) must be different because they have different values for wheel size", hash1, hash2)
+	}
+
+	if hash1 != hash3 {
+		t.Errorf("hash1 (%d) and hash3(%d) must be the same because although they point to different objects, they have the same values for wheel size", hash1, hash3)
+	}
+}