This is an automated email from the ASF dual-hosted git repository.
pcongiusti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit 3a4338e4cee66c2787544793a1257dc7ea95549b
Author: Luis Sergio Carneiro <luis.carne...@sensedia.com>
AuthorDate: Fri Aug 9 12:10:37 2024 -0300
bugfix(#5755): avoiding deadlock between builds with dependencies build
order strategy when the builds share enough dependencies
---
pkg/apis/camel/v1/build_type_support_test.go | 301 +++++++++++++++++++++++++++
pkg/apis/camel/v1/build_types_support.go | 46 +++-
2 files changed, 341 insertions(+), 6 deletions(-)
diff --git a/pkg/apis/camel/v1/build_type_support_test.go
b/pkg/apis/camel/v1/build_type_support_test.go
index 7fba1de19..01a635310 100644
--- a/pkg/apis/camel/v1/build_type_support_test.go
+++ b/pkg/apis/camel/v1/build_type_support_test.go
@@ -19,6 +19,7 @@ package v1
import (
"testing"
+ "time"
"github.com/stretchr/testify/assert"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -37,6 +38,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:timer",
"camel:log",
},
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
},
},
},
@@ -58,6 +62,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:log",
"camel:bean",
},
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
},
},
},
@@ -80,6 +87,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:bean",
"camel:zipfile",
},
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
},
},
},
@@ -101,6 +111,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:component-a",
"camel:component-b",
},
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
},
},
},
@@ -126,3 +139,291 @@ func TestMatchingBuildsPending(t *testing.T) {
assert.False(t, matches)
assert.Nil(t, buildMatch)
}
+
+func TestMatchingBuildsSchedulingSharedDependencies(t *testing.T) {
+ timestamp, _ := time.Parse("2006-01-02T15:04:05-0700",
"2024-08-09T10:00:00Z")
+ creationTimestamp := v1.Time{Time: timestamp}
+ buildA := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildA",
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:core",
+ "camel:rest",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+ buildB := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildB",
+ CreationTimestamp: creationTimestamp,
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+
"mvn:org.apache.camel.k:camel-k-cron",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ }},
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+
+ buildList := BuildList{
+ Items: []Build{buildA, buildB},
+ }
+
+ // both builds share dependencies and have the same creationTimestamp
+ // buildA should be prioritized so there should be not matching build
for it
+
+ matches, buildMatch := buildList.HasMatchingBuild(&buildA)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+ matches, buildMatch = buildList.HasMatchingBuild(&buildB)
+ assert.True(t, matches)
+ assert.True(t, buildMatch.Name == buildA.Name)
+}
+
+func TestMatchingBuildsSchedulingSameDependenciesDIfferentRuntimes(t
*testing.T) {
+ timestamp, _ := time.Parse("2006-01-02T15:04:05-0700",
"2024-08-09T10:00:00Z")
+ creationTimestamp := v1.Time{Time: timestamp}
+ buildA := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildA",
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+
"mvn:org.apache.camel.k:camel-k-cron",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+ buildB := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildB",
+ CreationTimestamp: creationTimestamp,
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+
"mvn:org.apache.camel.k:camel-k-cron",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.2.3",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+
+ buildList := BuildList{
+ Items: []Build{buildA, buildB},
+ }
+
+ // each build uses a different runtime, so they should not match
+
+ matches, buildMatch := buildList.HasMatchingBuild(&buildA)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+ matches, buildMatch = buildList.HasMatchingBuild(&buildB)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+}
+
+func TestMatchingBuildsSchedulingSameDependenciesSameRuntime(t *testing.T) {
+ timestamp, _ := time.Parse("2006-01-02T15:04:05-0700",
"2024-08-09T10:00:00Z")
+ creationTimestamp := v1.Time{Time: timestamp}
+ buildA := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildA",
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+
"mvn:org.apache.camel.k:camel-k-cron",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+ buildB := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildB",
+ CreationTimestamp: creationTimestamp,
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+
"mvn:org.apache.camel.k:camel-k-cron",
+
"mvn:org.apache.camel.k:camel-k-runtime",
+
"mvn:org.apache.camel.quarkus:camel-quarkus-yaml-dsl",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+
+ buildList := BuildList{
+ Items: []Build{buildA, buildB},
+ }
+
+ // ebuilds have the same dependencies, runtime and creation timestamp
+
+ matches, buildMatch := buildList.HasMatchingBuild(&buildA)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+ matches, buildMatch = buildList.HasMatchingBuild(&buildB)
+ assert.True(t, matches)
+ assert.True(t, buildMatch.Name == buildA.Name)
+}
+
+func TestMatchingBuildsSchedulingFewCommonDependencies(t *testing.T) {
+ timestamp, _ := time.Parse("2006-01-02T15:04:05-0700",
"2024-08-09T10:00:00Z")
+ creationTimestamp := v1.Time{Time: timestamp}
+ buildA := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildA",
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+ "camel:componenta1",
+ "camel:componentb1",
+ "camel:componentc1",
+ "camel:componentd1",
+ "camel:componente1",
+ "camel:componentf1",
+ "camel:componentg1",
+ "camel:componenth1",
+ "camel:componenti1",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+ buildB := Build{
+ ObjectMeta: v1.ObjectMeta{
+ Name: "buildB",
+ CreationTimestamp: creationTimestamp,
+ },
+ Spec: BuildSpec{
+ Tasks: []Task{
+ {
+ Builder: &BuilderTask{
+ Dependencies: []string{
+ "camel:quartz",
+ "camel:componenta2",
+ "camel:componentb2",
+ "camel:componentc2",
+ "camel:componentd2",
+ "camel:componente2",
+ "camel:componentf2",
+ "camel:componentg2",
+ "camel:componenth2",
+ "camel:componenti2",
+ },
+ Runtime: RuntimeSpec{
+ Version: "3.8.1",
+ },
+ },
+ },
+ },
+ },
+ Status: BuildStatus{
+ Phase: BuildPhaseScheduling,
+ },
+ }
+
+ buildList := BuildList{
+ Items: []Build{buildA, buildB},
+ }
+
+ // builds have only 1 out of 10 shared dependencies. they should not
match
+
+ matches, buildMatch := buildList.HasMatchingBuild(&buildA)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+ matches, buildMatch = buildList.HasMatchingBuild(&buildB)
+ assert.False(t, matches)
+ assert.Nil(t, buildMatch)
+}
diff --git a/pkg/apis/camel/v1/build_types_support.go
b/pkg/apis/camel/v1/build_types_support.go
index a264e5168..978c941f3 100644
--- a/pkg/apis/camel/v1/build_types_support.go
+++ b/pkg/apis/camel/v1/build_types_support.go
@@ -18,6 +18,8 @@ limitations under the License.
package v1
import (
+ "strings"
+
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
@@ -73,6 +75,14 @@ func (build *Build) BuilderDependencies() []string {
return []string{}
}
+func (build *Build) RuntimeVersion() *string {
+ if builder, ok := FindBuilderTask(build.Spec.Tasks); ok {
+ return &builder.Runtime.Version
+ }
+
+ return nil
+}
+
// FindBuilderTask returns the 1st builder task from the task list.
func FindBuilderTask(tasks []Task) (*BuilderTask, bool) {
for _, t := range tasks {
@@ -272,11 +282,17 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool,
*Build) {
if len(required) == 0 {
return false, nil
}
+ runtimeVersion := build.RuntimeVersion()
for _, b := range bl.Items {
if b.Name == build.Name || b.Status.IsFinished() {
continue
}
+ bRuntimeVersion := b.RuntimeVersion()
+
+ if *runtimeVersion != *bRuntimeVersion {
+ continue
+ }
dependencies := b.BuilderDependencies()
dependencyMap := make(map[string]int, len(dependencies))
@@ -286,16 +302,17 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool,
*Build) {
allMatching := true
missing := 0
+ commonDependencies := 0
for _, item := range required {
if _, ok := dependencyMap[item]; !ok {
allMatching = false
missing++
+ } else {
+ commonDependencies++
}
}
- // Heuristic approach: if there are too many unrelated
libraries then this image is
- // not suitable to be used as base image
- if !allMatching && missing > len(required)/2 {
+ if commonDependencies < len(required)/2 {
continue
}
@@ -311,10 +328,27 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool,
*Build) {
// additionally check for the creation timestamp
if
b.CreationTimestamp.Before(&build.CreationTimestamp) {
return true, &b
+ } else if
b.CreationTimestamp.Equal(&build.CreationTimestamp) {
+ if strings.Compare(b.Name, build.Name)
< 0 {
+ return true, &b
+ }
+ }
+
+ } else if !allMatching && commonDependencies > 0 {
+ // there are common dependencies. let's compare
the total number of dependencies
+ // in each build. whichever build has less
dependencies should run first
+ if len(dependencies) < len(required) {
+ return true, &b
+ } else if len(dependencies) == len(required) {
+ // same number of total dependencies.
+ if
b.CreationTimestamp.Before(&build.CreationTimestamp) {
+ return true, &b
+ } else if
b.CreationTimestamp.Equal(&build.CreationTimestamp) &&
+ strings.Compare(b.Name,
build.Name) < 0 {
+ return true, &b
+ }
}
- } else if missing > 0 {
- // found another suitable scheduled build with
fewer dependencies that should build first in order to reuse the produced image
- return true, &b
+ continue
}
}
}
