Browse code

Merge pull request #3508 from soltysh/issue3502

Merged by openshift-bot

OpenShift Bot authored on 2015/07/16 07:49:25
Showing 14 changed files
1 1
new file mode 100644
... ...
@@ -0,0 +1,14 @@
0
+# Upgrading
1
+
2
+This document describes future changes that will affect your current resources used
3
+inside of OpenShift. Each change contains description of the change and information
4
+when that change will happen.
5
+
6
+
7
+## Origin 1.0.x / OSE 3.0.x
8
+
9
+* Currently all build pods have a label named `build`. This label is being deprecated
10
+  in favor of `openshift.io/build.name` in Origin 1.0.x / OSE 3.1.x at which point both
11
+  labels will be supported. All the newly created builds will have just the new label.
12
+  In Origin 1.y / OSE 3.y the support for the old label (`build`) will be removed entirely.
13
+  See #3502.
... ...
@@ -10,9 +10,10 @@ import (
10 10
 const (
11 11
 	// BuildAnnotation is an annotation that identifies a Pod as being for a Build
12 12
 	BuildAnnotation = "openshift.io/build.name"
13
-
13
+	// DeprecatedBuildLabel is old value of BuildLabel, it'll be removed in OpenShift 3.1.
14
+	DeprecatedBuildLabel = "build"
14 15
 	// BuildLabel is the key of a Pod label whose value is the Name of a Build which is run.
15
-	BuildLabel = "build"
16
+	BuildLabel = "openshift.io/build.name"
16 17
 )
17 18
 
18 19
 // Build encapsulates the inputs needed to produce a new deployable image, as well as
... ...
@@ -7,9 +7,6 @@ import (
7 7
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
8 8
 )
9 9
 
10
-// BuildLabel is the key of a Pod label whose value is the Name of a Build which is run.
11
-const BuildLabel = "build"
12
-
13 10
 // Build encapsulates the inputs needed to produce a new deployable image, as well as
14 11
 // the status of the execution and a reference to the Pod which executed the build.
15 12
 type Build struct {
... ...
@@ -7,9 +7,6 @@ import (
7 7
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
8 8
 )
9 9
 
10
-// BuildLabel is the key of a Pod label whose value is the Name of a Build which is run.
11
-const BuildLabel = "build"
12
-
13 10
 // Build encapsulates the inputs needed to produce a new deployable image, as well as
14 11
 // the status of the execution and a reference to the Pod which executed the build.
15 12
 type Build struct {
... ...
@@ -303,8 +303,8 @@ func (bc *BuildDeleteController) HandleBuildDeletion(build *buildapi.Build) erro
303 303
 		glog.V(2).Infof("Did not find pod with name %s for Build %s in namespace %s", podName, build.Name, build.Namespace)
304 304
 		return nil
305 305
 	}
306
-	if pod.Labels[buildapi.BuildLabel] != build.Name {
307
-		glog.V(2).Infof("Not deleting pod %s/%s because the build label %s does not match the build name %s", pod.Namespace, podName, pod.Labels[buildapi.BuildLabel], build.Name)
306
+	if buildName, _ := buildutil.GetBuildLabel(pod); buildName != build.Name {
307
+		glog.V(2).Infof("Not deleting pod %s/%s because the build label %s does not match the build name %s", pod.Namespace, podName, buildName, build.Name)
308 308
 		return nil
309 309
 	}
310 310
 	err = bc.PodManager.DeletePod(build.Namespace, pod)
... ...
@@ -108,7 +108,7 @@ func (*errNotFoundImageStreamClient) GetImageStream(namespace, name string) (*im
108 108
 	return nil, kerrors.NewNotFound("ImageStream", name)
109 109
 }
110 110
 
111
-func mockBuild(status buildapi.BuildPhase, output buildapi.BuildOutput) *buildapi.Build {
111
+func mockBuild(phase buildapi.BuildPhase, output buildapi.BuildOutput) *buildapi.Build {
112 112
 	return &buildapi.Build{
113 113
 		ObjectMeta: kapi.ObjectMeta{
114 114
 			Name:      "data-build",
... ...
@@ -132,7 +132,7 @@ func mockBuild(status buildapi.BuildPhase, output buildapi.BuildOutput) *buildap
132 132
 			Output: output,
133 133
 		},
134 134
 		Status: buildapi.BuildStatus{
135
-			Phase: status,
135
+			Phase: phase,
136 136
 		},
137 137
 	}
138 138
 }
... ...
@@ -644,3 +644,262 @@ func TestCancelBuild(t *testing.T) {
644 644
 		}
645 645
 	}
646 646
 }
647
+
648
+type customPodManager struct {
649
+	CreatePodFunc func(namespace string, pod *kapi.Pod) (*kapi.Pod, error)
650
+	DeletePodFunc func(namespace string, pod *kapi.Pod) error
651
+	GetPodFunc    func(namespace, name string) (*kapi.Pod, error)
652
+}
653
+
654
+func (c *customPodManager) CreatePod(namespace string, pod *kapi.Pod) (*kapi.Pod, error) {
655
+	return c.CreatePodFunc(namespace, pod)
656
+}
657
+
658
+func (c *customPodManager) DeletePod(namespace string, pod *kapi.Pod) error {
659
+	return c.DeletePodFunc(namespace, pod)
660
+}
661
+
662
+func (c *customPodManager) GetPod(namespace, name string) (*kapi.Pod, error) {
663
+	return c.GetPodFunc(namespace, name)
664
+}
665
+
666
+func TestHandleHandleBuildDeletionOK(t *testing.T) {
667
+	deleteWasCalled := false
668
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
669
+	ctrl := BuildDeleteController{&customPodManager{
670
+		GetPodFunc: func(namespace, names string) (*kapi.Pod, error) {
671
+			return &kapi.Pod{ObjectMeta: kapi.ObjectMeta{Labels: map[string]string{buildapi.BuildLabel: build.Name}}}, nil
672
+		},
673
+		DeletePodFunc: func(namespace string, pod *kapi.Pod) error {
674
+			deleteWasCalled = true
675
+			return nil
676
+		},
677
+	}}
678
+
679
+	err := ctrl.HandleBuildDeletion(build)
680
+	if err != nil {
681
+		t.Errorf("Unexpected error %v", err)
682
+	}
683
+	if !deleteWasCalled {
684
+		t.Error("DeletePod was not called when it should!")
685
+	}
686
+}
687
+
688
+func TestHandleHandleBuildDeletionOKDeprecatedLabel(t *testing.T) {
689
+	deleteWasCalled := false
690
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
691
+	ctrl := BuildDeleteController{&customPodManager{
692
+		GetPodFunc: func(namespace, names string) (*kapi.Pod, error) {
693
+			return &kapi.Pod{ObjectMeta: kapi.ObjectMeta{Labels: map[string]string{buildapi.DeprecatedBuildLabel: build.Name}}}, nil
694
+		},
695
+		DeletePodFunc: func(namespace string, pod *kapi.Pod) error {
696
+			deleteWasCalled = true
697
+			return nil
698
+		},
699
+	}}
700
+
701
+	err := ctrl.HandleBuildDeletion(build)
702
+	if err != nil {
703
+		t.Errorf("Unexpected error %v", err)
704
+	}
705
+	if !deleteWasCalled {
706
+		t.Error("DeletePod was not called when it should!")
707
+	}
708
+}
709
+
710
+func TestHandleHandleBuildDeletionFailGetPod(t *testing.T) {
711
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
712
+	ctrl := BuildDeleteController{&customPodManager{
713
+		GetPodFunc: func(namespace, name string) (*kapi.Pod, error) {
714
+			return nil, errors.New("random")
715
+		},
716
+	}}
717
+
718
+	err := ctrl.HandleBuildDeletion(build)
719
+	if err == nil {
720
+		t.Error("Expected random error got none!")
721
+	}
722
+}
723
+
724
+func TestHandleHandleBuildDeletionGetPodNotFound(t *testing.T) {
725
+	deleteWasCalled := false
726
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
727
+	ctrl := BuildDeleteController{&customPodManager{
728
+		GetPodFunc: func(namespace, name string) (*kapi.Pod, error) {
729
+			return nil, kerrors.NewNotFound("Pod", name)
730
+		},
731
+		DeletePodFunc: func(namespace string, pod *kapi.Pod) error {
732
+			deleteWasCalled = true
733
+			return nil
734
+		},
735
+	}}
736
+
737
+	err := ctrl.HandleBuildDeletion(build)
738
+	if err != nil {
739
+		t.Errorf("Unexpected error, %v", err)
740
+	}
741
+	if deleteWasCalled {
742
+		t.Error("DeletePod was called when it should not!")
743
+	}
744
+}
745
+
746
+func TestHandleHandleBuildDeletionMismatchedLabels(t *testing.T) {
747
+	deleteWasCalled := false
748
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
749
+	ctrl := BuildDeleteController{&customPodManager{
750
+		GetPodFunc: func(namespace, names string) (*kapi.Pod, error) {
751
+			return &kapi.Pod{}, nil
752
+		},
753
+		DeletePodFunc: func(namespace string, pod *kapi.Pod) error {
754
+			deleteWasCalled = true
755
+			return nil
756
+		},
757
+	}}
758
+
759
+	err := ctrl.HandleBuildDeletion(build)
760
+	if err != nil {
761
+		t.Errorf("Unexpected error %v", err)
762
+	}
763
+	if deleteWasCalled {
764
+		t.Error("DeletePod was called when it should not!")
765
+	}
766
+}
767
+
768
+func TestHandleHandleBuildDeletionDeletePodError(t *testing.T) {
769
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
770
+	ctrl := BuildDeleteController{&customPodManager{
771
+		GetPodFunc: func(namespace, names string) (*kapi.Pod, error) {
772
+			return &kapi.Pod{ObjectMeta: kapi.ObjectMeta{Labels: map[string]string{buildapi.BuildLabel: build.Name}}}, nil
773
+		},
774
+		DeletePodFunc: func(namespace string, pod *kapi.Pod) error {
775
+			return errors.New("random")
776
+		},
777
+	}}
778
+
779
+	err := ctrl.HandleBuildDeletion(build)
780
+	if err == nil {
781
+		t.Error("Expected random error got none!")
782
+	}
783
+}
784
+
785
+type customBuildUpdater struct {
786
+	UpdateFunc func(namespace string, build *buildapi.Build) error
787
+}
788
+
789
+func (c *customBuildUpdater) Update(namespace string, build *buildapi.Build) error {
790
+	return c.UpdateFunc(namespace, build)
791
+}
792
+
793
+func mockBuildPodDeleteController(build *buildapi.Build, buildUpdater *customBuildUpdater, err error) *BuildPodDeleteController {
794
+	return &BuildPodDeleteController{
795
+		BuildStore:   buildtest.FakeBuildStore{Build: build, Err: err},
796
+		BuildUpdater: buildUpdater,
797
+	}
798
+}
799
+
800
+func TestHandleBuildPodDeletionOK(t *testing.T) {
801
+	updateWasCalled := false
802
+	// only not finished build (buildutil.IsBuildComplete) should be handled
803
+	build := mockBuild(buildapi.BuildPhaseRunning, buildapi.BuildOutput{})
804
+	ctrl := mockBuildPodDeleteController(build, &customBuildUpdater{
805
+		UpdateFunc: func(namespace string, build *buildapi.Build) error {
806
+			updateWasCalled = true
807
+			return nil
808
+		},
809
+	}, nil)
810
+	pod := mockPod(kapi.PodSucceeded, 0)
811
+
812
+	err := ctrl.HandleBuildPodDeletion(pod)
813
+	if err != nil {
814
+		t.Errorf("Unexpected error %v", err)
815
+	}
816
+	if !updateWasCalled {
817
+		t.Error("UpdateBuild was not called when it should!")
818
+	}
819
+}
820
+
821
+func TestHandleBuildPodDeletionOKFinishedBuild(t *testing.T) {
822
+	updateWasCalled := false
823
+	// finished build buildutil.IsBuildComplete should not be handled
824
+	build := mockBuild(buildapi.BuildPhaseComplete, buildapi.BuildOutput{})
825
+	ctrl := mockBuildPodDeleteController(build, &customBuildUpdater{
826
+		UpdateFunc: func(namespace string, build *buildapi.Build) error {
827
+			updateWasCalled = true
828
+			return nil
829
+		},
830
+	}, nil)
831
+	pod := mockPod(kapi.PodSucceeded, 0)
832
+
833
+	err := ctrl.HandleBuildPodDeletion(pod)
834
+	if err != nil {
835
+		t.Errorf("Unexpected error %v", err)
836
+	}
837
+	if updateWasCalled {
838
+		t.Error("UpdateBuild was called when it should not!")
839
+	}
840
+}
841
+
842
+func TestHandleBuildPodDeletionOKErroneousBuild(t *testing.T) {
843
+	updateWasCalled := false
844
+	// erroneous builds should not be handled
845
+	build := mockBuild(buildapi.BuildPhaseError, buildapi.BuildOutput{})
846
+	ctrl := mockBuildPodDeleteController(build, &customBuildUpdater{
847
+		UpdateFunc: func(namespace string, build *buildapi.Build) error {
848
+			updateWasCalled = true
849
+			return nil
850
+		},
851
+	}, nil)
852
+	pod := mockPod(kapi.PodSucceeded, 0)
853
+
854
+	err := ctrl.HandleBuildPodDeletion(pod)
855
+	if err != nil {
856
+		t.Errorf("Unexpected error %v", err)
857
+	}
858
+	if updateWasCalled {
859
+		t.Error("UpdateBuild was called when it should not!")
860
+	}
861
+}
862
+
863
+func TestHandleBuildPodDeletionBuildGetError(t *testing.T) {
864
+	ctrl := mockBuildPodDeleteController(nil, &customBuildUpdater{}, errors.New("random"))
865
+	pod := mockPod(kapi.PodSucceeded, 0)
866
+
867
+	err := ctrl.HandleBuildPodDeletion(pod)
868
+	if err == nil {
869
+		t.Error("Expected random error, but got none!")
870
+	}
871
+}
872
+
873
+func TestHandleBuildPodDeletionBuildNotExists(t *testing.T) {
874
+	updateWasCalled := false
875
+	ctrl := mockBuildPodDeleteController(nil, &customBuildUpdater{
876
+		UpdateFunc: func(namespace string, build *buildapi.Build) error {
877
+			updateWasCalled = true
878
+			return nil
879
+		},
880
+	}, nil)
881
+	pod := mockPod(kapi.PodSucceeded, 0)
882
+
883
+	err := ctrl.HandleBuildPodDeletion(pod)
884
+	if err != nil {
885
+		t.Errorf("Unexpected error %v", err)
886
+	}
887
+	if updateWasCalled {
888
+		t.Error("UpdateBuild was called when it should not!")
889
+	}
890
+}
891
+
892
+func TestHandleBuildPodDeletionBuildUpdateError(t *testing.T) {
893
+	build := mockBuild(buildapi.BuildPhaseRunning, buildapi.BuildOutput{})
894
+	ctrl := mockBuildPodDeleteController(build, &customBuildUpdater{
895
+		UpdateFunc: func(namespace string, build *buildapi.Build) error {
896
+			return errors.New("random")
897
+		},
898
+	}, nil)
899
+	pod := mockPod(kapi.PodSucceeded, 0)
900
+
901
+	err := ctrl.HandleBuildPodDeletion(pod)
902
+	if err == nil {
903
+		t.Error("Expected random error, but got none!")
904
+	}
905
+}
... ...
@@ -323,13 +323,53 @@ type podLW struct {
323 323
 
324 324
 // List lists all Pods that have a build label.
325 325
 func (lw *podLW) List() (runtime.Object, error) {
326
-	sel, _ := labels.Parse(buildapi.BuildLabel)
327
-	return lw.client.Pods(kapi.NamespaceAll).List(sel, fields.Everything())
326
+	return listPods(lw.client)
327
+}
328
+
329
+func listPods(client kclient.Interface) (*kapi.PodList, error) {
330
+	// get builds with new label
331
+	sel, err := labels.Parse(buildapi.BuildLabel)
332
+	if err != nil {
333
+		return nil, err
334
+	}
335
+	listNew, err := client.Pods(kapi.NamespaceAll).List(sel, fields.Everything())
336
+	if err != nil {
337
+		return nil, err
338
+	}
339
+	// FIXME: get builds with old label - remove this when depracated label will be removed
340
+	selOld, err := labels.Parse(buildapi.DeprecatedBuildLabel)
341
+	if err != nil {
342
+		return nil, err
343
+	}
344
+	listOld, err := client.Pods(kapi.NamespaceAll).List(selOld, fields.Everything())
345
+	if err != nil {
346
+		return nil, err
347
+	}
348
+	listNew.Items = mergeWithoutDuplicates(listNew.Items, listOld.Items)
349
+	return listNew, nil
350
+}
351
+
352
+func mergeWithoutDuplicates(arrays ...[]kapi.Pod) []kapi.Pod {
353
+	tmpMap := make(map[string]kapi.Pod)
354
+	for _, array := range arrays {
355
+		for _, v := range array {
356
+			tmpMap[fmt.Sprintf("%s/%s", v.Namespace, v.Name)] = v
357
+		}
358
+	}
359
+	var result []kapi.Pod
360
+	for _, v := range tmpMap {
361
+		result = append(result, v)
362
+	}
363
+	return result
328 364
 }
329 365
 
330 366
 // Watch watches all Pods that have a build label.
331 367
 func (lw *podLW) Watch(resourceVersion string) (watch.Interface, error) {
332
-	sel, _ := labels.Parse(buildapi.BuildLabel)
368
+	// FIXME: since we cannot have OR on label name we'll just get builds with new label
369
+	sel, err := labels.Parse(buildapi.BuildLabel)
370
+	if err != nil {
371
+		return nil, err
372
+	}
333 373
 	return lw.client.Pods(kapi.NamespaceAll).Watch(sel, fields.Everything(), resourceVersion)
334 374
 }
335 375
 
... ...
@@ -357,31 +397,32 @@ type buildDeleteLW struct {
357 357
 // List returns an empty list but adds delete events to the store for all Builds that have been deleted but still have pods.
358 358
 func (lw *buildDeleteLW) List() (runtime.Object, error) {
359 359
 	glog.V(5).Info("Checking for deleted builds")
360
-	sel, _ := labels.Parse(buildapi.BuildLabel)
361
-	podList, err := lw.KubeClient.Pods(kapi.NamespaceAll).List(sel, fields.Everything())
360
+	podList, err := listPods(lw.KubeClient)
362 361
 	if err != nil {
363 362
 		glog.V(4).Infof("Failed to find any pods due to error %v", err)
364 363
 		return nil, err
365 364
 	}
365
+
366 366
 	for _, pod := range podList.Items {
367
-		if len(pod.Labels[buildapi.BuildLabel]) == 0 {
367
+		buildName, exists := buildutil.GetBuildLabel(&pod)
368
+		if !exists {
368 369
 			continue
369 370
 		}
370 371
 		glog.V(5).Infof("Found build pod %s/%s", pod.Namespace, pod.Name)
371 372
 
372
-		build, err := lw.Client.Builds(pod.Namespace).Get(pod.Labels[buildapi.BuildLabel])
373
+		build, err := lw.Client.Builds(pod.Namespace).Get(buildName)
373 374
 		if err != nil && !kerrors.IsNotFound(err) {
374 375
 			glog.V(4).Infof("Error getting build for pod %s/%s: %v", pod.Namespace, pod.Name, err)
375 376
 			return nil, err
376 377
 		}
377 378
 		if err != nil && kerrors.IsNotFound(err) {
378 379
 			build = nil
379
-		}
380 380
 
381
+		}
381 382
 		if build == nil {
382 383
 			deletedBuild := &buildapi.Build{
383 384
 				ObjectMeta: kapi.ObjectMeta{
384
-					Name:      pod.Labels[buildapi.BuildLabel],
385
+					Name:      buildName,
385 386
 					Namespace: pod.Namespace,
386 387
 				},
387 388
 			}
... ...
@@ -399,7 +440,6 @@ func (lw *buildDeleteLW) List() (runtime.Object, error) {
399 399
 
400 400
 // Watch watches all Builds.
401 401
 func (lw *buildDeleteLW) Watch(resourceVersion string) (watch.Interface, error) {
402
-	//return lw.client.Client.Builds(kapi.NamespaceAll).Watch(labels.Everything(), fields.Everything(), resourceVersion)
403 402
 	return lw.Client.Builds(kapi.NamespaceAll).Watch(labels.Everything(), fields.Everything(), resourceVersion)
404 403
 }
405 404
 
... ...
@@ -454,12 +494,17 @@ func (lw *buildPodDeleteLW) List() (runtime.Object, error) {
454 454
 			continue
455 455
 		}
456 456
 		pod, err := lw.KubeClient.Pods(build.Namespace).Get(buildutil.GetBuildPodName(&build))
457
-		if err != nil && !kerrors.IsNotFound(err) {
458
-			glog.V(4).Infof("Error getting pod for build %s/%s: %v", build.Namespace, build.Name, err)
459
-			return nil, err
460
-		}
461
-		if (err != nil && kerrors.IsNotFound(err)) || pod.Labels[buildapi.BuildLabel] != build.Name {
462
-			pod = nil
457
+		if err != nil {
458
+			if !kerrors.IsNotFound(err) {
459
+				glog.V(4).Infof("Error getting pod for build %s/%s: %v", build.Namespace, build.Name, err)
460
+				return nil, err
461
+			} else {
462
+				pod = nil
463
+			}
464
+		} else {
465
+			if buildName, _ := buildutil.GetBuildLabel(pod); buildName != build.Name {
466
+				pod = nil
467
+			}
463 468
 		}
464 469
 		if pod == nil {
465 470
 			deletedPod := &kapi.Pod{
... ...
@@ -482,7 +527,11 @@ func (lw *buildPodDeleteLW) List() (runtime.Object, error) {
482 482
 
483 483
 // Watch watches all Pods that have a build label, for deletion
484 484
 func (lw *buildPodDeleteLW) Watch(resourceVersion string) (watch.Interface, error) {
485
-	sel, _ := labels.Parse(buildapi.BuildLabel)
485
+	// FIXME: since we cannot have OR on label name we'll just get builds with new label
486
+	sel, err := labels.Parse(buildapi.BuildLabel)
487
+	if err != nil {
488
+		return nil, err
489
+	}
486 490
 	return lw.KubeClient.Pods(kapi.NamespaceAll).Watch(sel, fields.Everything(), resourceVersion)
487 491
 }
488 492
 
... ...
@@ -35,12 +35,7 @@ func TestCustomCreateBuildPod(t *testing.T) {
35 35
 	if expected, actual := buildutil.GetBuildPodName(expected), actual.ObjectMeta.Name; expected != actual {
36 36
 		t.Errorf("Expected %s, but got %s!", expected, actual)
37 37
 	}
38
-	expectedLabels := make(map[string]string)
39
-	for k, v := range expected.Labels {
40
-		expectedLabels[k] = v
41
-	}
42
-	expectedLabels[buildapi.BuildLabel] = expected.Name
43
-	if !reflect.DeepEqual(expectedLabels, actual.Labels) {
38
+	if !reflect.DeepEqual(map[string]string{buildapi.BuildLabel: expected.Name}, actual.Labels) {
44 39
 		t.Errorf("Pod Labels does not match Build Labels!")
45 40
 	}
46 41
 	container := actual.Spec.Containers[0]
... ...
@@ -27,12 +27,7 @@ func TestDockerCreateBuildPod(t *testing.T) {
27 27
 	if expected, actual := buildutil.GetBuildPodName(expected), actual.ObjectMeta.Name; expected != actual {
28 28
 		t.Errorf("Expected %s, but got %s!", expected, actual)
29 29
 	}
30
-	expectedLabels := make(map[string]string)
31
-	for k, v := range expected.Labels {
32
-		expectedLabels[k] = v
33
-	}
34
-	expectedLabels[buildapi.BuildLabel] = expected.Name
35
-	if !reflect.DeepEqual(expectedLabels, actual.Labels) {
30
+	if !reflect.DeepEqual(map[string]string{buildapi.BuildLabel: expected.Name}, actual.Labels) {
36 31
 		t.Errorf("Pod Labels does not match Build Labels!")
37 32
 	}
38 33
 	container := actual.Spec.Containers[0]
... ...
@@ -34,12 +34,7 @@ func TestSTICreateBuildPod(t *testing.T) {
34 34
 	if expected, actual := buildutil.GetBuildPodName(expected), actual.ObjectMeta.Name; expected != actual {
35 35
 		t.Errorf("Expected %s, but got %s!", expected, actual)
36 36
 	}
37
-	expectedLabels := make(map[string]string)
38
-	for k, v := range expected.Labels {
39
-		expectedLabels[k] = v
40
-	}
41
-	expectedLabels[buildapi.BuildLabel] = expected.Name
42
-	if !reflect.DeepEqual(expectedLabels, actual.Labels) {
37
+	if !reflect.DeepEqual(map[string]string{buildapi.BuildLabel: expected.Name}, actual.Labels) {
43 38
 		t.Errorf("Pod Labels does not match Build Labels!")
44 39
 	}
45 40
 	container := actual.Spec.Containers[0]
... ...
@@ -184,12 +184,7 @@ func getContainerVerbosity(containerEnv []kapi.EnvVar) (verbosity string) {
184 184
 	return
185 185
 }
186 186
 
187
-// getPodLabels copies build labels and adds additional one with build name itself
187
+// getPodLabels creates labels for the Build Pod
188 188
 func getPodLabels(build *buildapi.Build) map[string]string {
189
-	podLabels := make(map[string]string)
190
-	for k, v := range build.Labels {
191
-		podLabels[k] = v
192
-	}
193
-	podLabels[buildapi.BuildLabel] = build.Name
194
-	return podLabels
189
+	return map[string]string{buildapi.BuildLabel: build.Name}
195 190
 }
... ...
@@ -38,13 +38,13 @@ func (s FakeBuildStore) ContainedIDs() util.StringSet {
38 38
 	return util.NewStringSet()
39 39
 }
40 40
 
41
-func (s FakeBuildStore) Get(obj interface{}) (item interface{}, exists bool, err error) {
41
+func (s FakeBuildStore) Get(obj interface{}) (interface{}, bool, error) {
42 42
 	return s.GetByKey("")
43 43
 }
44 44
 
45
-func (s FakeBuildStore) GetByKey(id string) (item interface{}, exists bool, err error) {
45
+func (s FakeBuildStore) GetByKey(id string) (interface{}, bool, error) {
46 46
 	if s.Err != nil {
47
-		return nil, false, err
47
+		return nil, false, s.Err
48 48
 	}
49 49
 	if s.Build == nil {
50 50
 		return nil, false, nil
... ...
@@ -63,8 +63,15 @@ func NameFromImageStream(namespace string, ref *kapi.ObjectReference, tag string
63 63
 
64 64
 // IsBuildComplete returns whether the provided build is complete or not
65 65
 func IsBuildComplete(build *buildapi.Build) bool {
66
-	if build.Status.Phase != buildapi.BuildPhaseRunning && build.Status.Phase != buildapi.BuildPhasePending && build.Status.Phase != buildapi.BuildPhaseNew {
67
-		return true
66
+	return build.Status.Phase != buildapi.BuildPhaseRunning && build.Status.Phase != buildapi.BuildPhasePending && build.Status.Phase != buildapi.BuildPhaseNew
67
+}
68
+
69
+// GetBuildLabel retrieves build label from a Pod returning its value and
70
+// a boolean value informing whether the value was found
71
+func GetBuildLabel(pod *kapi.Pod) (string, bool) {
72
+	value, exists := pod.Labels[buildapi.BuildLabel]
73
+	if !exists {
74
+		value, exists = pod.Labels[buildapi.DeprecatedBuildLabel]
68 75
 	}
69
-	return false
76
+	return value, exists
70 77
 }
... ...
@@ -12,3 +12,41 @@ func TestGetBuildPodName(t *testing.T) {
12 12
 		t.Errorf("Expected %s, got %s", expected, actual)
13 13
 	}
14 14
 }
15
+
16
+func TestGetBuildLabel(t *testing.T) {
17
+	type getBuildLabelTest struct {
18
+		labels         map[string]string
19
+		expectedValue  string
20
+		expectedExists bool
21
+	}
22
+
23
+	tests := []getBuildLabelTest{
24
+		{
25
+			// 0 - new label
26
+			labels:         map[string]string{buildapi.BuildLabel: "value"},
27
+			expectedValue:  "value",
28
+			expectedExists: true,
29
+		},
30
+		{
31
+			// 1 - deprecated label
32
+			labels:         map[string]string{buildapi.DeprecatedBuildLabel: "value"},
33
+			expectedValue:  "value",
34
+			expectedExists: true,
35
+		},
36
+		{
37
+			// 2 - deprecated label
38
+			labels:         map[string]string{},
39
+			expectedValue:  "",
40
+			expectedExists: false,
41
+		},
42
+	}
43
+	for i, tc := range tests {
44
+		value, exists := GetBuildLabel(&kapi.Pod{ObjectMeta: kapi.ObjectMeta{Labels: tc.labels}})
45
+		if value != tc.expectedValue {
46
+			t.Errorf("(%d) unexpected value, expected %s, got %s", i, tc.expectedValue, value)
47
+		}
48
+		if exists != tc.expectedExists {
49
+			t.Errorf("(%d) unexpected exists flag, expected %v, got %v", i, tc.expectedExists, exists)
50
+		}
51
+	}
52
+}