This is an automated email from the ASF dual-hosted git repository. cdeppisch pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/camel-k.git
The following commit(s) were added to refs/heads/main by this push: new 4b38fc17b feat(build): Add Build waiting condition 4b38fc17b is described below commit 4b38fc17b37c2f6e5bf045247bca944dfcc553c7 Author: Gaelle Fournier <gaelle.fournier.w...@gmail.com> AuthorDate: Mon Feb 12 14:22:03 2024 +0100 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 | 73 +++++++++ e2e/common/traits/builder_test.go | 5 +- e2e/support/test_support.go | 10 ++ pkg/apis/camel/v1/build_types.go | 8 + pkg/controller/build/build_monitor.go | 51 +++++-- pkg/controller/build/build_monitor_test.go | 236 +++++++++++++++++++---------- pkg/controller/build/schedule.go | 29 +++- 7 files changed, 316 insertions(+), 96 deletions(-) diff --git a/e2e/builder/build_test.go b/e2e/builder/build_test.go index f449f0ef2..c90f91f3d 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 { @@ -131,8 +132,10 @@ func TestKitMaxBuildLimit(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(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) }) @@ -288,6 +291,76 @@ func TestKitMaxBuildLimitDependencyMatchingStrategy(t *testing.T) { }) } +func TestMaxBuildLimitWaitingBuilds(t *testing.T) { + WithNewTestNamespace(t, func(ns string) { + createOperator(ns, "8m0s", "--global", "--force") + + pl := Platform(ns)() + // set maximum number of running builds and order strategy + pl.Spec.Build.MaxRunningBuilds = 1 + pl.Spec.Build.BuildConfiguration.OrderStrategy = v1.BuildOrderStrategyFIFO + if err := TestClient().Update(TestContext, pl); err != nil { + t.Error(err) + t.FailNow() + } + + buildA := "integration-a" + buildB := "integration-b" + buildC := "integration-c" + + doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{ + operatorID: fmt.Sprintf("camel-k-%s", ns), + dependencies: []string{ + "camel:timer", "camel:log", + }, + traits: []string{ + "builder.properties=build-property=A", + }, + }, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning) + + doKitBuildInNamespace(buildB, ns, TestTimeoutShort, kitOptions{ + operatorID: fmt.Sprintf("camel-k-%s", ns), + dependencies: []string{ + "camel:cron", "camel:log", "camel:joor", + }, + traits: []string{ + "builder.properties=build-property=B", + }, + }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + + doKitBuildInNamespace(buildC, ns, TestTimeoutShort, kitOptions{ + operatorID: fmt.Sprintf("camel-k-%s", ns), + dependencies: []string{ + "camel:timer", "camel:log", "camel:joor", "camel:http", + }, + traits: []string{ + "builder.properties=build-property=C", + }, + }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + + // verify that last build is waiting + Eventually(BuildConditions(ns, buildC), TestTimeoutMedium).ShouldNot(BeNil()) + Eventually( + BuildCondition(ns, buildC, v1.BuildConditionType(v1.BuildConditionScheduled))().Status, + TestTimeoutShort).Should(Equal(corev1.ConditionFalse)) + Eventually( + BuildCondition(ns, buildC, v1.BuildConditionType(v1.BuildConditionScheduled))().Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionWaitingReason)) + + // verify that last build is scheduled + 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( + BuildCondition(ns, buildC, v1.BuildConditionType(v1.BuildConditionScheduled))().Status, + TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually( + BuildCondition(ns, buildC, v1.BuildConditionType(v1.BuildConditionScheduled))().Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionReadyReason)) + }) +} + func TestKitTimerToLogFullBuild(t *testing.T) { doKitFullBuild(t, "timer-to-log", "8m0s", TestTimeoutLong, kitOptions{ dependencies: []string{ diff --git a/e2e/common/traits/builder_test.go b/e2e/common/traits/builder_test.go index 5a1df5d30..dea284e83 100644 --- a/e2e/common/traits/builder_test.go +++ b/e2e/common/traits/builder_test.go @@ -225,11 +225,12 @@ func TestBuilderTrait(t *testing.T) { // Check containers conditions Eventually(Build(integrationKitNamespace, integrationKitName), TestTimeoutLong).ShouldNot(BeNil()) Eventually(BuildConditions(integrationKitNamespace, integrationKitName), TestTimeoutLong).ShouldNot(BeNil()) + Eventually(BuildCondition(integrationKitNamespace, integrationKitName, v1.BuildConditionType("Container custom1 succeeded")), TestTimeoutMedium).ShouldNot(BeNil()) Eventually( - Build(integrationKitNamespace, integrationKitName)().Status.GetCondition(v1.BuildConditionType("Container custom1 succeeded")).Status, + BuildCondition(integrationKitNamespace, integrationKitName, v1.BuildConditionType("Container custom1 succeeded"))().Status, TestTimeoutShort).Should(Equal(corev1.ConditionFalse)) Eventually( - Build(integrationKitNamespace, integrationKitName)().Status.GetCondition(v1.BuildConditionType("Container custom1 succeeded")).Message, + BuildCondition(integrationKitNamespace, integrationKitName, v1.BuildConditionType("Container custom1 succeeded"))().Message, TestTimeoutShort).Should(ContainSubstring("No such file or directory")) Expect(Kamel("delete", "--all", "-n", ns).Execute()).To(Succeed()) diff --git a/e2e/support/test_support.go b/e2e/support/test_support.go index 7b66c21ba..1a46a12a1 100644 --- a/e2e/support/test_support.go +++ b/e2e/support/test_support.go @@ -1872,6 +1872,16 @@ func BuildConditions(ns, name string) func() []v1.BuildCondition { } } +func BuildCondition(ns string, name string, conditionType v1.BuildConditionType) func() *v1.BuildCondition { + return func() *v1.BuildCondition { + build := Build(ns, name)() + if build != nil && &build.Status != nil && build.Status.Conditions != nil { + return build.Status.GetCondition(conditionType) + } + return &v1.BuildCondition{} + } +} + func BuildFailureRecovery(ns, name string) func() int { return func() int { build := Build(ns, name)() diff --git a/pkg/apis/camel/v1/build_types.go b/pkg/apis/camel/v1/build_types.go index 188aa0ad1..7c1dc4e43 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 2d45b2223..cbdbedc17 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" @@ -31,6 +32,8 @@ import ( "github.com/apache/camel-k/v2/pkg/util/kubernetes" ) +const enqueuedMsg = "%s - the build (%s) gets enqueued" + var runningBuilds sync.Map type Monitor struct { @@ -38,7 +41,8 @@ type Monitor struct { buildOrderStrategy v1.BuildOrderStrategy } -func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Build) (bool, error) { +func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Build) (bool, *v1.BuildCondition, error) { + var runningBuildsTotal int32 runningBuilds.Range(func(_, v interface{}) bool { runningBuildsTotal++ @@ -54,24 +58,27 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui } if runningBuildsTotal >= bm.maxRunningBuilds { + reason := fmt.Sprintf( + "Maximum number of running builds (%d) exceeded", + runningBuildsTotal, + ) 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) - + ForBuild(build).Infof(enqueuedMsg, reason, build.Name) // max number of running builds limit exceeded - return false, nil + return false, scheduledWaitingBuildcondition(build.Name, reason), nil } layout := build.Labels[v1.IntegrationKitLayoutLabel] // Native builds can be run in parallel, as incremental images is not applicable. if layout == v1.IntegrationKitLayoutNativeSources { - return true, nil + return true, scheduledReadyBuildcondition(build.Name), nil } // We assume incremental images is only applicable across images whose layout is identical withCompatibleLayout, err := labels.NewRequirement(v1.IntegrationKitLayoutLabel, selection.Equals, []string{layout}) if err != nil { - return false, err + return false, nil, err } builds := &v1.BuildList{} @@ -83,11 +90,12 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui Selector: labels.NewSelector().Add(*withCompatibleLayout), }) if err != nil { - return false, err + return false, nil, err } var reason string allowed := true + condition := scheduledReadyBuildcondition(build.Name) switch bm.buildOrderStrategy { case v1.BuildOrderStrategyFIFO: // Check on builds that have been created before the current build and grant precedence if any. @@ -116,10 +124,11 @@ 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) + ForBuild(build).Infof(enqueuedMsg, reason, build.Name) + condition = scheduledWaitingBuildcondition(build.Name, reason) } - return allowed, nil + return allowed, condition, nil } func monitorRunningBuild(build *v1.Build) { @@ -129,3 +138,27 @@ func monitorRunningBuild(build *v1.Build) { func monitorFinishedBuild(build *v1.Build) { runningBuilds.Delete(types.NamespacedName{Namespace: build.Namespace, Name: build.Name}.String()) } + +func scheduledReadyBuildcondition(buildName string) *v1.BuildCondition { + return scheduledBuildcondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, fmt.Sprintf( + "the build (%s) is scheduled", + buildName, + )) +} + +func scheduledWaitingBuildcondition(buildName string, reason string) *v1.BuildCondition { + return scheduledBuildcondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, fmt.Sprintf( + enqueuedMsg, + reason, + buildName, + )) +} + +func scheduledBuildcondition(status corev1.ConditionStatus, reason string, msg string) *v1.BuildCondition { + return &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: status, + Reason: reason, + Message: msg, + } +} diff --git a/pkg/controller/build/build_monitor_test.go b/pkg/controller/build/build_monitor_test.go index 9f1c051b9..a5be2b658 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { - 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-native-build) is scheduled"), }, { 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-native-build) is scheduled"), }, { name: "limitMaxRunningBuilds", @@ -84,6 +90,8 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued"), }, { name: "limitMaxRunningNativeBuilds", @@ -97,14 +105,17 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "queueBuildsInSameNamespaceWithSameLayout", @@ -117,6 +128,8 @@ func TestMonitorSequentialBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Found a running build in this namespace - the build (my-build) gets enqueued"), }, { name: "allowBuildsInNewNamespace", @@ -127,8 +140,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, } @@ -154,10 +168,14 @@ func TestMonitorSequentialBuilds(t *testing.T) { monitorRunningBuild(build) } - allowed, err := bm.canSchedule(context.TODO(), c, tc.build) + allowed, condition, err := bm.canSchedule(context.TODO(), c, tc.build) assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + assert.Equal(t, tc.condition.Type, condition.Type) + assert.Equal(t, tc.condition.Status, condition.Status) + assert.Equal(t, tc.condition.Reason, condition.Reason) + assert.Equal(t, tc.condition.Message, condition.Message) }) } } @@ -180,40 +198,45 @@ func TestAllowBuildRequeue(t *testing.T) { monitorRunningBuild(newBuild("another-ns", "my-build-3")) build := newBuild("ns", "my-build") - allowed, err := bm.canSchedule(context.TODO(), c, build) + allowed, condition, err := bm.canSchedule(context.TODO(), c, build) assert.Nil(t, err) assert.False(t, allowed) + assert.Equal(t, corev1.ConditionFalse, condition.Status) monitorFinishedBuild(runningBuild) - allowed, err = bm.canSchedule(context.TODO(), c, build) + allowed, condition, err = bm.canSchedule(context.TODO(), c, build) assert.Nil(t, err) assert.True(t, allowed) + assert.Equal(t, corev1.ConditionTrue, condition.Status) } 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { - 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowNewBuildWhenOthersFinished", @@ -222,8 +245,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowNewNativeBuildWhenOthersFinished", @@ -232,8 +256,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "limitMaxRunningBuilds", @@ -247,6 +272,8 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued"), }, { name: "limitMaxRunningNativeBuilds", @@ -260,14 +287,17 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowParallelBuildsInSameNamespace", @@ -279,8 +309,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "queueBuildWhenOlderBuildIsAlreadyInitialized", @@ -294,6 +325,8 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Waiting for build (my-build-new) because it has been created before - the build (my-build) gets enqueued"), }, { name: "queueBuildWhenOlderBuildIsAlreadyScheduled", @@ -307,6 +340,8 @@ func TestMonitorFIFOBuilds(t *testing.T) { }, build: newBuild("ns", "my-build"), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Waiting for build (my-build-new) because it has been created before - the build (my-build) gets enqueued"), }, { name: "allowBuildsInNewNamespace", @@ -317,8 +352,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, } @@ -344,10 +380,14 @@ func TestMonitorFIFOBuilds(t *testing.T) { monitorRunningBuild(build) } - allowed, err := bm.canSchedule(context.TODO(), c, tc.build) + allowed, condition, err := bm.canSchedule(context.TODO(), c, tc.build) assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + assert.Equal(t, tc.condition.Type, condition.Type) + assert.Equal(t, tc.condition.Status, condition.Status) + assert.Equal(t, tc.condition.Reason, condition.Reason) + assert.Equal(t, tc.condition.Message, condition.Message) }) } } @@ -356,25 +396,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { - 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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowNewBuildWhenOthersFinished", @@ -383,8 +426,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowNewNativeBuildWhenOthersFinished", @@ -393,8 +437,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "limitMaxRunningBuilds", @@ -408,6 +453,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "Maximum number of running builds (3) exceeded - the build (my-build) gets enqueued"), }, { name: "limitMaxRunningNativeBuilds", @@ -421,14 +468,17 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization, deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowParallelBuildsInSameNamespaceWithTooManyMissingDependencies", @@ -440,8 +490,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "allowParallelBuildsInSameNamespaceWithLessDependencies", @@ -453,8 +504,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, { name: "queueBuildWhenSuitableBuildHasOnlyOneSingleMissingDependency", @@ -468,6 +520,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", append(deps, "camel:test")...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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 +535,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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 +550,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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 +565,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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 +580,8 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { }, build: newBuild("ns", "my-build", deps...), allowed: false, + condition: newCondition(corev1.ConditionFalse, v1.BuildConditionWaitingReason, + "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 +592,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: newCondition(corev1.ConditionTrue, v1.BuildConditionReadyReason, "the build (my-build) is scheduled"), }, } @@ -557,10 +620,14 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) { monitorRunningBuild(build) } - allowed, err := bm.canSchedule(context.TODO(), c, tc.build) + allowed, condition, err := bm.canSchedule(context.TODO(), c, tc.build) assert.Nil(t, err) assert.Equal(t, tc.allowed, allowed) + assert.Equal(t, tc.condition.Type, condition.Type) + assert.Equal(t, tc.condition.Status, condition.Status) + assert.Equal(t, tc.condition.Reason, condition.Reason) + assert.Equal(t, tc.condition.Message, condition.Message) }) } } @@ -572,6 +639,15 @@ func cleanRunningBuildsMonitor() { }) } +func newCondition(status corev1.ConditionStatus, reason string, msg string) *v1.BuildCondition { + return &v1.BuildCondition{ + Type: v1.BuildConditionScheduled, + Status: status, + Reason: reason, + Message: msg, + } +} + func newBuild(namespace string, name string, dependencies ...string) *v1.Build { return newBuildWithLayoutInPhase(namespace, name, v1.IntegrationKitLayoutFastJar, v1.BuildPhasePending, dependencies...) } diff --git a/pkg/controller/build/schedule.go b/pkg/controller/build/schedule.go index 35fe2bd84..a0188ddd1 100644 --- a/pkg/controller/build/schedule.go +++ b/pkg/controller/build/schedule.go @@ -59,28 +59,47 @@ func (action *scheduleAction) Handle(ctx context.Context, build *v1.Build) (*v1. action.lock.Lock() defer action.lock.Unlock() - if allowed, err := action.buildMonitor.canSchedule(ctx, action.reader, build); err != nil { + allowed, schedulingCondition, err := action.buildMonitor.canSchedule(ctx, action.reader, build) + + if err != nil { 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 + // Update the condition without reseting the Build Status. + // This must be done in the critical section, rather than delegated to the controller. + return nil, action.toUpdatedCondition(ctx, build, schedulingCondition) } // Reset the Build status, and transition it to pending phase. // This must be done in the critical section, rather than delegated to the controller. - return nil, action.toPendingPhase(ctx, build) + return nil, action.toUpdatedStatus(ctx, build, schedulingCondition, v1.BuildPhasePending) } -func (action *scheduleAction) toPendingPhase(ctx context.Context, build *v1.Build) error { +func (action *scheduleAction) toUpdatedCondition(ctx context.Context, build *v1.Build, condition *v1.BuildCondition) error { + return action.patchBuildStatus(ctx, build, func(b *v1.Build) { + b.Status = v1.BuildStatus{ + Phase: b.Status.Phase, + StartedAt: b.Status.StartedAt, + Failure: b.Status.Failure, + Conditions: b.Status.Conditions, + } + b.Status.SetConditions(*condition) + }) + +} + +func (action *scheduleAction) toUpdatedStatus(ctx context.Context, build *v1.Build, condition *v1.BuildCondition, phase v1.BuildPhase) error { err := action.patchBuildStatus(ctx, build, func(b *v1.Build) { now := metav1.Now() b.Status = v1.BuildStatus{ - Phase: v1.BuildPhasePending, + Phase: phase, StartedAt: &now, Failure: b.Status.Failure, Conditions: b.Status.Conditions, } + b.Status.SetConditions(*condition) }) + if err != nil { return err }