@@ -50,11 +50,11 @@ func (c *ImageChangeController) Handle(stream *imageapi.ImageStream) error {

 				continue

 			}

-			// All initial deployments (latestVersion == 0) should have their images resolved in order

-			// to be able to work and not try to pull non-existent images from DockerHub.

-			// Deployments with automatic set to false that have been deployed at least once (latestVersion > 0)

-			// shouldn't have their images updated.

-			if !params.Automatic && config.Status.LatestVersion != 0 {

+			// All initial deployments should have their images resolved in order to

+			// be able to work and not try to pull non-existent images from DockerHub.

+			// Deployments with automatic set to false that have been deployed at least

+			// once shouldn't have their images updated.

+			if !params.Automatic && len(params.LastTriggeredImage) > 0 {

 				continue

 			}

@@ -52,6 +52,8 @@ func TestHandle_changeForNonAutomaticTag(t *testing.T) {

 			config := testapi.OkDeploymentConfig(1)

 			config.Namespace = kapi.NamespaceDefault

 			config.Spec.Triggers[0].ImageChangeParams.Automatic = false

+			// The image has been resolved at least once before.

+			config.Spec.Triggers[0].ImageChangeParams.LastTriggeredImage = testapi.DockerImageReference

 			return []*deployapi.DeploymentConfig{config}, nil

 		},

@@ -137,11 +139,14 @@ func TestHandle_changeForUnregisteredTag(t *testing.T) {

 // match) properly.

 func TestHandle_matchScenarios(t *testing.T) {

 	tests := []struct {

+		name string

+

 		param   *deployapi.DeploymentTriggerImageChangeParams

 		matches bool

 	}{

-		// Update from empty last image ID to a new one with explicit namespaces

 		{

+			name: "automatic=true, initial trigger, explicit namespace",

+

 			param: &deployapi.DeploymentTriggerImageChangeParams{

 				Automatic:          true,

 				ContainerNames:     []string{"container1"},

@@ -150,8 +155,9 @@ func TestHandle_matchScenarios(t *testing.T) {

 			},

 			matches: true,

 		},

-		// Update from empty last image ID to a new one with implicit namespaces

 		{

+			name: "automatic=true, initial trigger, implicit namespace",

+

 			param: &deployapi.DeploymentTriggerImageChangeParams{

 				Automatic:          true,

 				ContainerNames:     []string{"container1"},

@@ -160,18 +166,31 @@ func TestHandle_matchScenarios(t *testing.T) {

 			},

 			matches: true,

 		},

-		// Update from empty last image ID to a new one, but not marked automatic

 		{

+			name: "automatic=false, initial trigger",

+

 			param: &deployapi.DeploymentTriggerImageChangeParams{

 				Automatic:          false,

 				ContainerNames:     []string{"container1"},

 				From:               kapi.ObjectReference{Namespace: kapi.NamespaceDefault, Name: imageapi.JoinImageStreamTag(testapi.ImageStreamName, imageapi.DefaultImageTag)},

 				LastTriggeredImage: "",

 			},

+			matches: true,

+		},

+		{

+			name: "(no-op) automatic=false, already triggered",

+

+			param: &deployapi.DeploymentTriggerImageChangeParams{

+				Automatic:          false,

+				ContainerNames:     []string{"container1"},

+				From:               kapi.ObjectReference{Namespace: kapi.NamespaceDefault, Name: imageapi.JoinImageStreamTag(testapi.ImageStreamName, imageapi.DefaultImageTag)},

+				LastTriggeredImage: testapi.DockerImageReference,

+			},

 			matches: false,

 		},

-		// Updated image ID is equal to the last triggered ID

 		{

+			name: "(no-op) automatic=true, image is already deployed",

+

 			param: &deployapi.DeploymentTriggerImageChangeParams{

 				Automatic:          true,

 				ContainerNames:     []string{"container1"},

@@ -180,8 +199,9 @@ func TestHandle_matchScenarios(t *testing.T) {

 			},

 			matches: false,

 		},

-		// Trigger stream reference doesn't match

 		{

+			name: "(no-op) trigger doesn't match the stream",

+

 			param: &deployapi.DeploymentTriggerImageChangeParams{

 				Automatic:          true,

 				ContainerNames:     []string{"container1"},

@@ -192,13 +212,13 @@ func TestHandle_matchScenarios(t *testing.T) {

 		},

 	}

-	for i, test := range tests {

+	for _, test := range tests {

 		updated := false

 		fake := &testclient.Fake{}

 		fake.AddReactor("update", "deploymentconfigs", func(action ktestclient.Action) (handled bool, ret runtime.Object, err error) {

 			if !test.matches {

-				t.Fatalf("unexpected deploymentconfig update for scenario %d", i)

+				t.Fatal("unexpected deploymentconfig update")

 			}

 			updated = true

 			return true, nil, nil

@@ -220,15 +240,15 @@ func TestHandle_matchScenarios(t *testing.T) {

 			client: fake,

 		}

-		t.Logf("running scenario: %d", i)

+		t.Logf("running test %q", test.name)

 		stream := makeStream(testapi.ImageStreamName, imageapi.DefaultImageTag, testapi.DockerImageReference, testapi.ImageID)

 		if err := controller.Handle(stream); err != nil {

-			t.Fatalf("unexpected error for scenario %v: %v", i, err)

+			t.Fatalf("unexpected error: %v", err)

 		}

 		// assert updates occurred

 		if test.matches && !updated {

-			t.Fatalf("expected update for scenario: %v", test)

+			t.Fatal("expected an update")

 		}

 	}

 }

@@ -5,6 +5,7 @@ package integration

 import (

 	"fmt"

 	"testing"

+	"time"

 	kapi "k8s.io/kubernetes/pkg/api"

 	kclient "k8s.io/kubernetes/pkg/client/unversioned"

@@ -114,17 +115,17 @@ func TestTriggers_imageChange(t *testing.T) {

 	configWatch, err := openshiftProjectAdminClient.DeploymentConfigs(testutil.Namespace()).Watch(kapi.ListOptions{})

 	if err != nil {

-		t.Fatalf("Couldn't subscribe to Deployments %v", err)

+		t.Fatalf("Couldn't subscribe to deploymentconfigs %v", err)

 	}

 	defer configWatch.Stop()

 	if imageStream, err = openshiftProjectAdminClient.ImageStreams(testutil.Namespace()).Create(imageStream); err != nil {

-		t.Fatalf("Couldn't create ImageStream: %v", err)

+		t.Fatalf("Couldn't create imagestream: %v", err)

 	}

 	imageWatch, err := openshiftProjectAdminClient.ImageStreams(testutil.Namespace()).Watch(kapi.ListOptions{})

 	if err != nil {

-		t.Fatalf("Couldn't subscribe to ImageStreams: %s", err)

+		t.Fatalf("Couldn't subscribe to imagestreams: %v", err)

 	}

 	defer imageWatch.Stop()

@@ -147,29 +148,29 @@ func TestTriggers_imageChange(t *testing.T) {

 			t.Fatalf("unexpected error: %v", err)

 		}

-		t.Log("Waiting for image stream mapping to be reflected in the IS status...")

+		t.Log("Waiting for image stream mapping to be reflected in the image stream status...")

 	statusLoop:

 		for {

 			select {

 			case event := <-imageWatch.ResultChan():

 				stream := event.Object.(*imageapi.ImageStream)

 				if _, ok := stream.Status.Tags[imageapi.DefaultImageTag]; ok {

-					t.Logf("ImageStream %s now has Status with tags: %#v", stream.Name, stream.Status.Tags)

+					t.Logf("imagestream %q now has status with tags: %#v", stream.Name, stream.Status.Tags)

 					break statusLoop

 				}

-				t.Logf("Still waiting for latest tag status on ImageStream %s", stream.Name)

+				t.Logf("Still waiting for latest tag status on imagestream %q", stream.Name)

 			}

 		}

 	}

 	if config, err = openshiftProjectAdminClient.DeploymentConfigs(testutil.Namespace()).Create(config); err != nil {

-		t.Fatalf("Couldn't create DeploymentConfig: %v", err)

+		t.Fatalf("Couldn't create deploymentconfig: %v", err)

 	}

 	createTagEvent()

 	var newConfig *deployapi.DeploymentConfig

-	t.Log("Waiting for a new deployment config in response to ImageStream update")

+	t.Log("Waiting for a new deployment config in response to imagestream update")

 waitForNewConfig:

 	for {

 		select {

@@ -184,12 +185,155 @@ waitForNewConfig:

 					}

 					break waitForNewConfig

 				}

-				t.Log("Still waiting for a new deployment config in response to ImageStream update")

+				t.Log("Still waiting for a new deployment config in response to imagestream update")

 			}

 		}

 	}

 }

+func TestTriggers_imageChange_nonAutomatic(t *testing.T) {

+	testutil.RequireEtcd(t)

+	_, clusterAdminKubeConfig, err := testserver.StartTestMaster()

+	if err != nil {

+		t.Fatalf("error starting master: %v", err)

+	}

+	openshiftClusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)

+	if err != nil {

+		t.Fatalf("error getting cluster admin client: %v", err)

+	}

+	openshiftClusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)

+	if err != nil {

+		t.Fatalf("error getting cluster admin client config: %v", err)

+	}

+	openshiftProjectAdminClient, err := testserver.CreateNewProject(openshiftClusterAdminClient, *openshiftClusterAdminClientConfig, testutil.Namespace(), "bob")

+	if err != nil {

+		t.Fatalf("error creating project: %v", err)

+	}

+

+	imageStream := &imageapi.ImageStream{ObjectMeta: kapi.ObjectMeta{Name: deploytest.ImageStreamName}}

+

+	if imageStream, err = openshiftProjectAdminClient.ImageStreams(testutil.Namespace()).Create(imageStream); err != nil {

+		t.Fatalf("Couldn't create imagestream: %v", err)

+	}

+

+	imageWatch, err := openshiftProjectAdminClient.ImageStreams(testutil.Namespace()).Watch(kapi.ListOptions{})

+	if err != nil {

+		t.Fatalf("Couldn't subscribe to imagestreams: %v", err)

+	}

+	defer imageWatch.Stop()

+

+	image := fmt.Sprintf("sha256:%s", deploytest.ImageID)

+	pullSpec := fmt.Sprintf("registry:8080/%s/%s@%s", testutil.Namespace(), deploytest.ImageStreamName, image)

+	// Make a function which can create a new tag event for the image stream and

+	// then wait for the stream status to be asynchronously updated.

+	mapping := &imageapi.ImageStreamMapping{

+		ObjectMeta: kapi.ObjectMeta{Name: imageStream.Name},

+		Tag:        imageapi.DefaultImageTag,

+		Image: imageapi.Image{

+			ObjectMeta: kapi.ObjectMeta{

+				Name: image,

+			},

+			DockerImageReference: pullSpec,

+		},

+	}

+	updated := ""

+

+	createTagEvent := func(mapping *imageapi.ImageStreamMapping) {

+		if err := openshiftProjectAdminClient.ImageStreamMappings(testutil.Namespace()).Create(mapping); err != nil {

+			t.Fatalf("unexpected error: %v", err)

+		}

+

+		t.Log("Waiting for image stream mapping to be reflected in the image stream status...")

+

+		for {

+			select {

+			case event := <-imageWatch.ResultChan():

+				stream := event.Object.(*imageapi.ImageStream)

+				tagEventList, ok := stream.Status.Tags[imageapi.DefaultImageTag]

+				if ok {

+					if updated != tagEventList.Items[0].DockerImageReference {

+						updated = tagEventList.Items[0].DockerImageReference

+						return

+					}

+				}

+				t.Logf("Still waiting for latest tag status update on imagestream %q", stream.Name)

+			}

+		}

+	}

+

+	configWatch, err := openshiftProjectAdminClient.DeploymentConfigs(testutil.Namespace()).Watch(kapi.ListOptions{})

+	if err != nil {

+		t.Fatalf("Couldn't subscribe to deploymentconfigs: %v", err)

+	}

+	defer configWatch.Stop()

+

+	config := deploytest.OkDeploymentConfig(0)

+	config.Namespace = testutil.Namespace()

+	config.Spec.Triggers = []deployapi.DeploymentTriggerPolicy{deploytest.OkImageChangeTrigger()}

+	config.Spec.Triggers[0].ImageChangeParams.Automatic = false

+	if config, err = openshiftProjectAdminClient.DeploymentConfigs(testutil.Namespace()).Create(config); err != nil {

+		t.Fatalf("Couldn't create deploymentconfig: %v", err)

+	}

+

+	createTagEvent(mapping)

+

+	var newConfig *deployapi.DeploymentConfig

+	t.Log("Waiting for the initial deploymentconfig update in response to the imagestream update")

+

+	timeout := time.After(30 * time.Second)

+

+	// This is the initial deployment with automatic=false in its ICT - it should be updated to pullSpec

+out:

+	for {

+		select {

+		case event := <-configWatch.ResultChan():

+			if event.Type != watchapi.Modified {

+				continue

+			}

+

+			newConfig = event.Object.(*deployapi.DeploymentConfig)

+

+			if newConfig.Status.LatestVersion > 0 {

+				t.Fatalf("unexpected latestVersion update - the config has no config change trigger")

+			}

+

+			if e, a := updated, newConfig.Spec.Template.Spec.Containers[0].Image; e == a {

+				break out

+			}

+		case <-timeout:

+			t.Fatalf("timed out waiting for the image update to happen")

+		}

+	}

+

+	t.Log("Waiting for the second imagestream update - it shouldn't update the deploymentconfig")

+

+	// Subsequent updates to the image shouldn't update the pod template image

+	mapping.Image.Name = "sha256:thisupdatedimageshouldneverlandinthepodtemplate"

+	mapping.Image.DockerImageReference = fmt.Sprintf("registry:8080/%s/%s@%s", testutil.Namespace(), deploytest.ImageStreamName, mapping.Image.Name)

+	createTagEvent(mapping)

+

+	for {

+		select {

+		case event := <-configWatch.ResultChan():

+			if event.Type != watchapi.Modified {

+				continue

+			}

+

+			newConfig = event.Object.(*deployapi.DeploymentConfig)

+

+			if newConfig.Status.LatestVersion > 0 {

+				t.Fatalf("unexpected latestVersion update - the config has no config change trigger")

+			}

+

+			if e, a := updated, newConfig.Spec.Template.Spec.Containers[0].Image; e == a {

+				t.Fatalf("unexpected image update, expected initial image to be the same")

+			}

+		case <-timeout:

+			return

+		}

+	}

+}

+

 func TestTriggers_configChange(t *testing.T) {

 	const namespace = "test-triggers-configchange"