Merge pull request #17108 from jim-minter/issue17074

Automatic merge from submit-queue.

allow image trigger controller to create custom builds

fixes #17074

pkg/cmd/server/bootstrappolicy/controller_policy.go

@@ -213,6 +213,7 @@ func init() {
 			rbac.NewRule("create").Groups(buildGroup, legacyBuildGroup).Resources(
 				authorizationapi.SourceBuildResource,
 				authorizationapi.DockerBuildResource,
+				authorizationapi.CustomBuildResource,
 				authorizationapi.OptimizedDockerBuildResource,
 				authorizationapi.JenkinsPipelineBuildResource,
 			).RuleOrDie(),

pkg/cmd/server/origin/controller/image.go

@@ -68,7 +68,7 @@ func (c *ImageTriggerControllerConfig) RunController(ctx ControllerContext) (boo
 			Informer:  ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer(),
 			Store:     ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer().GetIndexer(),
 			TriggerFn: triggerbuildconfigs.NewBuildConfigTriggerIndexer,
-			Reactor:   &triggerbuildconfigs.BuildConfigReactor{Instantiator: bcInstantiator},
+			Reactor:   triggerbuildconfigs.NewBuildConfigReactor(bcInstantiator, kclient.Core().RESTClient()),
 		})
 	}
 	if !c.HasDeploymentsEnabled {

pkg/image/controller/trigger/image_trigger_controller_test.go

@@ -297,9 +297,7 @@ func TestTriggerControllerSyncBuildConfigResource(t *testing.T) {
 			},
 		}
 		inst := fakeBuildConfigInstantiator(test.bc, test.is)
-		reaction := &buildconfigs.BuildConfigReactor{
-			Instantiator: inst,
-		}
+		reaction := buildconfigs.NewBuildConfigReactor(inst, nil)
 		controller := TriggerController{
 			triggerCache: NewTriggerCache(),
 			lister:       lister,
@@ -398,9 +396,7 @@ func TestTriggerControllerSyncBuildConfigResourceErrorHandling(t *testing.T) {
 		if test.err != nil {
 			inst.err = test.err
 		}
-		reaction := &buildconfigs.BuildConfigReactor{
-			Instantiator: inst,
-		}
+		reaction := buildconfigs.NewBuildConfigReactor(inst, nil)
 		controller := TriggerController{
 			triggerCache: NewTriggerCache(),
 			lister:       lister,

pkg/image/trigger/buildconfigs/buildconfigs.go

@@ -1,10 +1,16 @@
 package buildconfigs
 
 import (
+	"fmt"
 	"reflect"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
+	clientv1 "k8s.io/client-go/pkg/api/v1"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/tools/record"
 	kapi "k8s.io/kubernetes/pkg/api"
 
 	"github.com/golang/glog"
@@ -107,15 +113,25 @@ type BuildConfigInstantiator interface {
 	Instantiate(namespace string, request *buildapi.BuildRequest) (*buildapi.Build, error)
 }
 
-// BuildConfigReactor converts trigger changes into new builds. It will request a build if
+// buildConfigReactor converts trigger changes into new builds. It will request a build if
 // at least one image is out of date.
-type BuildConfigReactor struct {
-	Instantiator BuildConfigInstantiator
+type buildConfigReactor struct {
+	instantiator  BuildConfigInstantiator
+	eventRecorder record.EventRecorder
+}
+
+// NewBuildConfigReactor creates a new buildConfigReactor
+func NewBuildConfigReactor(instantiator BuildConfigInstantiator, restclient rest.Interface) trigger.ImageReactor {
+	eventBroadcaster := record.NewBroadcaster()
+	eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(restclient).Events("")})
+	eventRecorder := eventBroadcaster.NewRecorder(kapi.Scheme, clientv1.EventSource{Component: "buildconfig-controller"})
+
+	return &buildConfigReactor{instantiator: instantiator, eventRecorder: eventRecorder}
 }
 
 // ImageChanged is passed a build config and a set of changes and updates the object if
 // necessary.
-func (r *BuildConfigReactor) ImageChanged(obj interface{}, tagRetriever trigger.TagRetriever) error {
+func (r *buildConfigReactor) ImageChanged(obj interface{}, tagRetriever trigger.TagRetriever) error {
 	bc := obj.(*buildapi.BuildConfig)
 
 	var request *buildapi.BuildRequest
@@ -193,6 +209,11 @@ func (r *BuildConfigReactor) ImageChanged(obj interface{}, tagRetriever trigger.
 
 	// instantiate new build
 	glog.V(4).Infof("Requesting build for BuildConfig based on image triggers %s/%s: %#v", bc.Namespace, bc.Name, request)
-	_, err := r.Instantiator.Instantiate(bc.Namespace, request)
+	_, err := r.instantiator.Instantiate(bc.Namespace, request)
+	if err != nil {
+		instantiateErr := fmt.Errorf("error triggering Build for BuildConfig %s/%s: %v", bc.Namespace, bc.Name, err)
+		utilruntime.HandleError(instantiateErr)
+		r.eventRecorder.Event(bc, kapi.EventTypeWarning, "BuildConfigTriggerFailed", instantiateErr.Error())
+	}
 	return err
 }

pkg/image/trigger/buildconfigs/buildconfigs_test.go

@@ -246,7 +246,7 @@ func TestBuildConfigReactor(t *testing.T) {
 
 	for i, test := range testCases {
 		instantiator := &instantiator{build: test.response}
-		r := BuildConfigReactor{Instantiator: instantiator}
+		r := buildConfigReactor{instantiator: instantiator}
 		initial, err := kapi.Scheme.DeepCopy(test.obj)
 		if err != nil {
 			t.Fatal(err)

test/extended/builds/imagechangetriggers.go

@@ -0,0 +1,58 @@
+package builds
+
+import (
+	"time"
+
+	g "github.com/onsi/ginkgo"
+	o "github.com/onsi/gomega"
+
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
+
+	exutil "github.com/openshift/origin/test/extended/util"
+)
+
+var _ = g.Describe("[Feature:Builds][Conformance] imagechangetriggers", func() {
+	defer g.GinkgoRecover()
+
+	var (
+		buildFixture = exutil.FixturePath("testdata", "builds", "test-imagechangetriggers.yaml")
+		oc           = exutil.NewCLI("imagechangetriggers", exutil.KubeConfigPath())
+	)
+
+	g.Context("", func() {
+		g.JustBeforeEach(func() {
+			g.By("waiting for builder service account")
+			err := exutil.WaitForBuilderAccount(oc.AdminKubeClient().Core().ServiceAccounts(oc.Namespace()))
+			o.Expect(err).NotTo(o.HaveOccurred())
+		})
+
+		g.AfterEach(func() {
+			if g.CurrentGinkgoTestDescription().Failed {
+				exutil.DumpPodStates(oc)
+				exutil.DumpPodLogsStartingWith("", oc)
+			}
+		})
+
+		g.It("imagechangetriggers should trigger builds of all types", func() {
+			err := oc.AsAdmin().Run("create").Args("-f", buildFixture).Execute()
+			o.Expect(err).NotTo(o.HaveOccurred())
+
+			err = wait.Poll(time.Second, 30*time.Second, func() (done bool, err error) {
+				for _, build := range []string{"bc-docker-1", "bc-jenkins-1", "bc-source-1", "bc-custom-1"} {
+					_, err := oc.BuildClient().Build().Builds(oc.Namespace()).Get(build, metav1.GetOptions{})
+					if err == nil {
+						continue
+					}
+					if kerrors.IsNotFound(err) {
+						return false, nil
+					}
+					return false, err
+				}
+				return true, nil
+			})
+			o.Expect(err).NotTo(o.HaveOccurred())
+		})
+	})
+})

test/extended/testdata/builds/test-imagechangetriggers.yaml

@@ -0,0 +1,90 @@
+kind: List
+apiVersion: v1
+items:
+- kind: ImageStream
+  apiVersion: v1
+  metadata:
+    name: nodejs-ex
+  spec:
+    tags:
+    - name: latest
+      from:
+        kind: DockerImage
+        name: centos/nodejs-6-centos7:latest
+
+- kind: BuildConfig
+  apiVersion: v1
+  metadata:
+    name: bc-source
+  spec:
+    source:
+      type: Git
+      git:
+        uri: https://github.com/openshift/nodejs-ex.git
+    strategy:
+      type: Source
+      sourceStrategy:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+    triggers:
+    - type: ImageChange
+      imageChange:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+
+- kind: BuildConfig
+  apiVersion: v1
+  metadata:
+    name: bc-docker
+  spec:
+    source:
+      type: Dockerfile
+      dockerfile: FROM nodejs
+    strategy:
+      type: Docker
+      dockerStrategy:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+    triggers:
+    - type: ImageChange
+      imageChange:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+
+- kind: BuildConfig
+  apiVersion: v1
+  metadata:
+    name: bc-custom
+  spec:
+    strategy:
+      type: Custom
+      customStrategy:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+    triggers:
+    - type: ImageChange
+      imageChange:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest
+
+- kind: BuildConfig
+  apiVersion: v1
+  metadata:
+    name: bc-jenkins
+  spec:
+    strategy:
+      type: Jenkins
+      jenkinsPipelineStrategy:
+        jenkinsfile: node {}
+    triggers:
+    - type: ImageChange
+      imageChange:
+        from:
+          kind: ImageStreamTag
+          name: nodejs-ex:latest