From 3153f8042f31328a19448e483dfa9e307b518669 Mon Sep 17 00:00:00 2001 From: Jim Minter Date: Tue, 31 Oct 2017 11:09:53 -0600 Subject: [PATCH 1/3] allow image trigger controller to create custom builds --- pkg/cmd/server/bootstrappolicy/controller_policy.go | 1 + test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml | 1 + test/testdata/bootstrappolicy/bootstrap_policy_file.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/pkg/cmd/server/bootstrappolicy/controller_policy.go b/pkg/cmd/server/bootstrappolicy/controller_policy.go index f219dd002e25..00582567b379 100644 --- a/pkg/cmd/server/bootstrappolicy/controller_policy.go +++ b/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(), diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml index 1cad6a44b02d..4e11c067fe45 100644 --- a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml @@ -3299,6 +3299,7 @@ items: - "" - build.openshift.io resources: + - builds/custom - builds/docker - builds/jenkinspipeline - builds/optimizeddocker diff --git a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml index f83e95fe40e0..d2ca40afb225 100644 --- a/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_policy_file.yaml @@ -3602,6 +3602,7 @@ items: - build.openshift.io attributeRestrictions: null resources: + - builds/custom - builds/docker - builds/jenkinspipeline - builds/optimizeddocker From bcf0a772bbf0ae7268f9f0b20529fd84d1029b19 Mon Sep 17 00:00:00 2001 From: Jim Minter Date: Tue, 31 Oct 2017 14:49:42 -0600 Subject: [PATCH 2/3] add event when build image trigger fails --- pkg/cmd/server/origin/controller/image.go | 2 +- .../trigger/image_trigger_controller_test.go | 8 ++--- .../trigger/buildconfigs/buildconfigs.go | 31 ++++++++++++++++--- .../trigger/buildconfigs/buildconfigs_test.go | 2 +- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/server/origin/controller/image.go b/pkg/cmd/server/origin/controller/image.go index 70ab2a1277fe..523ae883b9b5 100644 --- a/pkg/cmd/server/origin/controller/image.go +++ b/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 { diff --git a/pkg/image/controller/trigger/image_trigger_controller_test.go b/pkg/image/controller/trigger/image_trigger_controller_test.go index 3ce84cbea076..7a745d198103 100644 --- a/pkg/image/controller/trigger/image_trigger_controller_test.go +++ b/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, diff --git a/pkg/image/trigger/buildconfigs/buildconfigs.go b/pkg/image/trigger/buildconfigs/buildconfigs.go index 67157b6dbf57..3e4f3c1203c3 100644 --- a/pkg/image/trigger/buildconfigs/buildconfigs.go +++ b/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 } diff --git a/pkg/image/trigger/buildconfigs/buildconfigs_test.go b/pkg/image/trigger/buildconfigs/buildconfigs_test.go index 05db5da7bc07..effd0693922d 100644 --- a/pkg/image/trigger/buildconfigs/buildconfigs_test.go +++ b/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) From 9d9243a41d3c36f8a3fda969b396256d48126fc4 Mon Sep 17 00:00:00 2001 From: Jim Minter Date: Mon, 6 Nov 2017 16:09:24 -0600 Subject: [PATCH 3/3] verify that imagechangetriggers trigger all build types --- test/extended/builds/imagechangetriggers.go | 58 +++++++++ test/extended/testdata/bindata.go | 110 ++++++++++++++++++ .../builds/test-imagechangetriggers.yaml | 90 ++++++++++++++ 3 files changed, 258 insertions(+) create mode 100644 test/extended/builds/imagechangetriggers.go create mode 100644 test/extended/testdata/builds/test-imagechangetriggers.yaml diff --git a/test/extended/builds/imagechangetriggers.go b/test/extended/builds/imagechangetriggers.go new file mode 100644 index 000000000000..ad466caf7e5d --- /dev/null +++ b/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()) + }) + }) +}) diff --git a/test/extended/testdata/bindata.go b/test/extended/testdata/bindata.go index 5a8a87bf622f..ba0551a5880c 100644 --- a/test/extended/testdata/bindata.go +++ b/test/extended/testdata/bindata.go @@ -65,6 +65,7 @@ // test/extended/testdata/builds/test-docker-build.json // test/extended/testdata/builds/test-docker-no-outputname.json // test/extended/testdata/builds/test-env-build.json +// test/extended/testdata/builds/test-imagechangetriggers.yaml // test/extended/testdata/builds/test-imageresolution-custom-build.yaml // test/extended/testdata/builds/test-imageresolution-docker-build.yaml // test/extended/testdata/builds/test-imageresolution-s2i-build.yaml @@ -3285,6 +3286,113 @@ func testExtendedTestdataBuildsTestEnvBuildJson() (*asset, error) { return a, nil } +var _testExtendedTestdataBuildsTestImagechangetriggersYaml = []byte(`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 +`) + +func testExtendedTestdataBuildsTestImagechangetriggersYamlBytes() ([]byte, error) { + return _testExtendedTestdataBuildsTestImagechangetriggersYaml, nil +} + +func testExtendedTestdataBuildsTestImagechangetriggersYaml() (*asset, error) { + bytes, err := testExtendedTestdataBuildsTestImagechangetriggersYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "test/extended/testdata/builds/test-imagechangetriggers.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + var _testExtendedTestdataBuildsTestImageresolutionCustomBuildYaml = []byte(`apiVersion: v1 kind: List metadata: {} @@ -28465,6 +28573,7 @@ var _bindata = map[string]func() (*asset, error){ "test/extended/testdata/builds/test-docker-build.json": testExtendedTestdataBuildsTestDockerBuildJson, "test/extended/testdata/builds/test-docker-no-outputname.json": testExtendedTestdataBuildsTestDockerNoOutputnameJson, "test/extended/testdata/builds/test-env-build.json": testExtendedTestdataBuildsTestEnvBuildJson, + "test/extended/testdata/builds/test-imagechangetriggers.yaml": testExtendedTestdataBuildsTestImagechangetriggersYaml, "test/extended/testdata/builds/test-imageresolution-custom-build.yaml": testExtendedTestdataBuildsTestImageresolutionCustomBuildYaml, "test/extended/testdata/builds/test-imageresolution-docker-build.yaml": testExtendedTestdataBuildsTestImageresolutionDockerBuildYaml, "test/extended/testdata/builds/test-imageresolution-s2i-build.yaml": testExtendedTestdataBuildsTestImageresolutionS2iBuildYaml, @@ -28868,6 +28977,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ "test-docker-build.json": &bintree{testExtendedTestdataBuildsTestDockerBuildJson, map[string]*bintree{}}, "test-docker-no-outputname.json": &bintree{testExtendedTestdataBuildsTestDockerNoOutputnameJson, map[string]*bintree{}}, "test-env-build.json": &bintree{testExtendedTestdataBuildsTestEnvBuildJson, map[string]*bintree{}}, + "test-imagechangetriggers.yaml": &bintree{testExtendedTestdataBuildsTestImagechangetriggersYaml, map[string]*bintree{}}, "test-imageresolution-custom-build.yaml": &bintree{testExtendedTestdataBuildsTestImageresolutionCustomBuildYaml, map[string]*bintree{}}, "test-imageresolution-docker-build.yaml": &bintree{testExtendedTestdataBuildsTestImageresolutionDockerBuildYaml, map[string]*bintree{}}, "test-imageresolution-s2i-build.yaml": &bintree{testExtendedTestdataBuildsTestImageresolutionS2iBuildYaml, map[string]*bintree{}}, diff --git a/test/extended/testdata/builds/test-imagechangetriggers.yaml b/test/extended/testdata/builds/test-imagechangetriggers.yaml new file mode 100644 index 000000000000..f4e0794afbfb --- /dev/null +++ b/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