From bda1d26bb58ba2b786c750ad89ddc5551006b715 Mon Sep 17 00:00:00 2001 From: Christoph Deppisch Date: Thu, 1 Jun 2023 15:08:13 +0200 Subject: [PATCH] fix(#592): Introduce build order strategy - Run builds on same operator namespace with user defined strategy - Default strategy is "sequential" running only one single build at a time - Also support "fifo" strategy where builds are run/queued based on their creation timestamp. This strategy allows parallel builds as long as individual build dependency lists are not colliding - Users may adjust/overwrite the build order strategy via install command option, in the (local) integration platform settings or via the builder trait option - Max number of running builds limitation is not affected by the build order strategy (ensure to always obey the limitation) --- .../ROOT/pages/architecture/cr/build.adoc | 9 + .../pages/installation/advanced/advanced.adoc | 3 +- e2e/builder/build_test.go | 77 +++++++- e2e/common/traits/builder_test.go | 27 +++ .../local_platform_test.go | 1 + pkg/apis/camel/v1/build_types_support.go | 25 +++ pkg/apis/camel/v1/common_types.go | 18 ++ pkg/apis/camel/v1/common_types_support.go | 1 + pkg/apis/camel/v1/trait/builder.go | 2 + .../camel/v1/buildconfiguration.go | 23 ++- pkg/cmd/install.go | 22 +++ pkg/cmd/install_test.go | 7 + pkg/controller/build/build_controller.go | 3 +- pkg/controller/build/build_monitor.go | 43 +++-- pkg/controller/build/build_monitor_test.go | 173 +++++++++++++++++- pkg/controller/integrationkit/build.go | 13 +- pkg/platform/defaults.go | 9 + pkg/platform/defaults_test.go | 11 +- pkg/trait/builder.go | 43 ++++- 19 files changed, 462 insertions(+), 48 deletions(-) diff --git a/docs/modules/ROOT/pages/architecture/cr/build.adoc b/docs/modules/ROOT/pages/architecture/cr/build.adoc index 306fb22bfb..f331906f1d 100644 --- a/docs/modules/ROOT/pages/architecture/cr/build.adoc +++ b/docs/modules/ROOT/pages/architecture/cr/build.adoc @@ -36,6 +36,15 @@ At the moment the available strategies are: - buildStrategy: pod (each build is run in a separate pod, the operator monitors the pod state) - buildStrategy: routine (each build is run as a go routine inside the operator pod) +[[build-order-strategy]] +== Build order strategy + +You can choose from different build order strategies. The strategy defines in which order queued builds are run. +At the moment the available strategies are: + +- buildOrderStrategy: sequential (runs builds strictly sequential so that only one single build per operator namespace is running at a time.) +- buildOrderStrategy: fifo (performs the builds with first in first out strategy based on the creation timestamp. The strategy allows builds to run in parallel to each other but oldest builds will be run first.) + [[build-queue]] == Build queues diff --git a/docs/modules/ROOT/pages/installation/advanced/advanced.adoc b/docs/modules/ROOT/pages/installation/advanced/advanced.adoc index 2303b193b1..50b3bd948f 100644 --- a/docs/modules/ROOT/pages/installation/advanced/advanced.adoc +++ b/docs/modules/ROOT/pages/installation/advanced/advanced.adoc @@ -38,6 +38,7 @@ We have several configuration used to influence the building of an integration: --build-publish-strategy string Set the build publish strategy --build-publish-strategy-option stringArray Add a build publish strategy option, as --build-strategy string Set the build strategy +--build-order-strategy string Set the build order strategy --build-timeout string Set how long the build process can last ``` A very important set of configuration you can provide is related to Maven: @@ -85,4 +86,4 @@ We have also certain configuration that let you control how to deploy your Camel --global Configure the operator to watch all namespaces. No integration platform is created. You can run integrations in a namespace by installing an integration platform: 'kamel install --skip-operator-setup -n my-namespace' --operator-id string Set the operator id that is used to select the resources this operator should manage (default "camel-k") ``` -Learn more about xref:installation/advanced/multi.adoc[Camel K multi-tenancy]. \ No newline at end of file +Learn more about xref:installation/advanced/multi.adoc[Camel K multi-tenancy]. diff --git a/e2e/builder/build_test.go b/e2e/builder/build_test.go index ae4835f133..ccb266706c 100644 --- a/e2e/builder/build_test.go +++ b/e2e/builder/build_test.go @@ -45,8 +45,9 @@ func TestKitMaxBuildLimit(t *testing.T) { createOperator(ns, "8m0s", "--global", "--force") pl := Platform(ns)() - // set maximum number of running builds + // set maximum number of running builds and order strategy pl.Spec.Build.MaxRunningBuilds = 2 + pl.Spec.Build.BuildConfiguration.OrderStrategy = v1.BuildOrderStrategySequential if err := TestClient().Update(TestContext, pl); err != nil { t.Error(err) t.FailNow() @@ -139,6 +140,80 @@ func TestKitMaxBuildLimit(t *testing.T) { }) } +func TestKitMaxBuildLimitFIFOStrategy(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 = 2 + 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:timer", "camel:log", + }, + traits: []string{ + "builder.properties=build-property=B", + }, + }, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning) + + doKitBuildInNamespace(buildC, ns, TestTimeoutShort, kitOptions{ + operatorID: fmt.Sprintf("camel-k-%s", ns), + dependencies: []string{ + "camel:timer", "camel:log", + }, + traits: []string{ + "builder.properties=build-property=C", + }, + }, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone) + + var notExceedsMaxBuildLimit = func(runningBuilds int) bool { + return runningBuilds <= 2 + } + + limit := 0 + for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning { + // verify that number of running builds does not exceed max build limit + Consistently(BuildsRunning(BuildPhase(ns, buildA), BuildPhase(ns, buildB), BuildPhase(ns, buildC)), TestTimeoutShort, 10*time.Second).Should(Satisfy(notExceedsMaxBuildLimit)) + limit++ + } + + // make sure we have verified max build limit at least once + if limit == 0 { + t.Error(errors.New(fmt.Sprintf("Unexpected build phase '%s' for %s - not able to verify max builds limit", BuildPhase(ns, buildA)(), buildA))) + t.FailNow() + } + + // 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(ns, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded)) + 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)) + }) +} + 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 da8e6f39b0..2114261e38 100644 --- a/e2e/common/traits/builder_test.go +++ b/e2e/common/traits/builder_test.go @@ -51,6 +51,32 @@ func TestBuilderTrait(t *testing.T) { integrationKitName := IntegrationKit(ns, name)() builderKitName := fmt.Sprintf("camel-k-%s-builder", integrationKitName) Eventually(BuildConfig(ns, integrationKitName)().Strategy, TestTimeoutShort).Should(Equal(v1.BuildStrategyRoutine)) + Eventually(BuildConfig(ns, integrationKitName)().OrderStrategy, TestTimeoutShort).Should(Equal(v1.BuildOrderStrategySequential)) + // Default resource CPU Check + Eventually(BuildConfig(ns, integrationKitName)().RequestCPU, TestTimeoutShort).Should(Equal("")) + Eventually(BuildConfig(ns, integrationKitName)().LimitCPU, TestTimeoutShort).Should(Equal("")) + Eventually(BuildConfig(ns, integrationKitName)().RequestMemory, TestTimeoutShort).Should(Equal("")) + Eventually(BuildConfig(ns, integrationKitName)().LimitMemory, TestTimeoutShort).Should(Equal("")) + + Eventually(BuilderPod(ns, builderKitName), TestTimeoutShort).Should(BeNil()) + + // We need to remove the kit as well + Expect(Kamel("reset", "-n", ns).Execute()).To(Succeed()) + }) + + t.Run("Run build order strategy fifo", func(t *testing.T) { + Expect(KamelRunWithID(operatorID, ns, "files/Java.java", + "--name", name, + "-t", "builder.order-strategy=fifo").Execute()).To(Succeed()) + + Eventually(IntegrationPodPhase(ns, name), TestTimeoutLong).Should(Equal(corev1.PodRunning)) + Eventually(IntegrationConditionStatus(ns, name, v1.IntegrationConditionReady), TestTimeoutShort).Should(Equal(corev1.ConditionTrue)) + Eventually(IntegrationLogs(ns, name), TestTimeoutShort).Should(ContainSubstring("Magicstring!")) + + integrationKitName := IntegrationKit(ns, name)() + builderKitName := fmt.Sprintf("camel-k-%s-builder", integrationKitName) + Eventually(BuildConfig(ns, integrationKitName)().Strategy, TestTimeoutShort).Should(Equal(v1.BuildStrategyPod)) + Eventually(BuildConfig(ns, integrationKitName)().OrderStrategy, TestTimeoutShort).Should(Equal(v1.BuildOrderStrategyFIFO)) // Default resource CPU Check Eventually(BuildConfig(ns, integrationKitName)().RequestCPU, TestTimeoutShort).Should(Equal("")) Eventually(BuildConfig(ns, integrationKitName)().LimitCPU, TestTimeoutShort).Should(Equal("")) @@ -80,6 +106,7 @@ func TestBuilderTrait(t *testing.T) { builderKitName := fmt.Sprintf("camel-k-%s-builder", integrationKitName) Eventually(BuildConfig(ns, integrationKitName)().Strategy, TestTimeoutShort).Should(Equal(v1.BuildStrategyPod)) + Eventually(BuildConfig(ns, integrationKitName)().OrderStrategy, TestTimeoutShort).Should(Equal(v1.BuildOrderStrategySequential)) Eventually(BuildConfig(ns, integrationKitName)().RequestCPU, TestTimeoutShort).Should(Equal("500m")) Eventually(BuildConfig(ns, integrationKitName)().LimitCPU, TestTimeoutShort).Should(Equal("1000m")) Eventually(BuildConfig(ns, integrationKitName)().RequestMemory, TestTimeoutShort).Should(Equal("2Gi")) diff --git a/e2e/commonwithcustominstall/local_platform_test.go b/e2e/commonwithcustominstall/local_platform_test.go index 05ce8c3b7c..2f924ac20c 100644 --- a/e2e/commonwithcustominstall/local_platform_test.go +++ b/e2e/commonwithcustominstall/local_platform_test.go @@ -82,6 +82,7 @@ func TestLocalPlatform(t *testing.T) { local := Platform(ns1)() Expect(local.Status.Build.PublishStrategy).To(Equal(pl.Status.Build.PublishStrategy)) Expect(local.Status.Build.BuildConfiguration.Strategy).To(Equal(pl.Status.Build.BuildConfiguration.Strategy)) + Expect(local.Status.Build.BuildConfiguration.OrderStrategy).To(Equal(pl.Status.Build.BuildConfiguration.OrderStrategy)) Expect(local.Status.Build.Maven.LocalRepository).To(Equal(pl.Status.Build.Maven.LocalRepository)) Expect(local.Status.Build.Maven.CLIOptions).To(ContainElements(pl.Status.Build.Maven.CLIOptions)) Expect(local.Status.Build.Maven.Extension).To(BeEmpty()) diff --git a/pkg/apis/camel/v1/build_types_support.go b/pkg/apis/camel/v1/build_types_support.go index 6131f909b2..8c05f66e7b 100644 --- a/pkg/apis/camel/v1/build_types_support.go +++ b/pkg/apis/camel/v1/build_types_support.go @@ -201,3 +201,28 @@ func (c BuildCondition) GetReason() string { func (c BuildCondition) GetMessage() string { return c.Message } + +func (bl BuildList) HasRunningBuilds() bool { + for _, b := range bl.Items { + if b.Status.Phase == BuildPhasePending || b.Status.Phase == BuildPhaseRunning { + return true + } + } + + return false +} + +func (bl BuildList) HasScheduledBuildsBefore(build *Build) bool { + for _, b := range bl.Items { + if b.Name == build.Name { + continue + } + + if (b.Status.Phase == BuildPhaseInitialization || b.Status.Phase == BuildPhaseScheduling) && + b.CreationTimestamp.Before(&build.CreationTimestamp) { + return true + } + } + + return false +} diff --git a/pkg/apis/camel/v1/common_types.go b/pkg/apis/camel/v1/common_types.go index ede52e028a..5ad6469238 100644 --- a/pkg/apis/camel/v1/common_types.go +++ b/pkg/apis/camel/v1/common_types.go @@ -43,6 +43,8 @@ type BuildConfiguration struct { BuilderPodNamespace string `json:"operatorNamespace,omitempty"` // the strategy to adopt Strategy BuildStrategy `property:"strategy" json:"strategy,omitempty"` + // the build order strategy to adopt + OrderStrategy BuildOrderStrategy `property:"order-strategy" json:"orderStrategy,omitempty"` // The minimum amount of CPU required. Only used for `pod` strategy RequestCPU string `property:"request-cpu" json:"requestCPU,omitempty"` // The minimum amount of memory required. Only used for `pod` strategy @@ -67,6 +69,12 @@ const ( // BuildStrategyPod performs the build in a `Pod` (will schedule a new builder ephemeral `Pod` which will take care of the build action). // This strategy has the limitation that every build will have to download all the dependencies required by the Maven build. BuildStrategyPod BuildStrategy = "pod" + + // BuildOrderStrategyFIFO performs the builds with first in first out strategy based on the creation timestamp. + // The strategy allows builds to run in parallel to each other but oldest builds will be run first. + BuildOrderStrategyFIFO BuildOrderStrategy = "fifo" + // BuildOrderStrategySequential runs builds strictly sequential so that only one single build per operator namespace is running at a time. + BuildOrderStrategySequential BuildOrderStrategy = "sequential" ) // BuildStrategies is a list of strategies allowed for the build @@ -75,6 +83,16 @@ var BuildStrategies = []BuildStrategy{ BuildStrategyPod, } +// BuildOrderStrategy specifies how builds are reconciled and queued. +// +kubebuilder:validation:Enum=fifo;sequential +type BuildOrderStrategy string + +// BuildOrderStrategies is a list of order strategies allowed for the build +var BuildOrderStrategies = []BuildOrderStrategy{ + BuildOrderStrategyFIFO, + BuildOrderStrategySequential, +} + // ConfigurationSpec represents a generic configuration specification type ConfigurationSpec struct { // represents the type of configuration, ie: property, configmap, secret, ... diff --git a/pkg/apis/camel/v1/common_types_support.go b/pkg/apis/camel/v1/common_types_support.go index b66dca9a81..6b48170d34 100644 --- a/pkg/apis/camel/v1/common_types_support.go +++ b/pkg/apis/camel/v1/common_types_support.go @@ -159,6 +159,7 @@ var _ json.Unmarshaler = (*RawMessage)(nil) // IsEmpty -- . func (bc *BuildConfiguration) IsEmpty() bool { return bc.Strategy == "" && + bc.OrderStrategy == "" && bc.RequestCPU == "" && bc.RequestMemory == "" && bc.LimitCPU == "" && diff --git a/pkg/apis/camel/v1/trait/builder.go b/pkg/apis/camel/v1/trait/builder.go index 6bfe965467..86e682ab3d 100644 --- a/pkg/apis/camel/v1/trait/builder.go +++ b/pkg/apis/camel/v1/trait/builder.go @@ -29,6 +29,8 @@ type BuilderTrait struct { Properties []string `property:"properties" json:"properties,omitempty"` // The strategy to use, either `pod` or `routine` (default routine) Strategy string `property:"strategy" json:"strategy,omitempty"` + // The build order strategy to use, either `fifo` or `sequential` (default sequential) + OrderStrategy string `property:"order-strategy" json:"orderStrategy,omitempty"` // When using `pod` strategy, the minimum amount of CPU required by the pod builder. RequestCPU string `property:"request-cpu" json:"requestCPU,omitempty"` // When using `pod` strategy, the minimum amount of memory required by the pod builder. diff --git a/pkg/client/camel/applyconfiguration/camel/v1/buildconfiguration.go b/pkg/client/camel/applyconfiguration/camel/v1/buildconfiguration.go index 581db6aa51..7aa865add3 100644 --- a/pkg/client/camel/applyconfiguration/camel/v1/buildconfiguration.go +++ b/pkg/client/camel/applyconfiguration/camel/v1/buildconfiguration.go @@ -26,13 +26,14 @@ import ( // BuildConfigurationApplyConfiguration represents an declarative configuration of the BuildConfiguration type for use // with apply. type BuildConfigurationApplyConfiguration struct { - ToolImage *string `json:"toolImage,omitempty"` - BuilderPodNamespace *string `json:"operatorNamespace,omitempty"` - Strategy *v1.BuildStrategy `json:"strategy,omitempty"` - RequestCPU *string `json:"requestCPU,omitempty"` - RequestMemory *string `json:"requestMemory,omitempty"` - LimitCPU *string `json:"limitCPU,omitempty"` - LimitMemory *string `json:"limitMemory,omitempty"` + ToolImage *string `json:"toolImage,omitempty"` + BuilderPodNamespace *string `json:"operatorNamespace,omitempty"` + Strategy *v1.BuildStrategy `json:"strategy,omitempty"` + OrderStrategy *v1.BuildOrderStrategy `json:"orderStrategy,omitempty"` + RequestCPU *string `json:"requestCPU,omitempty"` + RequestMemory *string `json:"requestMemory,omitempty"` + LimitCPU *string `json:"limitCPU,omitempty"` + LimitMemory *string `json:"limitMemory,omitempty"` } // BuildConfigurationApplyConfiguration constructs an declarative configuration of the BuildConfiguration type for use with @@ -65,6 +66,14 @@ func (b *BuildConfigurationApplyConfiguration) WithStrategy(value v1.BuildStrate return b } +// WithOrderStrategy sets the OrderStrategy field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the OrderStrategy field is set to the value of the last call. +func (b *BuildConfigurationApplyConfiguration) WithOrderStrategy(value v1.BuildOrderStrategy) *BuildConfigurationApplyConfiguration { + b.OrderStrategy = &value + return b +} + // WithRequestCPU sets the RequestCPU field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the RequestCPU field is set to the value of the last call. diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index 7c95ba1c2a..64168a0006 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -107,6 +107,7 @@ func newCmdInstall(rootCmdOptions *RootCmdOptions) (*cobra.Command, *installCmdO cmd.Flags().String("operator-image", "", "Set the operator Image used for the operator deployment") cmd.Flags().String("operator-image-pull-policy", "", "Set the operator ImagePullPolicy used for the operator deployment") cmd.Flags().String("build-strategy", "", "Set the build strategy") + cmd.Flags().String("build-order-strategy", "", "Set the build order strategy") cmd.Flags().String("build-publish-strategy", "", "Set the build publish strategy") cmd.Flags().StringArray("build-publish-strategy-option", nil, "Add a build publish strategy option, as ") cmd.Flags().String("build-timeout", "", "Set how long the build process can last") @@ -187,6 +188,7 @@ type installCmdOptions struct { OperatorImage string `mapstructure:"operator-image"` OperatorImagePullPolicy string `mapstructure:"operator-image-pull-policy"` BuildStrategy string `mapstructure:"build-strategy"` + BuildOrderStrategy string `mapstructure:"build-order-strategy"` BuildPublishStrategy string `mapstructure:"build-publish-strategy"` BuildPublishStrategyOptions []string `mapstructure:"build-publish-strategy-options"` BuildTimeout string `mapstructure:"build-timeout"` @@ -535,6 +537,9 @@ func (o *installCmdOptions) setupIntegrationPlatform(c client.Client, namespace if o.BuildStrategy != "" { platform.Spec.Build.BuildConfiguration.Strategy = v1.BuildStrategy(o.BuildStrategy) } + if o.BuildOrderStrategy != "" { + platform.Spec.Build.BuildConfiguration.OrderStrategy = v1.BuildOrderStrategy(o.BuildOrderStrategy) + } if o.BuildPublishStrategy != "" { platform.Spec.Build.PublishStrategy = v1.IntegrationPlatformBuildPublishStrategy(o.BuildPublishStrategy) } @@ -767,6 +772,23 @@ func (o *installCmdOptions) validate(_ *cobra.Command, _ []string) error { } } + if o.BuildOrderStrategy != "" { + found := false + for _, s := range v1.BuildOrderStrategies { + if string(s) == o.BuildOrderStrategy { + found = true + break + } + } + if !found { + var strategies []string + for _, s := range v1.BuildOrderStrategies { + strategies = append(strategies, string(s)) + } + return fmt.Errorf("unknown build order strategy: %s. One of [%s] is expected", o.BuildOrderStrategy, strings.Join(strategies, ", ")) + } + } + if o.BuildPublishStrategy != "" { found := false for _, s := range v1.IntegrationPlatformBuildPublishStrategies { diff --git a/pkg/cmd/install_test.go b/pkg/cmd/install_test.go index a34fdef834..dccf4ca59e 100644 --- a/pkg/cmd/install_test.go +++ b/pkg/cmd/install_test.go @@ -107,6 +107,13 @@ func TestInstallBuildStrategyFlag(t *testing.T) { assert.Equal(t, "someString", installCmdOptions.BuildStrategy) } +func TestInstallBuildOrderStrategyFlag(t *testing.T) { + installCmdOptions, rootCmd, _ := initializeInstallCmdOptions(t) + _, err := test.ExecuteCommand(rootCmd, cmdInstall, "--build-order-strategy", "someString") + assert.Nil(t, err) + assert.Equal(t, "someString", installCmdOptions.BuildOrderStrategy) +} + func TestInstallBuildTimeoutFlag(t *testing.T) { installCmdOptions, rootCmd, _ := initializeInstallCmdOptions(t) _, err := test.ExecuteCommand(rootCmd, cmdInstall, "--build-timeout", "10") diff --git a/pkg/controller/build/build_controller.go b/pkg/controller/build/build_controller.go index 33fe525d87..75439a3d2b 100644 --- a/pkg/controller/build/build_controller.go +++ b/pkg/controller/build/build_controller.go @@ -147,7 +147,8 @@ func (r *reconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques return reconcile.Result{}, err } buildMonitor := Monitor{ - maxRunningBuilds: ip.Status.Build.MaxRunningBuilds, + maxRunningBuilds: ip.Status.Build.MaxRunningBuilds, + buildOrderStrategy: ip.Status.Build.BuildConfiguration.OrderStrategy, } switch instance.BuilderConfiguration().Strategy { diff --git a/pkg/controller/build/build_monitor.go b/pkg/controller/build/build_monitor.go index 521dd6b866..a60ce1b3af 100644 --- a/pkg/controller/build/build_monitor.go +++ b/pkg/controller/build/build_monitor.go @@ -33,7 +33,8 @@ import ( var runningBuilds sync.Map type Monitor struct { - maxRunningBuilds int32 + maxRunningBuilds int32 + buildOrderStrategy v1.BuildOrderStrategy } func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Build) (bool, error) { @@ -43,15 +44,15 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui return true }) - if runningBuildsTotal >= bm.maxRunningBuilds { - requestName := build.Name - requestNamespace := build.Namespace - buildCreator := kubernetes.GetCamelCreator(build) - if buildCreator != nil { - requestName = buildCreator.Name - requestNamespace = buildCreator.Namespace - } + requestName := build.Name + requestNamespace := build.Namespace + buildCreator := kubernetes.GetCamelCreator(build) + if buildCreator != nil { + requestName = buildCreator.Name + requestNamespace = buildCreator.Namespace + } + 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 gets enqueued", runningBuildsTotal) @@ -84,18 +85,22 @@ func (bm *Monitor) canSchedule(ctx context.Context, c ctrl.Reader, build *v1.Bui return false, err } - // Emulate a serialized working queue to only allow one build to run at a given time. - // This is currently necessary for the incremental build to work as expected. - // We may want to explicitly manage build priority as opposed to relying on - // the reconciliation loop to handle the queuing. - for _, b := range builds.Items { - if b.Status.Phase == v1.BuildPhasePending || b.Status.Phase == v1.BuildPhaseRunning { - // Let's requeue the build in case one is already running - return false, nil - } + var allowed bool + switch bm.buildOrderStrategy { + case v1.BuildOrderStrategyFIFO: + // Check on builds that have been created before the current build and grant precedence if any. + allowed = !builds.HasScheduledBuildsBefore(build) + case v1.BuildOrderStrategySequential: + // Emulate a serialized working queue to only allow one build to run at a given time. + // Let's requeue the build in case one is already running + allowed = !builds.HasRunningBuilds() + default: + Log.WithValues("request-namespace", requestNamespace, "request-name", requestName, "order-strategy", bm.buildOrderStrategy). + ForBuild(build).Infof("Unsupported build order strategy (%s) - the build gets enqueued", bm.buildOrderStrategy) + allowed = false } - return true, nil + return allowed, nil } func monitorRunningBuild(build *v1.Build) { diff --git a/pkg/controller/build/build_monitor_test.go b/pkg/controller/build/build_monitor_test.go index 0dd4ee2d3d..b44cd05336 100644 --- a/pkg/controller/build/build_monitor_test.go +++ b/pkg/controller/build/build_monitor_test.go @@ -30,7 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -func TestMonitorBuilds(t *testing.T) { +func TestMonitorSequentialBuilds(t *testing.T) { testcases := []struct { name string running []*v1.Build @@ -49,7 +49,7 @@ func TestMonitorBuilds(t *testing.T) { name: "allowNewNativeBuild", running: []*v1.Build{}, finished: []*v1.Build{}, - build: newNativeBuild("ns", "my-build"), + build: newNativeBuild("ns", "my-native-build"), allowed: true, }, { @@ -69,7 +69,7 @@ func TestMonitorBuilds(t *testing.T) { newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), newNativeBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), }, - build: newNativeBuild("ns", "my-build"), + build: newNativeBuild("ns", "my-native-build"), allowed: true, }, { @@ -144,7 +144,8 @@ func TestMonitorBuilds(t *testing.T) { assert.Nil(t, err) bm := Monitor{ - maxRunningBuilds: 3, + maxRunningBuilds: 3, + buildOrderStrategy: v1.BuildOrderStrategySequential, } // reset running builds in memory cache @@ -167,7 +168,8 @@ func TestAllowBuildRequeue(t *testing.T) { assert.Nil(t, err) bm := Monitor{ - maxRunningBuilds: 3, + maxRunningBuilds: 3, + buildOrderStrategy: v1.BuildOrderStrategySequential, } runningBuild := newBuild("some-ns", "my-build-1") @@ -191,6 +193,165 @@ func TestAllowBuildRequeue(t *testing.T) { assert.True(t, allowed) } +func TestMonitorFIFOBuilds(t *testing.T) { + testcases := []struct { + name string + running []*v1.Build + builds []*v1.Build + build *v1.Build + allowed bool + }{ + { + name: "allowNewBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newBuild("ns", "my-build"), + allowed: true, + }, + { + name: "allowNewNativeBuild", + running: []*v1.Build{}, + builds: []*v1.Build{}, + build: newNativeBuild("ns1", "my-build"), + allowed: true, + }, + { + name: "allowNewBuildWhenOthersFinished", + running: []*v1.Build{}, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + newBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), + }, + build: newBuild("ns", "my-build"), + allowed: true, + }, + { + name: "allowNewNativeBuildWhenOthersFinished", + running: []*v1.Build{}, + builds: []*v1.Build{ + newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + newNativeBuildInPhase("ns", "my-build-failed", v1.BuildPhaseFailed), + }, + build: newNativeBuild("ns", "my-build"), + allowed: true, + }, + { + name: "limitMaxRunningBuilds", + running: []*v1.Build{ + newBuild("some-ns", "my-build-1"), + newBuild("other-ns", "my-build-2"), + newBuild("another-ns", "my-build-3"), + }, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + }, + build: newBuild("ns", "my-build"), + allowed: false, + }, + { + name: "limitMaxRunningNativeBuilds", + running: []*v1.Build{ + newBuildInPhase("some-ns", "my-build-1", v1.BuildPhaseRunning), + newNativeBuildInPhase("other-ns", "my-build-2", v1.BuildPhaseRunning), + newNativeBuildInPhase("another-ns", "my-build-3", v1.BuildPhaseRunning), + }, + builds: []*v1.Build{ + newNativeBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + }, + build: newNativeBuildInPhase("ns", "my-build", v1.BuildPhaseInitialization), + allowed: false, + }, + { + name: "allowParallelBuildsWithDifferentLayout", + running: []*v1.Build{ + newNativeBuildInPhase("ns", "my-build-1", v1.BuildPhaseRunning), + }, + build: newBuild("ns", "my-build"), + allowed: true, + }, + { + name: "allowParallelBuildsInSameNamespace", + running: []*v1.Build{ + newBuild("ns", "my-build-1"), + newBuild("other-ns", "my-build-2"), + }, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + newBuildInPhase("other-ns", "my-build-new", v1.BuildPhaseScheduling), + }, + build: newBuild("ns", "my-build"), + allowed: true, + }, + { + name: "queueBuildWhenOlderBuildIsAlreadyInitialized", + running: []*v1.Build{ + newBuild("ns", "my-build-1"), + newBuild("other-ns", "my-build-2"), + }, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + newBuildInPhase("ns", "my-build-new", v1.BuildPhaseInitialization), + }, + build: newBuild("ns", "my-build"), + allowed: false, + }, + { + name: "queueBuildWhenOlderBuildIsAlreadyScheduled", + running: []*v1.Build{ + newBuild("ns", "my-build-1"), + newBuild("other-ns", "my-build-2"), + }, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + newBuildInPhase("ns", "my-build-new", v1.BuildPhaseScheduling), + }, + build: newBuild("ns", "my-build"), + allowed: false, + }, + { + name: "allowBuildsInNewNamespace", + running: []*v1.Build{ + newBuild("some-ns", "my-build-1"), + newBuild("other-ns", "my-build-2"), + }, + builds: []*v1.Build{ + newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded), + }, + build: newBuild("ns", "my-build"), + allowed: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + var initObjs []runtime.Object + for _, build := range append(tc.running, tc.builds...) { + initObjs = append(initObjs, build) + } + + c, err := test.NewFakeClient(initObjs...) + + assert.Nil(t, err) + + bm := Monitor{ + maxRunningBuilds: 3, + buildOrderStrategy: v1.BuildOrderStrategyFIFO, + } + + // reset running builds in memory cache + cleanRunningBuildsMonitor() + for _, build := range tc.running { + monitorRunningBuild(build) + } + + allowed, err := bm.canSchedule(context.TODO(), c, tc.build) + + assert.Nil(t, err) + assert.Equal(t, tc.allowed, allowed) + }) + } +} + func cleanRunningBuildsMonitor() { runningBuilds.Range(func(key interface{}, v interface{}) bool { runningBuilds.Delete(key) @@ -226,6 +387,7 @@ func newBuildWithLayoutInPhase(namespace string, name string, layout string, pha Labels: map[string]string{ v1.IntegrationKitLayoutLabel: layout, }, + CreationTimestamp: metav1.NewTime(time.Now()), }, Spec: v1.BuildSpec{ Tasks: []v1.Task{ @@ -233,6 +395,7 @@ func newBuildWithLayoutInPhase(namespace string, name string, layout string, pha Builder: &v1.BuilderTask{ Configuration: v1.BuildConfiguration{ Strategy: v1.BuildStrategyRoutine, + OrderStrategy: v1.BuildOrderStrategySequential, ToolImage: "camel:latest", BuilderPodNamespace: "ns", }, diff --git a/pkg/controller/integrationkit/build.go b/pkg/controller/integrationkit/build.go index 24cf949a74..221fe7b9e1 100644 --- a/pkg/controller/integrationkit/build.go +++ b/pkg/controller/integrationkit/build.go @@ -114,9 +114,16 @@ func (action *buildAction) handleBuildSubmitted(ctx context.Context, kit *v1.Int if buildConfig.IsEmpty() { // default to IntegrationPlatform configuration buildConfig = &env.Platform.Status.Build.BuildConfiguration - } else if buildConfig.Strategy == "" { - // we always need to define a strategy, so we default to platform if none - buildConfig.Strategy = env.Platform.Status.Build.BuildConfiguration.Strategy + } else { + if buildConfig.Strategy == "" { + // we always need to define a strategy, so we default to platform if none + buildConfig.Strategy = env.Platform.Status.Build.BuildConfiguration.Strategy + } + + if buildConfig.OrderStrategy == "" { + // we always need to define an order strategy, so we default to platform if none + buildConfig.OrderStrategy = env.Platform.Status.Build.BuildConfiguration.OrderStrategy + } } // nolint: contextcheck diff --git a/pkg/platform/defaults.go b/pkg/platform/defaults.go index aff2bf41b1..fef1df44d0 100644 --- a/pkg/platform/defaults.go +++ b/pkg/platform/defaults.go @@ -86,6 +86,11 @@ func ConfigureDefaults(ctx context.Context, c client.Client, p *v1.IntegrationPl log.Debugf("Integration Platform %s [%s]: setting build strategy %s", p.Name, p.Namespace, p.Status.Build.BuildConfiguration.Strategy) } + if p.Status.Build.BuildConfiguration.OrderStrategy == "" { + p.Status.Build.BuildConfiguration.OrderStrategy = v1.BuildOrderStrategySequential + log.Debugf("Integration Platform %s [%s]: setting build order strategy %s", p.Name, p.Namespace, p.Status.Build.BuildConfiguration.OrderStrategy) + } + err := setPlatformDefaults(p, verbose) if err != nil { return err @@ -226,6 +231,10 @@ func applyPlatformSpec(source *v1.IntegrationPlatform, target *v1.IntegrationPla target.Status.Build.BuildConfiguration.Strategy = source.Status.Build.BuildConfiguration.Strategy } + if target.Status.Build.BuildConfiguration.OrderStrategy == "" { + target.Status.Build.BuildConfiguration.OrderStrategy = source.Status.Build.BuildConfiguration.OrderStrategy + } + if target.Status.Build.RuntimeVersion == "" { log.Debugf("Integration Platform %s [%s]: setting runtime version", target.Name, target.Namespace) target.Status.Build.RuntimeVersion = source.Status.Build.RuntimeVersion diff --git a/pkg/platform/defaults_test.go b/pkg/platform/defaults_test.go index b2a51fb221..2f05f0452b 100644 --- a/pkg/platform/defaults_test.go +++ b/pkg/platform/defaults_test.go @@ -40,7 +40,8 @@ func TestApplyGlobalPlatformSpec(t *testing.T) { Spec: v1.IntegrationPlatformSpec{ Build: v1.IntegrationPlatformBuildSpec{ BuildConfiguration: v1.BuildConfiguration{ - Strategy: v1.BuildStrategyRoutine, + Strategy: v1.BuildStrategyRoutine, + OrderStrategy: v1.BuildOrderStrategyFIFO, }, Maven: v1.MavenSpec{ Properties: map[string]string{ @@ -83,6 +84,7 @@ func TestApplyGlobalPlatformSpec(t *testing.T) { assert.Equal(t, v1.IntegrationPlatformClusterOpenShift, ip.Status.Cluster) assert.Equal(t, v1.TraitProfileOpenShift, ip.Status.Profile) assert.Equal(t, v1.BuildStrategyRoutine, ip.Status.Build.BuildConfiguration.Strategy) + assert.Equal(t, v1.BuildOrderStrategyFIFO, ip.Status.Build.BuildConfiguration.OrderStrategy) assert.True(t, ip.Status.Build.MaxRunningBuilds == 3) // default for build strategy routine assert.Equal(t, len(global.Status.Build.Maven.CLIOptions), len(ip.Status.Build.Maven.CLIOptions)) assert.Equal(t, global.Status.Build.Maven.CLIOptions, ip.Status.Build.Maven.CLIOptions) @@ -187,7 +189,8 @@ func TestRetainLocalPlatformSpec(t *testing.T) { Spec: v1.IntegrationPlatformSpec{ Build: v1.IntegrationPlatformBuildSpec{ BuildConfiguration: v1.BuildConfiguration{ - Strategy: v1.BuildStrategyRoutine, + Strategy: v1.BuildStrategyRoutine, + OrderStrategy: v1.BuildOrderStrategySequential, }, Maven: v1.MavenSpec{ Properties: map[string]string{ @@ -224,7 +227,8 @@ func TestRetainLocalPlatformSpec(t *testing.T) { Spec: v1.IntegrationPlatformSpec{ Build: v1.IntegrationPlatformBuildSpec{ BuildConfiguration: v1.BuildConfiguration{ - Strategy: v1.BuildStrategyPod, + Strategy: v1.BuildStrategyPod, + OrderStrategy: v1.BuildOrderStrategyFIFO, }, MaxRunningBuilds: 1, Maven: v1.MavenSpec{ @@ -251,6 +255,7 @@ func TestRetainLocalPlatformSpec(t *testing.T) { assert.Equal(t, v1.IntegrationPlatformClusterKubernetes, ip.Status.Cluster) assert.Equal(t, v1.TraitProfileKnative, ip.Status.Profile) assert.Equal(t, v1.BuildStrategyPod, ip.Status.Build.BuildConfiguration.Strategy) + assert.Equal(t, v1.BuildOrderStrategyFIFO, ip.Status.Build.BuildConfiguration.OrderStrategy) assert.True(t, ip.Status.Build.MaxRunningBuilds == 1) assert.Equal(t, len(global.Status.Build.Maven.CLIOptions), len(ip.Status.Build.Maven.CLIOptions)) assert.Equal(t, global.Status.Build.Maven.CLIOptions, ip.Status.Build.Maven.CLIOptions) diff --git a/pkg/trait/builder.go b/pkg/trait/builder.go index 6e583d88cc..af584f22c2 100644 --- a/pkg/trait/builder.go +++ b/pkg/trait/builder.go @@ -175,9 +175,10 @@ func (t *builderTrait) builderTask(e *Environment) (*v1.BuilderTask, error) { if trait := e.Catalog.GetTrait(quarkusTraitID); trait != nil { // The builder trait must define certain resources requirements when we have a native build if quarkus, ok := trait.(*quarkusTrait); ok && pointer.BoolDeref(quarkus.Enabled, true) && quarkus.isNativeIntegration(e) { - // Force the build to run in a separate Pod + // Force the build to run in a separate Pod and strictly sequential t.L.Info("This is a Quarkus native build: setting build configuration with build Pod strategy, 1 CPU core and 4 GiB memory. Make sure your cluster can handle it.") t.Strategy = string(v1.BuildStrategyPod) + t.OrderStrategy = string(v1.BuildOrderStrategySequential) t.RequestCPU = "1000m" t.RequestMemory = "4Gi" } @@ -192,13 +193,39 @@ func (t *builderTrait) builderTask(e *Environment) (*v1.BuilderTask, error) { if t.Strategy != "" { t.L.Infof("User defined build strategy %s", t.Strategy) - switch t.Strategy { - case string(v1.BuildStrategyPod): - buildConfig.Strategy = v1.BuildStrategyPod - case string(v1.BuildStrategyRoutine): - buildConfig.Strategy = v1.BuildStrategyRoutine - default: - return nil, fmt.Errorf("must specify either pod or routine build strategy, unknown %s", t.Strategy) + found := false + for _, s := range v1.BuildStrategies { + if string(s) == t.Strategy { + found = true + buildConfig.Strategy = s + break + } + } + if !found { + var strategies []string + for _, s := range v1.BuildStrategies { + strategies = append(strategies, string(s)) + } + return nil, fmt.Errorf("unknown build strategy: %s. One of [%s] is expected", t.Strategy, strings.Join(strategies, ", ")) + } + } + + if t.OrderStrategy != "" { + t.L.Infof("User defined build order strategy %s", t.OrderStrategy) + found := false + for _, s := range v1.BuildOrderStrategies { + if string(s) == t.OrderStrategy { + found = true + buildConfig.OrderStrategy = s + break + } + } + if !found { + var strategies []string + for _, s := range v1.BuildOrderStrategies { + strategies = append(strategies, string(s)) + } + return nil, fmt.Errorf("unknown build order strategy: %s. One of [%s] is expected", t.OrderStrategy, strings.Join(strategies, ", ")) } }