Ensure Namespace and UID are set in kubelet

cmd/integration/integration.go

@@ -622,7 +622,7 @@ func main() {
 			createdPods.Insert(p[:n-8])
 		}
 	}
-	// We expect 5: 2 net containers + 2 pods from the replication controller +
+	// We expect 9: 2 net containers + 2 pods from the replication controller +
 	//              1 net container + 2 pods from the URL +
 	//              1 net container + 1 pod from the service test.
 	if len(createdPods) != 9 {

pkg/kubelet/config/config.go

@@ -24,9 +24,11 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	apierrs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/config"
+	utilerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors"
 	"github.com/golang/glog"
 )
 
@@ -293,21 +295,28 @@ func (s *podStorage) seenSources(sources ...string) bool {
 func filterInvalidPods(pods []api.BoundPod, source string) (filtered []*api.BoundPod) {
 	names := util.StringSet{}
 	for i := range pods {
-		var errors []error
-		name := podUniqueName(&pods[i])
-		if names.Has(name) {
-			errors = append(errors, apierrs.NewFieldDuplicate("name", pods[i].Name))
+		pod := &pods[i]
+		var errlist []error
+		if errs := validation.ValidateBoundPod(pod); len(errs) != 0 {
+			errlist = append(errlist, errs...)
+			// If validation fails, don't trust it any further -
+			// even Name could be bad.
 		} else {
-			names.Insert(name)
-		}
-		if errs := validation.ValidateBoundPod(&pods[i]); len(errs) != 0 {
-			errors = append(errors, errs...)
+			name := podUniqueName(pod)
+			if names.Has(name) {
+				errlist = append(errlist, apierrs.NewFieldDuplicate("name", pod.Name))
+			} else {
+				names.Insert(name)
+			}
 		}
-		if len(errors) > 0 {
-			glog.Warningf("Pod %d (%s) from %s failed validation, ignoring: %v", i+1, pods[i].Name, source, errors)
+		if len(errlist) > 0 {
+			name := bestPodIdentString(pod)
+			err := utilerrors.NewAggregate(errlist)
+			glog.Warningf("Pod[%d] (%s) from %s failed validation, ignoring: %v", i+1, name, source, err)
+			record.Eventf(pod, "", "failedValidation", "Error validating pod %s from %s, ignoring: %v", name, source, err)
 			continue
 		}
-		filtered = append(filtered, &pods[i])
+		filtered = append(filtered, pod)
 	}
 	return
 }
@@ -337,11 +346,19 @@ func (s *podStorage) MergedState() interface{} {
 }
 
 // podUniqueName returns a value for a given pod that is unique across a source,
-// which is the combination of namespace and ID.
+// which is the combination of namespace and name.
 func podUniqueName(pod *api.BoundPod) string {
+	return fmt.Sprintf("%s.%s", pod.Name, pod.Namespace)
+}
+
+func bestPodIdentString(pod *api.BoundPod) string {
 	namespace := pod.Namespace
-	if len(namespace) == 0 {
-		namespace = api.NamespaceDefault
+	if namespace == "" {
+		namespace = "<empty-namespace>"
+	}
+	name := pod.Name
+	if name == "" {
+		name = "<empty-name>"
 	}
-	return fmt.Sprintf("%s.%s", pod.Name, namespace)
+	return fmt.Sprintf("%s.%s", name, namespace)
 }

pkg/kubelet/config/config_test.go

@@ -52,6 +52,7 @@ func (s sortedPods) Less(i, j int) bool {
 func CreateValidPod(name, namespace, source string) api.BoundPod {
 	return api.BoundPod{
 		ObjectMeta: api.ObjectMeta{
+			UID:         name, // for the purpose of testing, this is unique enough
 			Name:        name,
 			Namespace:   namespace,
 			Annotations: map[string]string{kubelet.ConfigSourceAnnotationKey: source},

pkg/kubelet/config/etcd.go

@@ -20,7 +20,6 @@ package config
 import (
 	"errors"
 	"path"
-	"strconv"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -105,11 +104,8 @@ func eventToPods(ev watch.Event) ([]api.BoundPod, error) {
 	}
 
 	for _, pod := range boundPods.Items {
-		// TODO: generate random UID if not present
-		if pod.UID == "" && !pod.CreationTimestamp.IsZero() {
-			pod.UID = strconv.FormatInt(pod.CreationTimestamp.Unix(), 10)
-		}
 		// Backwards compatibility with old api servers
+		// TODO: Remove this after 1.0 release.
 		if len(pod.Namespace) == 0 {
 			pod.Namespace = api.NamespaceDefault
 		}

pkg/kubelet/config/etcd_test.go

@@ -44,14 +44,14 @@ func TestEventToPods(t *testing.T) {
 			input: watch.Event{
 				Object: &api.BoundPods{
 					Items: []api.BoundPod{
-						{ObjectMeta: api.ObjectMeta{Name: "foo"}},
-						{ObjectMeta: api.ObjectMeta{Name: "bar"}},
+						{ObjectMeta: api.ObjectMeta{UID: "111", Name: "foo", Namespace: "foo"}},
+						{ObjectMeta: api.ObjectMeta{UID: "222", Name: "bar", Namespace: "bar"}},
 					},
 				},
 			},
 			pods: []api.BoundPod{
-				{ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "default"}, Spec: api.PodSpec{}},
-				{ObjectMeta: api.ObjectMeta{Name: "bar", Namespace: "default"}, Spec: api.PodSpec{}},
+				{ObjectMeta: api.ObjectMeta{UID: "111", Name: "foo", Namespace: "foo"}, Spec: api.PodSpec{}},
+				{ObjectMeta: api.ObjectMeta{UID: "222", Name: "bar", Namespace: "bar"}, Spec: api.PodSpec{}},
 			},
 			fail: false,
 		},
@@ -59,14 +59,12 @@ func TestEventToPods(t *testing.T) {
 			input: watch.Event{
 				Object: &api.BoundPods{
 					Items: []api.BoundPod{
-						{ObjectMeta: api.ObjectMeta{Name: "1"}},
-						{ObjectMeta: api.ObjectMeta{Name: "2", Namespace: "foo"}},
+						{ObjectMeta: api.ObjectMeta{UID: "111", Name: "foo"}},
 					},
 				},
 			},
 			pods: []api.BoundPod{
-				{ObjectMeta: api.ObjectMeta{Name: "1", Namespace: "default"}, Spec: api.PodSpec{}},
-				{ObjectMeta: api.ObjectMeta{Name: "2", Namespace: "foo"}, Spec: api.PodSpec{}},
+				{ObjectMeta: api.ObjectMeta{UID: "111", Name: "foo", Namespace: "default"}, Spec: api.PodSpec{}},
 			},
 			fail: false,
 		},

pkg/kubelet/config/file.go

@@ -18,15 +18,14 @@ limitations under the License.
 package config
 
 import (
-	"crypto/sha1"
-	"encoding/base32"
+	"crypto/md5"
+	"encoding/hex"
 	"fmt"
+	"hash/adler32"
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"regexp"
 	"sort"
-	"strings"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -155,11 +154,27 @@ func extractFromFile(filename string) (api.BoundPod, error) {
 		return pod, fmt.Errorf("can't convert pod from file %q: %v", filename, err)
 	}
 
+	hostname, err := os.Hostname() //TODO: kubelet name would be better
+	if err != nil {
+		return pod, err
+	}
+
 	if len(pod.UID) == 0 {
-		pod.UID = simpleSubdomainSafeHash(filename)
+		hasher := md5.New()
+		fmt.Fprintf(hasher, "host:%s", hostname)
+		fmt.Fprintf(hasher, "file:%s", filename)
+		util.DeepHashObject(hasher, pod)
+		pod.UID = hex.EncodeToString(hasher.Sum(nil)[0:])
+		glog.V(5).Infof("Generated UID %q for pod %q from file %s", pod.UID, pod.Name, filename)
 	}
 	if len(pod.Namespace) == 0 {
-		pod.Namespace = api.NamespaceDefault
+		hasher := adler32.New()
+		fmt.Fprint(hasher, filename)
+		// TODO: file-<sum>.hostname would be better, if DNS subdomains
+		// are allowed for namespace (some places only allow DNS
+		// labels).
+		pod.Namespace = fmt.Sprintf("file-%08x-%s", hasher.Sum32(), hostname)
+		glog.V(5).Infof("Generated namespace %q for pod %q from file %s", pod.Namespace, pod.Name, filename)
 	}
 	// TODO(dchen1107): BoundPod is not type of runtime.Object. Once we allow kubelet talks
 	// about Pod directly, we can use SelfLinker defined in package: latest
@@ -174,17 +189,3 @@ func extractFromFile(filename string) (api.BoundPod, error) {
 	}
 	return pod, nil
 }
-
-var simpleSubdomainSafeEncoding = base32.NewEncoding("0123456789abcdefghijklmnopqrstuv")
-var unsafeDNSLabelReplacement = regexp.MustCompile("[^a-z0-9]+")
-
-// simpleSubdomainSafeHash generates a pod name for the given path that is
-// suitable as a subdomain label.
-func simpleSubdomainSafeHash(path string) string {
-	name := strings.ToLower(filepath.Base(path))
-	name = unsafeDNSLabelReplacement.ReplaceAllString(name, "")
-	hasher := sha1.New()
-	hasher.Write([]byte(path))
-	sha := simpleSubdomainSafeEncoding.EncodeToString(hasher.Sum(nil))
-	return fmt.Sprintf("%.15s%.30s", name, sha)
-}

pkg/kubelet/config/file_test.go

@@ -21,20 +21,19 @@ import (
 	"io/ioutil"
 	"os"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet"
-
-	"github.com/ghodss/yaml"
 )
 
 func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) {
 	manifest := api.ContainerManifest{
 		ID:   id,
-		UUID: "uid",
+		UUID: id,
 		Containers: []api.Container{
 			{
 				Name:  "c" + id,
@@ -53,9 +52,8 @@ func ExampleManifestAndPod(id string) (api.ContainerManifest, api.BoundPod) {
 	}
 	expectedPod := api.BoundPod{
 		ObjectMeta: api.ObjectMeta{
-			Name:      id,
-			UID:       "uid",
-			Namespace: "default",
+			Name: id,
+			UID:  id,
 		},
 		Spec: api.PodSpec{
 			Containers: []api.Container{
@@ -116,7 +114,13 @@ func writeTestFile(t *testing.T, dir, name string, contents string) *os.File {
 }
 
 func TestReadFromFile(t *testing.T) {
-	file := writeTestFile(t, os.TempDir(), "test_pod_config", "version: v1beta1\nid: test\ncontainers:\n- image: test/image")
+	file := writeTestFile(t, os.TempDir(), "test_pod_config",
+		`{
+			"version": "v1beta1",
+			"uuid": "12345",
+			"id": "test",
+			"containers": [{ "image": "test/image" }]
+		}`)
 	defer os.Remove(file.Name())
 
 	ch := make(chan interface{})
@@ -127,14 +131,28 @@ func TestReadFromFile(t *testing.T) {
 		expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.BoundPod{
 			ObjectMeta: api.ObjectMeta{
 				Name:      "test",
-				UID:       simpleSubdomainSafeHash(file.Name()),
-				Namespace: "default",
-				SelfLink:  "/api/v1beta2/pods/test?namespace=default",
+				UID:       "12345",
+				Namespace: "",
+				SelfLink:  "",
 			},
 			Spec: api.PodSpec{
 				Containers: []api.Container{{Image: "test/image", TerminationMessagePath: "/dev/termination-log"}},
 			},
 		})
+
+		// There's no way to provide namespace in ContainerManifest, so
+		// it will be defaulted.
+		if !strings.HasPrefix(update.Pods[0].ObjectMeta.Namespace, "file-") {
+			t.Errorf("Unexpected namespace: %s", update.Pods[0].ObjectMeta.Namespace)
+		}
+		update.Pods[0].ObjectMeta.Namespace = ""
+
+		// SelfLink depends on namespace.
+		if !strings.HasPrefix(update.Pods[0].ObjectMeta.SelfLink, "/api/") {
+			t.Errorf("Unexpected selflink: %s", update.Pods[0].ObjectMeta.SelfLink)
+		}
+		update.Pods[0].ObjectMeta.SelfLink = ""
+
 		if !api.Semantic.DeepEqual(expected, update) {
 			t.Fatalf("Expected %#v, Got %#v", expected, update)
 		}
@@ -144,6 +162,29 @@ func TestReadFromFile(t *testing.T) {
 	}
 }
 
+func TestReadFromFileWithDefaults(t *testing.T) {
+	file := writeTestFile(t, os.TempDir(), "test_pod_config",
+		`{
+			"version": "v1beta1",
+			"id": "test",
+			"containers": [{ "image": "test/image" }]
+		}`)
+	defer os.Remove(file.Name())
+
+	ch := make(chan interface{})
+	NewSourceFile(file.Name(), time.Millisecond, ch)
+	select {
+	case got := <-ch:
+		update := got.(kubelet.PodUpdate)
+		if update.Pods[0].ObjectMeta.UID == "" {
+			t.Errorf("Unexpected UID: %s", update.Pods[0].ObjectMeta.UID)
+		}
+
+	case <-time.After(2 * time.Millisecond):
+		t.Errorf("Expected update, timeout instead")
+	}
+}
+
 func TestExtractFromBadDataFile(t *testing.T) {
 	file := writeTestFile(t, os.TempDir(), "test_pod_config", string([]byte{1, 2, 3}))
 	defer os.Remove(file.Name())
@@ -157,30 +198,6 @@ func TestExtractFromBadDataFile(t *testing.T) {
 	expectEmptyChannel(t, ch)
 }
 
-func TestExtractFromValidDataFile(t *testing.T) {
-	manifest, expectedPod := ExampleManifestAndPod("id")
-
-	text, err := json.Marshal(manifest)
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-	file := writeTestFile(t, os.TempDir(), "test_pod_config", string(text))
-	defer os.Remove(file.Name())
-
-	expectedPod.ObjectMeta.SelfLink = "/api/v1beta2/pods/" + expectedPod.Name + "?namespace=default"
-	ch := make(chan interface{}, 1)
-	c := sourceFile{file.Name(), ch}
-	err = c.extractFromPath()
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-	update := (<-ch).(kubelet.PodUpdate)
-	expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, expectedPod)
-	if !api.Semantic.DeepEqual(expected, update) {
-		t.Errorf("Expected %#v, Got %#v", expected, update)
-	}
-}
-
 func TestExtractFromEmptyDir(t *testing.T) {
 	dirName, err := ioutil.TempDir("", "foo")
 	if err != nil {
@@ -233,7 +250,6 @@ func TestExtractFromDir(t *testing.T) {
 		}
 		ioutil.WriteFile(name, data, 0755)
 		files[i] = file
-		pods[i].ObjectMeta.SelfLink = "/api/v1beta2/pods/" + pods[i].Name + "?namespace=default"
 	}
 
 	ch := make(chan interface{}, 1)
@@ -244,7 +260,14 @@ func TestExtractFromDir(t *testing.T) {
 	}
 
 	update := (<-ch).(kubelet.PodUpdate)
+	for i := range update.Pods {
+		update.Pods[i].Namespace = "foobar"
+		update.Pods[i].SelfLink = ""
+	}
 	expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, pods...)
+	for i := range expected.Pods {
+		expected.Pods[i].Namespace = "foobar"
+	}
 	sort.Sort(sortedPods(update.Pods))
 	sort.Sort(sortedPods(expected.Pods))
 	if !api.Semantic.DeepEqual(expected, update) {
@@ -256,60 +279,3 @@ func TestExtractFromDir(t *testing.T) {
 		}
 	}
 }
-
-func TestSubdomainSafeName(t *testing.T) {
-	type Case struct {
-		Input    string
-		Expected string
-	}
-	testCases := []Case{
-		{"/some/path/invalidUPPERCASE", "invaliduppercasa6hlenc0vpqbbdtt26ghneqsq3pvud"},
-		{"/some/path/_-!%$#&@^&*(){}", "nvhc03p016m60huaiv3avts372rl2p"},
-	}
-	for _, testCase := range testCases {
-		value := simpleSubdomainSafeHash(testCase.Input)
-		if value != testCase.Expected {
-			t.Errorf("Expected %s, Got %s", testCase.Expected, value)
-		}
-		value2 := simpleSubdomainSafeHash(testCase.Input)
-		if value != value2 {
-			t.Errorf("Value for %s was not stable across runs: %s %s", testCase.Input, value, value2)
-		}
-	}
-}
-
-// These are used for testing extract json (below)
-type TestData struct {
-	Value  string
-	Number int
-}
-
-type TestObject struct {
-	Name string
-	Data TestData
-}
-
-func verifyStringEquals(t *testing.T, actual, expected string) {
-	if actual != expected {
-		t.Errorf("Verification failed.  Expected: %s, Found %s", expected, actual)
-	}
-}
-
-func verifyIntEquals(t *testing.T, actual, expected int) {
-	if actual != expected {
-		t.Errorf("Verification failed.  Expected: %d, Found %d", expected, actual)
-	}
-}
-
-func TestExtractJSON(t *testing.T) {
-	obj := TestObject{}
-	data := `{ "name": "foo", "data": { "value": "bar", "number": 10 } }`
-
-	if err := yaml.Unmarshal([]byte(data), &obj); err != nil {
-		t.Fatalf("Could not unmarshal JSON: %v", err)
-	}
-
-	verifyStringEquals(t, obj.Name, "foo")
-	verifyStringEquals(t, obj.Data.Value, "bar")
-	verifyIntEquals(t, obj.Data.Number, 10)
-}

pkg/kubelet/config/http.go

@@ -19,7 +19,10 @@ package config
 
 import (
 	"bytes"
+	"crypto/md5"
+	"encoding/hex"
 	"fmt"
+	"hash/adler32"
 	"io/ioutil"
 	"net/http"
 	"time"
@@ -93,9 +96,7 @@ func (s *sourceURL) extractFromURL() error {
 		if err := api.Scheme.Convert(&manifest, &pod); err != nil {
 			return err
 		}
-		if len(pod.Namespace) == 0 {
-			pod.Namespace = api.NamespaceDefault
-		}
+		applyDefaults(&pod, s.url)
 		s.updates <- kubelet.PodUpdate{[]api.BoundPod{pod}, kubelet.SET, kubelet.HTTPSource}
 		return nil
 	}
@@ -130,12 +131,7 @@ func (s *sourceURL) extractFromURL() error {
 		}
 		for i := range boundPods.Items {
 			pod := &boundPods.Items[i]
-			if len(pod.Name) == 0 {
-				pod.Name = fmt.Sprintf("%d", i+1)
-			}
-			if len(pod.Namespace) == 0 {
-				pod.Namespace = api.NamespaceDefault
-			}
+			applyDefaults(pod, s.url)
 		}
 		s.updates <- kubelet.PodUpdate{boundPods.Items, kubelet.SET, kubelet.HTTPSource}
 		return nil
@@ -145,3 +141,19 @@ func (s *sourceURL) extractFromURL() error {
 		"single manifest (%v: %+v) or as multiple manifests (%v: %+v).\n",
 		s.url, string(data), singleErr, manifest, multiErr, manifests)
 }
+
+func applyDefaults(pod *api.BoundPod, url string) {
+	if len(pod.UID) == 0 {
+		hasher := md5.New()
+		fmt.Fprintf(hasher, "url:%s", url)
+		util.DeepHashObject(hasher, pod)
+		pod.UID = hex.EncodeToString(hasher.Sum(nil)[0:])
+		glog.V(5).Infof("Generated UID %q for pod %q from URL %s", pod.UID, pod.Name, url)
+	}
+	if len(pod.Namespace) == 0 {
+		hasher := adler32.New()
+		fmt.Fprint(hasher, url)
+		pod.Namespace = fmt.Sprintf("url-%08x", hasher.Sum32())
+		glog.V(5).Infof("Generated namespace %q for pod %q from URL %s", pod.Namespace, pod.Name, url)
+	}
+}

pkg/kubelet/config/http_test.go

@@ -19,6 +19,7 @@ package config
 import (
 	"encoding/json"
 	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 
@@ -26,6 +27,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors"
 )
 
 func TestURLErrorNotExistNoUpdate(t *testing.T) {
@@ -121,13 +123,14 @@ func TestExtractFromHTTP(t *testing.T) {
 	}{
 		{
 			desc:      "Single manifest",
-			manifests: api.ContainerManifest{Version: "v1beta1", ID: "foo"},
+			manifests: api.ContainerManifest{Version: "v1beta1", ID: "foo", UUID: "111"},
 			expected: CreatePodUpdate(kubelet.SET,
 				kubelet.HTTPSource,
 				api.BoundPod{
 					ObjectMeta: api.ObjectMeta{
+						UID:       "111",
 						Name:      "foo",
-						Namespace: "default",
+						Namespace: "foobar",
 					},
 					Spec: api.PodSpec{
 						RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
@@ -138,15 +141,16 @@ func TestExtractFromHTTP(t *testing.T) {
 		{
 			desc: "Multiple manifests",
 			manifests: []api.ContainerManifest{
-				{Version: "v1beta1", ID: "", Containers: []api.Container{{Name: "1", Image: "foo"}}},
-				{Version: "v1beta1", ID: "bar", Containers: []api.Container{{Name: "1", Image: "foo"}}},
+				{Version: "v1beta1", ID: "foo", UUID: "111", Containers: []api.Container{{Name: "1", Image: "foo"}}},
+				{Version: "v1beta1", ID: "bar", UUID: "222", Containers: []api.Container{{Name: "1", Image: "foo"}}},
 			},
 			expected: CreatePodUpdate(kubelet.SET,
 				kubelet.HTTPSource,
 				api.BoundPod{
 					ObjectMeta: api.ObjectMeta{
-						Name:      "1",
-						Namespace: "default",
+						UID:       "111",
+						Name:      "foo",
+						Namespace: "foobar",
 					},
 					Spec: api.PodSpec{
 						Containers: []api.Container{{
@@ -157,8 +161,9 @@ func TestExtractFromHTTP(t *testing.T) {
 				},
 				api.BoundPod{
 					ObjectMeta: api.ObjectMeta{
+						UID:       "222",
 						Name:      "bar",
-						Namespace: "default",
+						Namespace: "foobar",
 					},
 					Spec: api.PodSpec{
 						Containers: []api.Container{{
@@ -192,12 +197,21 @@ func TestExtractFromHTTP(t *testing.T) {
 			continue
 		}
 		update := (<-ch).(kubelet.PodUpdate)
+
+		for i := range update.Pods {
+			// There's no way to provide namespace in ContainerManifest, so
+			// it will be defaulted.
+			if !strings.HasPrefix(update.Pods[i].ObjectMeta.Namespace, "url-") {
+				t.Errorf("Unexpected namespace: %s", update.Pods[0].ObjectMeta.Namespace)
+			}
+			update.Pods[i].ObjectMeta.Namespace = "foobar"
+		}
 		if !api.Semantic.DeepEqual(testCase.expected, update) {
 			t.Errorf("%s: Expected: %#v, Got: %#v", testCase.desc, testCase.expected, update)
 		}
 		for i := range update.Pods {
 			if errs := validation.ValidateBoundPod(&update.Pods[i]); len(errs) != 0 {
-				t.Errorf("%s: Expected no validation errors on %#v, Got %#v", testCase.desc, update.Pods[i], errs)
+				t.Errorf("%s: Expected no validation errors on %#v, Got %v", testCase.desc, update.Pods[i], errors.NewAggregate(errs))
 			}
 		}
 	}

pkg/kubelet/dockertools/docker.go

@@ -545,10 +545,10 @@ func HashContainer(container *api.Container) uint64 {
 }
 
 // Creates a name which can be reversed to identify both full pod name and container name.
-func BuildDockerName(manifestUUID, podFullName string, container *api.Container) string {
+func BuildDockerName(podUID, podFullName string, container *api.Container) string {
 	containerName := container.Name + "." + strconv.FormatUint(HashContainer(container), 16)
 	// Note, manifest.ID could be blank.
-	if len(manifestUUID) == 0 {
+	if len(podUID) == 0 {
 		return fmt.Sprintf("%s_%s_%s_%08x",
 			containerNamePrefix,
 			containerName,
@@ -559,7 +559,7 @@ func BuildDockerName(manifestUUID, podFullName string, container *api.Container)
 			containerNamePrefix,
 			containerName,
 			podFullName,
-			manifestUUID,
+			podUID,
 			rand.Uint32())
 	}
 }
@@ -590,6 +590,12 @@ func ParseDockerName(name string) (podFullName, uuid, containerName string, hash
 	if len(parts) > 2 {
 		podFullName = parts[2]
 	}
+	// This is not an off-by-one.  We check for > 4 here because (sadly) the
+	// format generated by BuildDockerName() has an optional field in the
+	// middle.  If len(parts) > 3, parts[3] might be the optional field or
+	// the (poorly documented) random suffix.  If len(parts) > 4, then we
+	// know [3] is the UUID and [4] is the suffix.  Sort of pukey, should
+	// be fixed by making UID non-optional.
 	if len(parts) > 4 {
 		uuid = parts[3]
 	}

pkg/kubelet/dockertools/docker_test.go

@@ -85,31 +85,31 @@ func TestGetContainerID(t *testing.T) {
 	}
 }
 
-func verifyPackUnpack(t *testing.T, podNamespace, manifestUUID, podName, containerName string) {
+func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName string) {
 	container := &api.Container{Name: containerName}
 	hasher := adler32.New()
 	util.DeepHashObject(hasher, *container)
 	computedHash := uint64(hasher.Sum32())
 	podFullName := fmt.Sprintf("%s.%s", podName, podNamespace)
-	name := BuildDockerName(manifestUUID, podFullName, container)
-	returnedPodFullName, returnedUUID, returnedContainerName, hash := ParseDockerName(name)
-	if podFullName != returnedPodFullName || manifestUUID != returnedUUID || containerName != returnedContainerName || computedHash != hash {
-		t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, manifestUUID, containerName, computedHash, returnedPodFullName, returnedUUID, returnedContainerName, hash)
+	name := BuildDockerName(podUID, podFullName, container)
+	returnedPodFullName, returnedUID, returnedContainerName, hash := ParseDockerName(name)
+	if podFullName != returnedPodFullName || podUID != returnedUID || containerName != returnedContainerName || computedHash != hash {
+		t.Errorf("For (%s, %s, %s, %d), unpacked (%s, %s, %s, %d)", podFullName, podUID, containerName, computedHash, returnedPodFullName, returnedUID, returnedContainerName, hash)
 	}
 }
 
 func TestContainerManifestNaming(t *testing.T) {
-	manifestUUID := "d1b925c9-444a-11e4-a576-42010af0a203"
-	verifyPackUnpack(t, "file", manifestUUID, "manifest1234", "container5678")
-	verifyPackUnpack(t, "file", manifestUUID, "mani-fest-1234", "container5678")
-	// UUID is same as pod name
-	verifyPackUnpack(t, "file", manifestUUID, manifestUUID, "container123")
+	podUID := "d1b925c9-444a-11e4-a576-42010af0a203"
+	verifyPackUnpack(t, "file", podUID, "manifest1234", "container5678")
+	verifyPackUnpack(t, "file", podUID, "mani-fest-1234", "container5678")
+	// UID is same as pod name
+	verifyPackUnpack(t, "file", podUID, podUID, "container123")
 	// empty namespace
-	verifyPackUnpack(t, "", manifestUUID, manifestUUID, "container123")
-	// No UUID
-	verifyPackUnpack(t, "other", "", manifestUUID, "container456")
+	verifyPackUnpack(t, "", podUID, podUID, "container123")
+	// No UID
+	verifyPackUnpack(t, "other", "", podUID, "container456")
 	// No Container name
-	verifyPackUnpack(t, "other", "", manifestUUID, "")
+	verifyPackUnpack(t, "other", "", podUID, "")
 
 	container := &api.Container{Name: "container"}
 	podName := "foo"

pkg/kubelet/kubelet.go

@@ -1056,8 +1056,7 @@ func (kl *Kubelet) SyncPods(pods []api.BoundPod) error {
 
 		// Run the sync in an async manifest worker.
 		kl.podWorkers.Run(podFullName, func() {
-			err := kl.syncPod(pod, dockerContainers)
-			if err != nil {
+			if err := kl.syncPod(pod, dockerContainers); err != nil {
 				glog.Errorf("Error syncing pod, skipping: %v", err)
 				record.Eventf(pod, "", "failedSync", "Error syncing pod, skipping: %v", err)
 			}

pkg/kubelet/kubelet_test.go

@@ -291,19 +291,20 @@ func TestSyncPodsDoesNothing(t *testing.T) {
 	container := api.Container{Name: "bar"}
 	fakeDocker.ContainerList = []docker.APIContainers{
 		{
-			// format is k8s_<container-id>_<pod-fullname>
-			Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "_foo.new.test"},
+			// format is // k8s_<container-id>_<pod-fullname>_<pod-uid>_<random>
+			Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "_foo.new.test_12345678_0"},
 			ID:    "1234",
 		},
 		{
 			// network container
-			Names: []string{"/k8s_net_foo.new.test_"},
+			Names: []string{"/k8s_net_foo.new.test_12345678_0"},
 			ID:    "9876",
 		},
 	}
 	err := kubelet.SyncPods([]api.BoundPod{
 		{
 			ObjectMeta: api.ObjectMeta{
+				UID:         "12345678",
 				Name:        "foo",
 				Namespace:   "new",
 				Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},
@@ -476,13 +477,14 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
 	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// network container
-			Names: []string{"/k8s_net_foo.new.test_"},
+			Names: []string{"/k8s_net_foo.new.test_12345678_0"},
 			ID:    "9876",
 		},
 	}
 	err := kubelet.SyncPods([]api.BoundPod{
 		{
 			ObjectMeta: api.ObjectMeta{
+				UID:         "12345678",
 				Name:        "foo",
 				Namespace:   "new",
 				Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},
@@ -517,13 +519,14 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
 	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			// network container
-			Names: []string{"/k8s_net_foo.new.test_"},
+			Names: []string{"/k8s_net_foo.new.test_12345678_0"},
 			ID:    "9876",
 		},
 	}
 	err := kubelet.SyncPods([]api.BoundPod{
 		{
 			ObjectMeta: api.ObjectMeta{
+				UID:         "12345678",
 				Name:        "foo",
 				Namespace:   "new",
 				Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},
@@ -569,14 +572,15 @@ func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
 	kubelet, _, fakeDocker := newTestKubelet(t)
 	fakeDocker.ContainerList = []docker.APIContainers{
 		{
-			// format is k8s_<container-id>_<pod-fullname>
-			Names: []string{"/k8s_bar_foo.new.test"},
+			// format is // k8s_<container-id>_<pod-fullname>_<pod-uid>
+			Names: []string{"/k8s_bar_foo.new.test_12345678_0"},
 			ID:    "1234",
 		},
 	}
 	err := kubelet.SyncPods([]api.BoundPod{
 		{
 			ObjectMeta: api.ObjectMeta{
+				UID:         "12345678",
 				Name:        "foo",
 				Namespace:   "new",
 				Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},
@@ -694,17 +698,17 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {
 	dockerContainers := dockertools.DockerContainers{
 		"1234": &docker.APIContainers{
 			// the k8s prefix is required for the kubelet to manage the container
-			Names: []string{"/k8s_foo_bar.new.test_1"},
+			Names: []string{"/k8s_foo_bar.new.test_12345678_1111"},
 			ID:    "1234",
 		},
 		"9876": &docker.APIContainers{
 			// network container
-			Names: []string{"/k8s_net_bar.new.test_"},
+			Names: []string{"/k8s_net_bar.new.test_12345678_2222"},
 			ID:    "9876",
 		},
 		"4567": &docker.APIContainers{
 			// Duplicate for the same container.
-			Names: []string{"/k8s_foo_bar.new.test_2"},
+			Names: []string{"/k8s_foo_bar.new.test_12345678_3333"},
 			ID:    "4567",
 		},
 		"2304": &docker.APIContainers{
@@ -715,6 +719,7 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {
 	}
 	err := kubelet.syncPod(&api.BoundPod{
 		ObjectMeta: api.ObjectMeta{
+			UID:         "12345678",
 			Name:        "bar",
 			Namespace:   "new",
 			Annotations: map[string]string{ConfigSourceAnnotationKey: "test"},
@@ -732,7 +737,7 @@ func TestSyncPodDeletesDuplicate(t *testing.T) {
 	verifyCalls(t, fakeDocker, []string{"list", "stop"})
 
 	// Expect one of the duplicates to be killed.
-	if len(fakeDocker.Stopped) != 1 || (len(fakeDocker.Stopped) != 0 && fakeDocker.Stopped[0] != "1234" && fakeDocker.Stopped[0] != "4567") {
+	if len(fakeDocker.Stopped) != 1 || (fakeDocker.Stopped[0] != "1234" && fakeDocker.Stopped[0] != "4567") {
 		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
 	}
 }

pkg/kubelet/types.go

@@ -22,7 +22,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 )
 
-const ConfigSourceAnnotationKey = "kubernetes/config.source"
+const ConfigSourceAnnotationKey = "kubernetes.io/config.source"
 
 // PodOperation defines what changes will be made on a pod configuration.
 type PodOperation int