[KOGITO-1771] Checking build status before triggering a new build

pkg/client/openshift/buildconfig.go

@@ -154,9 +154,9 @@ func (b *buildConfig) checkBuildConfigExists(bc *buildv1.BuildConfig) (bool, err
 }
 
 // newBuildRequest creates a new BuildRequest for the build
-func newBuildRequest(triggedby string, bc *buildv1.BuildConfig) buildv1.BuildRequest {
+func newBuildRequest(triggeredBy string, bc *buildv1.BuildConfig) buildv1.BuildRequest {
 	buildRequest := buildv1.BuildRequest{ObjectMeta: metav1.ObjectMeta{Name: bc.Name}}
-	buildRequest.TriggeredBy = []buildv1.BuildTriggerCause{{Message: fmt.Sprintf("Triggered by %s operator", triggedby)}}
+	buildRequest.TriggeredBy = []buildv1.BuildTriggerCause{{Message: fmt.Sprintf("Triggered by %s operator", triggeredBy)}}
 	meta.SetGroupVersionKind(&buildRequest.TypeMeta, meta.KindBuildRequest)
 	return buildRequest
 }

pkg/controller/kogitoapp/build/trigger.go

@@ -0,0 +1,107 @@
+// Copyright 2020 Red Hat, Inc. and/or its affiliates
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package build
+
+import (
+	"fmt"
+	"github.com/kiegroup/kogito-cloud-operator/pkg/apis/app/v1alpha1"
+	"github.com/kiegroup/kogito-cloud-operator/pkg/client"
+	"github.com/kiegroup/kogito-cloud-operator/pkg/client/openshift"
+	"github.com/kiegroup/kogito-cloud-operator/pkg/util"
+	buildv1 "github.com/openshift/api/build/v1"
+)
+
+const (
+	buildConfigLabelSelector = "buildconfig"
+)
+
+// Trigger defines how to interact with build triggers in a OpenShift cluster
+type Trigger interface {
+	// SelectOneBuildConfigWithLabel restricts the trigger only on the first build with the given labels
+	SelectOneBuildConfigWithLabel(labelSelector map[string]string, buildConfigs ...*buildv1.BuildConfig) Trigger
+	// OnBuildConfig defines the build configuration to trigger the build.
+	// Another approach is using `SelectOneBuildConfigWithLabel` to select the build to trigger
+	OnBuildConfig(buildConfig *buildv1.BuildConfig) Trigger
+	// StartNewBuildIfNotRunning starts a new build for the filtered build config
+	StartNewBuildIfNotRunning() (result TriggerResult, err error)
+	// HasBuildConfiguration returns true if a build configuration was selected for this instance
+	HasBuildConfiguration() bool
+}
+
+// TriggerResult structure to hold build trigger information
+type TriggerResult struct {
+	Started   bool
+	BuildName string
+}
+
+type trigger struct {
+	triggeredBy string
+	client      *client.Client
+	buildConfig *buildv1.BuildConfig
+}
+
+func (t *trigger) HasBuildConfiguration() bool {
+	return t.buildConfig != nil
+}
+
+func (t *trigger) OnBuildConfig(buildConfig *buildv1.BuildConfig) Trigger {
+	t.buildConfig = buildConfig
+	return t
+}
+
+func (t *trigger) SelectOneBuildConfigWithLabel(labelSelector map[string]string, buildConfigs ...*buildv1.BuildConfig) Trigger {
+	for _, bc := range buildConfigs {
+		if util.MapContainsMap(bc.Labels, labelSelector) {
+			t.buildConfig = bc
+			return t
+		}
+	}
+	return t
+}
+
+func (t *trigger) StartNewBuildIfNotRunning() (result TriggerResult, err error) {
+	result = TriggerResult{
+		Started:   false,
+		BuildName: "",
+	}
+	if t.buildConfig == nil {
+		return
+	}
+	result.BuildName = t.buildConfig.Name
+	builds, err := openshift.BuildConfigC(t.client).GetBuildsStatus(t.buildConfig, fmt.Sprintf("%s=%s", buildConfigLabelSelector, t.buildConfig.Name))
+	if err != nil {
+		return
+	}
+	if !isBuildRunning(builds) {
+		if _, err = openshift.BuildConfigC(t.client).TriggerBuild(t.buildConfig, t.triggeredBy); err != nil {
+			return
+		}
+		result.Started = true
+	}
+	return
+}
+
+func isBuildRunning(builds *v1alpha1.Builds) bool {
+	return builds != nil && (len(builds.Running) > 0 || len(builds.Pending) > 0 || len(builds.New) > 0)
+}
+
+// NewTrigger creates a new build Trigger
+func NewTrigger(client *client.Client, triggeredBy string) Trigger {
+	return &trigger{
+		triggeredBy: triggeredBy,
+		client:      client,
+		buildConfig: nil,
+	}
+}

pkg/controller/kogitoapp/kogitoapp_controller.go

@@ -16,6 +16,7 @@ package kogitoapp
 
 import (
 	"fmt"
+	"github.com/kiegroup/kogito-cloud-operator/pkg/controller/kogitoapp/build"
 	"reflect"
 	"time"
 
@@ -248,9 +249,6 @@ func (r *ReconcileKogitoApp) Reconcile(request reconcile.Request) (result reconc
 		bcDelta := deltas[reflect.TypeOf(obuildv1.BuildConfig{})]
 		if bcDelta.HasChanges() {
 			var bcs []*obuildv1.BuildConfig
-			for _, bc := range bcDelta.Added {
-				bcs = append(bcs, bc.(*obuildv1.BuildConfig))
-			}
 			for _, bc := range bcDelta.Updated {
 				bcs = append(bcs, bc.(*obuildv1.BuildConfig))
 			}
@@ -287,29 +285,33 @@ func (r *ReconcileKogitoApp) updateKogitoAppStatus(request *reconcile.Request, i
 }
 
 func (r *ReconcileKogitoApp) triggerBuilds(instance *v1alpha1.KogitoApp, bcs ...*obuildv1.BuildConfig) error {
-	bcMap := map[string][]obuildv1.BuildConfig{}
-	for _, bc := range bcs {
-		bcMap[bc.Labels[kogitores.LabelKeyBuildType]] = append(bcMap[bc.Labels[kogitores.LabelKeyBuildType]], *bc)
+	trigger := build.NewTrigger(r.client, instance.Name).
+		SelectOneBuildConfigWithLabel(map[string]string{
+			kogitores.LabelKeyBuildType:    string(kogitores.BuildTypeS2I),
+			kogitores.LabelKeyBuildVariant: string(kogitores.BuildVariantSource),
+		}, bcs...)
+	result, err := trigger.StartNewBuildIfNotRunning()
+	if err != nil {
+		return err
+	}
+	if result.Started {
+		log.Infof("Triggered build named %s for Kogito Application %s", result.BuildName, instance.Name)
 	}
 
-	// Trigger only the S2I builds, they will trigger runtime builds
-	if bcMap[string(kogitores.BuildTypeS2I)] != nil {
-		for _, bc := range bcMap[string(kogitores.BuildTypeS2I)] {
-			log.Infof("Buildconfigs are created, triggering build %s", bc.GetName())
-			if _, err := openshift.BuildConfigC(r.client).TriggerBuild(&bc, instance.Name); err != nil {
-				return err
-			}
+	// we don't have a s2i build, let's trigger the build source from image since it could be changed by an image stream
+	// buildType here's Runtime
+	if !trigger.HasBuildConfiguration() {
+		result, err = trigger.
+			SelectOneBuildConfigWithLabel(map[string]string{kogitores.LabelKeyBuildVariant: string(kogitores.BuildVariantSource)}, bcs...).
+			StartNewBuildIfNotRunning()
+		if err != nil {
+			return err
 		}
-	} else {
-		for _, bc := range bcs {
-			if bc.Labels[kogitores.LabelKeyBuildVariant] == string(kogitores.BuildVariantSource) {
-				log.Infof("Buildconfigs are created, triggering build %s", bc.GetName())
-				if _, err := openshift.BuildConfigC(r.client).TriggerBuild(bc, instance.Name); err != nil {
-					return err
-				}
-			}
+		if result.Started {
+			log.Infof("Triggered build named %s for Kogito Application %s", result.BuildName, instance.Name)
 		}
 	}
+
 	return nil
 }
 

pkg/controller/kogitoapp/kogitoapp_controller_test.go

@@ -599,6 +599,37 @@ func TestReconcileKogitoApp_CustomVersionAndCustomTag(t *testing.T) {
 	assert.NotNil(t, hasIs)
 }
 
+func TestReconcileKogitoApp_BinaryBuilds(t *testing.T) {
+	kogitoApp := createFakeKogitoApp()
+	kogitoApp.Spec.Build.GitSource = v1alpha1.GitSource{}
+	fakeClient := test.CreateFakeClient([]runtime.Object{kogitoApp}, nil, []runtime.Object{})
+	fakeCache := &cachev1.FakeInformers{}
+	r := &ReconcileKogitoApp{
+		client: fakeClient,
+		scheme: meta.GetRegisteredSchema(),
+		cache:  fakeCache,
+	}
+	req := reconcile.Request{
+		NamespacedName: types.NamespacedName{
+			Name:      kogitoApp.Name,
+			Namespace: kogitoApp.Namespace,
+		},
+	}
+
+	result, err := r.Reconcile(req)
+	assert.NoError(t, err)
+	assert.False(t, result.Requeue)
+
+	exists, err := kubernetes.ResourceC(fakeClient).Fetch(kogitoApp)
+	assert.NoError(t, err)
+	assert.True(t, exists)
+
+	bc := &buildv1.BuildConfig{ObjectMeta: metav1.ObjectMeta{Name: kogitoApp.Name + "-binary", Namespace: kogitoApp.Namespace}}
+	exists, err = kubernetes.ResourceC(fakeClient).Fetch(bc)
+	assert.NoError(t, err)
+	assert.True(t, exists)
+}
+
 type mockCache struct {
 	cache.Cache
 	kogitoApp *v1alpha1.KogitoApp

pkg/util/arrays.go

@@ -14,13 +14,6 @@
 
 package util
 
-import (
-	"fmt"
-	"strings"
-)
-
-const keyPairSeparator = "="
-
 // Contains checks if the s string are within the array
 func Contains(s string, array []string) bool {
 	if len(s) == 0 {
@@ -34,48 +27,6 @@ func Contains(s string, array []string) bool {
 	return false
 }
 
-// FromStringsKeyPairToMap converts a string array in the key/pair format (key=value) to a map. Unconvertable strings will be skipped.
-func FromStringsKeyPairToMap(array []string) map[string]string {
-	if array == nil || len(array) == 0 {
-		return nil
-	}
-	keyPairMap := map[string]string{}
-	for _, item := range array {
-		keyPair := strings.SplitN(item, keyPairSeparator, 2)
-		if len(keyPair) == 0 {
-			break
-		}
-
-		if len(keyPair[0]) == 0 {
-			break
-		}
-
-		if len(keyPair) == 2 {
-			keyPairMap[keyPair[0]] = keyPair[1]
-		} else if len(keyPair) == 1 {
-			keyPairMap[keyPair[0]] = ""
-		}
-	}
-	return keyPairMap
-}
-
-// ParseStringsForKeyPair will parse the given string array for a valid key=pair format on each item.
-// Returns an error if any item is not in the valid format.
-func ParseStringsForKeyPair(array []string) error {
-	if array == nil || len(array) == 0 {
-		return nil
-	}
-	for _, item := range array {
-		if !strings.Contains(item, keyPairSeparator) {
-			return fmt.Errorf("Item %s does not contain the key/pair separator '%s'", item, keyPairSeparator)
-		}
-		if strings.HasPrefix(item, keyPairSeparator) {
-			return fmt.Errorf("Item %s starts with key/pair separator '%s'", item, keyPairSeparator)
-		}
-	}
-	return nil
-}
-
 // ArrayToSet converts an array of string to a set
 func ArrayToSet(array []string) map[string]bool {
 	set := make(map[string]bool, len(array))

pkg/util/arrays_test.go

@@ -19,32 +19,6 @@ import (
 	"testing"
 )
 
-func TestFromStringsKeyPairToMap(t *testing.T) {
-	type args struct {
-		array []string
-	}
-	tests := []struct {
-		name string
-		args args
-		want map[string]string
-	}{
-		{"happy path", args{[]string{"env1=value", "env2=value"}}, map[string]string{"env1": "value", "env2": "value"}},
-		{"happy path2", args{[]string{"env1=value"}}, map[string]string{"env1": "value"}},
-		{"only key", args{[]string{"env1="}}, map[string]string{"env1": ""}},
-		{"only key without sep", args{[]string{"env1"}}, map[string]string{"env1": ""}},
-		{"no key no value", args{[]string{""}}, map[string]string{}},
-		{"no key with value", args{[]string{"=value"}}, map[string]string{}},
-		{"various seps", args{[]string{"env1=value=value"}}, map[string]string{"env1": "value=value"}},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := FromStringsKeyPairToMap(tt.args.array); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("FromStringsKeyPairToMap() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestArrayToSet(t *testing.T) {
 	type args struct {
 		array []string

pkg/util/envs.go

@@ -37,11 +37,3 @@ func GetOSEnv(key, fallback string) string {
 	}
 	return value
 }
-
-// GetHomeDir gets the user home directory
-func GetHomeDir() string {
-	if h := os.Getenv("HOME"); h != "" {
-		return h
-	}
-	return os.Getenv("USERPROFILE") // windows
-}