From 855544b5d728dd16a34b73f75ce43c739e79542e Mon Sep 17 00:00:00 2001 From: Gaelle Fournier Date: Mon, 12 Feb 2024 14:22:03 +0100 Subject: [PATCH] feat(build): Add Build waiting condition * Add on Build.Status a "Scheduled" condition containing the reason for it's scheduling status Ref #4542 --- e2e/builder/build_test.go | 70 +++++ pkg/apis/camel/v1/build_types.go | 8 + pkg/controller/build/build_monitor.go | 21 ++ pkg/controller/build/build_monitor_test.go | 283 +++++++++++++++------ pkg/controller/build/schedule.go | 14 +- 5 files changed, 320 insertions(+), 76 deletions(-) diff --git a/e2e/builder/build_test.go b/e2e/builder/build_test.go index f449f0ef23..0111954c6f 100644 --- a/e2e/builder/build_test.go +++ b/e2e/builder/build_test.go @@ -32,6 +32,7 @@ import ( . "github.com/apache/camel-k/v2/e2e/support" v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1" + corev1 "k8s.io/api/core/v1" ) type kitOptions struct { @@ -130,11 +131,34 @@ func TestKitMaxBuildLimit(t *testing.T) { // verify that all builds are successful Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) + Eventually(BuildConditions(ns, buildA), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) Eventually(KitPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns1, buildB), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns1, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns1, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns2, buildC), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns2, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns2, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) }) }) }) @@ -207,10 +231,33 @@ func TestKitMaxBuildLimitFIFOStrategy(t *testing.T) { // verify that all builds are successful Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildA), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + Eventually(BuildPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildB), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + Eventually(BuildPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildC), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) }) } @@ -281,10 +328,33 @@ func TestKitMaxBuildLimitDependencyMatchingStrategy(t *testing.T) { // verify that all builds are successful Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildA), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildA)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + Eventually(BuildPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildB), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildB)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + Eventually(BuildPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildConditions(ns, buildC), TestTimeoutLong).ShouldNot(BeNil()) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) }) } diff --git a/pkg/apis/camel/v1/build_types.go b/pkg/apis/camel/v1/build_types.go index 188aa0ad14..7c1dc4e43b 100644 --- a/pkg/apis/camel/v1/build_types.go +++ b/pkg/apis/camel/v1/build_types.go @@ -256,6 +256,14 @@ const ( BuildPhaseInterrupted = "Interrupted" // BuildPhaseError -- . BuildPhaseError BuildPhase = "Error" + + // BuildConditionScheduled --. + BuildConditionScheduled BuildConditionType = "Scheduled" + + // BuildConditionReadyReason --. + BuildConditionReadyReason string = "Ready" + // BuildConditionWaitingReason --. + BuildConditionWaitingReason string = "Waiting" ) // +genclient diff --git a/pkg/controller/build/build_monitor.go b/pkg/controller/build/build_monitor.go index 2d45b2223e..8a9e83b195 100644 --- a/pkg/controller/build/build_monitor.go +++ b/pkg/controller/build/build_monitor.go @@ -22,6 +22,7 @@ import ( "fmt" "sync" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/types" @@ -56,6 +57,16 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui if runningBuildsTotal >= bm.maxRunningBuilds { Log.WithValues("request-namespace", requestNamespace, "request-name", requestName, "max-running-builds-limit", runningBuildsTotal). ForBuild(build).Infof("Maximum number of running builds (%d) exceeded - the build (%s) gets enqueued", runningBuildsTotal, build.Name) + build.Status.SetCondition( + v1.BuildConditionScheduled, + corev1.ConditionFalse, + v1.BuildConditionWaitingReason, + fmt.Sprintf( + "Maximum number of running builds (%d) exceeded - the build (%s) gets enqueued", + runningBuildsTotal, + build.Name, + ), + ) // max number of running builds limit exceeded return false, nil @@ -117,6 +128,16 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui if !allowed { Log.WithValues("request-namespace", requestNamespace, "request-name", requestName, "order-strategy", bm.buildOrderStrategy). ForBuild(build).Infof("%s - the build (%s) gets enqueued", reason, build.Name) + build.Status.SetCondition( + v1.BuildConditionScheduled, + corev1.ConditionFalse, + v1.BuildConditionWaitingReason, + fmt.Sprintf( + "%s - the build (%s) gets enqueued", + reason, + build.Name, + ), + ) } return allowed, nil diff --git a/pkg/controller/build/build_monitor_test.go b/pkg/controller/build/build_monitor_test.go index 9f1c051b96..321f6d5a7f 100644 --- a/pkg/controller/build/build_monitor_test.go +++ b/pkg/controller/build/build_monitor_test.go @@ -26,31 +26,35 @@ import ( "github.com/apache/camel-k/v2/pkg/util/test" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) func TestMonitorSequentialBuilds(t *testing.T) { testcases := []struct { - name string - running []*v1.Build - finished []*v1.Build - build *v1.Build - allowed bool + name string + running []*v1.Build + finished []*v1.Build + build *v1.Build + allowed bool + condition *v1.BuildCondition }{ { - name: "allowNewBuild", - running: []*v1.Build{}, - finished: []*v1.Build{}, - build: newBuild("ns", "my-build"), - allowed: true, + name: "allowNewBuild", + running: []*v1.Build{}, + finished: []*v1.Build{}, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { - name: "allowNewNativeBuild", - running: []*v1.Build{}, - finished: []*v1.Build{}, - build: newNativeBuild("ns", "my-native-build"), - allowed: true, + name: "allowNewNativeBuild", + running: []*v1.Build{}, + finished: []*v1.Build{}, + build: newNativeBuild("ns", "my-native-build"), + allowed: true, + condition: nil, }, { name: "allowNewBuildWhenOthersFinished", @@ -59,8 +63,9 @@ func TestMonitorSequentialBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "allowNewNativeBuildWhenOthersFinished", @@ -69,8 +74,9 @@ func TestMonitorSequentialBuilds(t *testing.T) { newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newNativeBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), }, - build: newNativeBuild("ns", "my-native-build"), - allowed: true, + build: newNativeBuild("ns", "my-native-build"), + allowed: true, + condition: nil, }, { name: "limitMaxRunningBuilds", @@ -84,6 +90,12 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "limitMaxRunningNativeBuilds", @@ -97,14 +109,21 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "allowParallelBuildsWithDifferentLayout", running: []*v1.Build{ newNativeBuildInPhase("ns", "my-build-1", v1.BuildPhaseRunning), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "queueBuildsInSameNamespaceWithSameLayout", @@ -117,6 +136,12 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Found a running build in this namespace - the build (my-build) gets enqueued", + }, }, { name: "allowBuildsInNewNamespace", @@ -127,8 +152,9 @@ func TestMonitorSequentialBuilds(t *testing.T) { finished: []*v1.Build{ newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, } @@ -158,6 +184,14 @@ func TestMonitorSequentialBuilds(t *testing.T) { assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + if tc.condition == nil { + assert.Nil(t, tc.build.Status.GetCondition(v1.BuildConditionScheduled)) + } else { + assert.Equal(t, tc.condition.Type, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Type) + assert.Equal(t, tc.condition.Status, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Status) + assert.Equal(t, tc.condition.Reason, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Reason) + assert.Equal(t, tc.condition.Message, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Message) + } }) } } @@ -195,25 +229,28 @@ func TestAllowBuildRequeue(t *testing.T) { func TestMonitorFIFOBuilds(t *testing.T) { testcases := []struct { - name string - running []*v1.Build - builds []*v1.Build - build *v1.Build - allowed bool + name string + running []*v1.Build + builds []*v1.Build + build *v1.Build + allowed bool + condition *v1.BuildCondition }{ { - name: "allowNewBuild", - running: []*v1.Build{}, - builds: []*v1.Build{}, - build: newBuild("ns", "my-build"), - allowed: true, + name: "allowNewBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { - name: "allowNewNativeBuild", - running: []*v1.Build{}, - builds: []*v1.Build{}, - build: newNativeBuild("ns1", "my-build"), - allowed: true, + name: "allowNewNativeBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newNativeBuild("ns1", "my-build"), + allowed: true, + condition: nil, }, { name: "allowNewBuildWhenOthersFinished", @@ -222,8 +259,9 @@ func TestMonitorFIFOBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "allowNewNativeBuildWhenOthersFinished", @@ -232,8 +270,9 @@ func TestMonitorFIFOBuilds(t *testing.T) { newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newNativeBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), }, - build: newNativeBuild("ns", "my-build"), - allowed: true, + build: newNativeBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "limitMaxRunningBuilds", @@ -247,6 +286,12 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "limitMaxRunningNativeBuilds", @@ -260,14 +305,21 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "allowParallelBuildsWithDifferentLayout", running: []*v1.Build{ newNativeBuildInPhase("ns", "my-build-1", v1.BuildPhaseRunning), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "allowParallelBuildsInSameNamespace", @@ -279,8 +331,9 @@ func TestMonitorFIFOBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newBuildInPhase("other-ns", "my-build-new", v1.BuildPhaseScheduling), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, { name: "queueBuildWhenOlderBuildIsAlreadyInitialized", @@ -294,6 +347,12 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-new) because it has been created before - the build (my-build) gets enqueued", + }, }, { name: "queueBuildWhenOlderBuildIsAlreadyScheduled", @@ -307,6 +366,12 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-new) because it has been created before - the build (my-build) gets enqueued", + }, }, { name: "allowBuildsInNewNamespace", @@ -317,8 +382,9 @@ func TestMonitorFIFOBuilds(t *testing.T) { builds: []*v1.Build{ newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), }, - build: newBuild("ns", "my-build"), - allowed: true, + build: newBuild("ns", "my-build"), + allowed: true, + condition: nil, }, } @@ -348,6 +414,14 @@ func TestMonitorFIFOBuilds(t *testing.T) { assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + if tc.condition == nil { + assert.Nil(t, tc.build.Status.GetCondition(v1.BuildConditionScheduled)) + } else { + assert.Equal(t, tc.condition.Type, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Type) + assert.Equal(t, tc.condition.Status, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Status) + assert.Equal(t, tc.condition.Reason, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Reason) + assert.Equal(t, tc.condition.Message, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Message) + } }) } } @@ -356,25 +430,28 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { deps := []string{"camel:core", "camel:timer", "camel:log", "mvn:org.apache.camel.k:camel-k-runtime"} testcases := []struct { - name string - running []*v1.Build - builds []*v1.Build - build *v1.Build - allowed bool + name string + running []*v1.Build + builds []*v1.Build + build *v1.Build + allowed bool + condition *v1.BuildCondition }{ { - name: "allowNewBuild", - running: []*v1.Build{}, - builds: []*v1.Build{}, - build: newBuild("ns", "my-build", deps...), - allowed: true, + name: "allowNewBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, { - name: "allowNewNativeBuild", - running: []*v1.Build{}, - builds: []*v1.Build{}, - build: newNativeBuild("ns1", "my-build", deps...), - allowed: true, + name: "allowNewNativeBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newNativeBuild("ns1", "my-build", deps...), + allowed: true, + condition: nil, }, { name: "allowNewBuildWhenOthersFinished", @@ -383,8 +460,9 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...), newBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed, deps...), }, - build: newBuild("ns", "my-build", deps...), - allowed: true, + build: newBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, { name: "allowNewNativeBuildWhenOthersFinished", @@ -393,8 +471,9 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...), newNativeBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed, deps...), }, - build: newNativeBuild("ns", "my-build", deps...), - allowed: true, + build: newNativeBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, { name: "limitMaxRunningBuilds", @@ -408,6 +487,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "limitMaxRunningNativeBuilds", @@ -421,14 +506,21 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization, deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued", + }, }, { name: "allowParallelBuildsWithDifferentLayout", running: []*v1.Build{ newNativeBuildInPhase("ns", "my-build-1", v1.BuildPhaseRunning, deps...), }, - build: newBuild("ns", "my-build", deps...), - allowed: true, + build: newBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, { name: "allowParallelBuildsInSameNamespaceWithTooManyMissingDependencies", @@ -440,8 +532,9 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...), newBuildInPhase("other-ns", "my-build-new", v1.BuildPhaseScheduling, deps...), }, - build: newBuild("ns", "my-build", append(deps, "camel:test", "camel:foo")...), - allowed: true, + build: newBuild("ns", "my-build", append(deps, "camel:test", "camel:foo")...), + allowed: true, + condition: nil, }, { name: "allowParallelBuildsInSameNamespaceWithLessDependencies", @@ -453,8 +546,9 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...), newBuildInPhase("ns", "my-build-scheduled", v1.BuildPhaseScheduling, append(deps, "camel:test")...), }, - build: newBuild("ns", "my-build", deps...), - allowed: true, + build: newBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, { name: "queueBuildWhenSuitableBuildHasOnlyOneSingleMissingDependency", @@ -468,6 +562,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", append(deps, "camel:test")...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-new) to finish in order to use incremental image builds - the build (my-build) gets enqueued", + }, }, { name: "queueBuildWhenSuitableBuildIsAlreadyRunning", @@ -481,6 +581,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-running) to finish in order to use incremental image builds - the build (my-build) gets enqueued", + }, }, { name: "queueBuildWhenSuitableBuildIsAlreadyPending", @@ -494,6 +600,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-pending) to finish in order to use incremental image builds - the build (my-build) gets enqueued", + }, }, { name: "queueBuildWhenSuitableBuildWithSupersetDependenciesIsAlreadyRunning", @@ -507,6 +619,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-running) to finish in order to use incremental image builds - the build (my-build) gets enqueued", + }, }, { name: "queueBuildWhenSuitableBuildWithSameDependenciesIsAlreadyScheduled", @@ -520,6 +638,12 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: corev1.ConditionFalse, + Reason: v1.BuildConditionWaitingReason, + Message: "Waiting for build (my-build-existing) to finish in order to use incremental image builds - the build (my-build) gets enqueued", + }, }, { name: "allowBuildsInNewNamespace", @@ -530,8 +654,9 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { builds: []*v1.Build{ newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...), }, - build: newBuild("ns", "my-build", deps...), - allowed: true, + build: newBuild("ns", "my-build", deps...), + allowed: true, + condition: nil, }, } @@ -561,6 +686,14 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + if tc.condition == nil { + assert.Nil(t, tc.build.Status.GetCondition(v1.BuildConditionScheduled)) + } else { + assert.Equal(t, tc.condition.Type, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Type) + assert.Equal(t, tc.condition.Status, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Status) + assert.Equal(t, tc.condition.Reason, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Reason) + assert.Equal(t, tc.condition.Message, tc.build.Status.GetCondition(v1.BuildConditionScheduled).Message) + } }) } } diff --git a/pkg/controller/build/schedule.go b/pkg/controller/build/schedule.go index 35fe2bd841..cfb19a1062 100644 --- a/pkg/controller/build/schedule.go +++ b/pkg/controller/build/schedule.go @@ -19,8 +19,10 @@ package build import ( "context" + "fmt" "sync" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime/pkg/client" @@ -63,7 +65,7 @@ func (action *scheduleAction) Handle(ctx context.Context, build *v1.Build) (*v1. return nil, err } else if !allowed { // Build not allowed at this state (probably max running builds limit exceeded) - let's requeue the build - return nil, nil + return build, nil } // Reset the Build status, and transition it to pending phase. @@ -80,7 +82,17 @@ func (action *scheduleAction) toPendingPhase(ctx context.Context, build *v1.Buil Failure: b.Status.Failure, Conditions: b.Status.Conditions, } + b.Status.SetCondition( + v1.BuildConditionScheduled, + corev1.ConditionTrue, + v1.BuildConditionReadyReason, + fmt.Sprintf( + "the build (%s) is scheduled", + build.Name, + ), + ) }) + if err != nil { return err }