From 05ed327f948c6e385c9ba5d5b224161e50bd42da 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 | 57 +++++ 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 | 14 +- 5 files changed, 272 insertions(+), 94 deletions(-) diff --git a/e2e/builder/build_test.go b/e2e/builder/build_test.go index f449f0ef23..f323c86c00 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 { @@ -111,6 +112,15 @@ func TestKitMaxBuildLimit(t *testing.T) { }, }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + // verify that last build is waiting + Eventually(BuildConditions(ns2, buildC), TestTimeoutMedium).ShouldNot(BeNil()) + Eventually( + Build(ns2, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionFalse)) + Eventually( + Build(ns2, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionWaitingReason)) + var notExceedsMaxBuildLimit = func(runningBuilds int) bool { return runningBuilds <= 2 } @@ -131,10 +141,21 @@ 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)) + + // verify that last build is scheduled + Eventually(BuildConditions(ns2, buildC), TestTimeoutMedium).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)) }) }) }) @@ -187,6 +208,15 @@ func TestKitMaxBuildLimitFIFOStrategy(t *testing.T) { }, }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + // verify that last build is waiting + Eventually(BuildConditions(ns, buildC), TestTimeoutMedium).ShouldNot(BeNil()) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionFalse)) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionWaitingReason)) + var notExceedsMaxBuildLimit = func(runningBuilds int) bool { return runningBuilds <= 2 } @@ -211,6 +241,15 @@ func TestKitMaxBuildLimitFIFOStrategy(t *testing.T) { Eventually(KitPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) Eventually(BuildPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + + // verify that last build is scheduled + 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)) }) } @@ -261,6 +300,15 @@ func TestKitMaxBuildLimitDependencyMatchingStrategy(t *testing.T) { }, }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + // verify that last build is waiting + Eventually(BuildConditions(ns, buildC), TestTimeoutMedium).ShouldNot(BeNil()) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Status, + TestTimeoutShort).Should(Equal(corev1.ConditionFalse)) + Eventually( + Build(ns, buildC)().Status.GetCondition(v1.BuildConditionScheduled).Reason, + TestTimeoutShort).Should(Equal(v1.BuildConditionWaitingReason)) + var notExceedsMaxBuildLimit = func(runningBuilds int) bool { return runningBuilds <= 2 } @@ -285,6 +333,15 @@ func TestKitMaxBuildLimitDependencyMatchingStrategy(t *testing.T) { Eventually(KitPhase(ns, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) Eventually(BuildPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) Eventually(KitPhase(ns, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady)) + + // verify that last build is scheduled + 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..cbdbedc174 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 9f1c051b96..a5be2b6582 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 35fe2bd841..0847fd6cbd 100644 --- a/pkg/controller/build/schedule.go +++ b/pkg/controller/build/schedule.go @@ -59,28 +59,32 @@ 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 + return nil, action.toUpdatedStatus(ctx, build, schedulingCondition, build.Status.Phase) } // 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) 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 }