From db8c0fa242147289f6b8385d147159d7bf987b54 Mon Sep 17 00:00:00 2001 From: Luis Sergio Carneiro Date: Wed, 14 Aug 2024 14:47:37 -0300 Subject: [PATCH] bugfix(#5758): changing dependencies build order strategy to be compatible with the incremental image build algorithm --- pkg/apis/camel/v1/build_type_support_test.go | 104 ++++++++++++++++++- pkg/apis/camel/v1/build_types_support.go | 23 ++-- 2 files changed, 112 insertions(+), 15 deletions(-) diff --git a/pkg/apis/camel/v1/build_type_support_test.go b/pkg/apis/camel/v1/build_type_support_test.go index 39e9a38162..36ac9b3788 100644 --- a/pkg/apis/camel/v1/build_type_support_test.go +++ b/pkg/apis/camel/v1/build_type_support_test.go @@ -403,14 +403,108 @@ func TestMatchingBuildsSchedulingSameDependenciesSameRuntime(t *testing.T) { Items: []Build{buildA, buildB}, } - // ebuilds have the same dependencies, runtime and creation timestamp + // builds have the same dependencies, runtime and creation timestamp + // buildB should wait for buildA 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) + assert.Equal(t, buildA.Name, buildMatch.Name) +} + +func TestMatchingBuildsSchedulingMutipleSubsets(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:component-a", + }, + 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:component-a", + "camel:component-b", + }, + Runtime: RuntimeSpec{ + Version: "3.8.1", + }, + }, + }, + }, + }, + Status: BuildStatus{ + Phase: BuildPhaseScheduling, + }, + } + buildC := Build{ + ObjectMeta: v1.ObjectMeta{ + Name: "buildC", + CreationTimestamp: creationTimestamp, + }, + Spec: BuildSpec{ + Tasks: []Task{ + { + Builder: &BuilderTask{ + Dependencies: []string{ + "camel:component-a", + "camel:component-b", + "camel:component-c", + }, + Runtime: RuntimeSpec{ + Version: "3.8.1", + }, + }, + }, + }, + }, + Status: BuildStatus{ + Phase: BuildPhaseScheduling, + }, + } + + buildList := BuildList{ + Items: []Build{buildA, buildB, buildC}, + } + + // buildA is a subset of buildB, which is a subset of buildC + // buildC should wait for B, which should wait for A + + matches, buildMatch := buildList.HasMatchingBuild(&buildA) + assert.False(t, matches) + assert.Nil(t, buildMatch) + matches, buildMatch = buildList.HasMatchingBuild(&buildB) + assert.True(t, matches) + assert.Equal(t, buildA.Name, buildMatch.Name) + matches, buildMatch = buildList.HasMatchingBuild(&buildC) + assert.True(t, matches) + assert.Equal(t, buildB.Name, buildMatch.Name) } func TestMatchingBuildsSchedulingFewCommonDependencies(t *testing.T) { @@ -475,12 +569,12 @@ func TestMatchingBuildsSchedulingFewCommonDependencies(t *testing.T) { Items: []Build{buildA, buildB}, } - // builds have only 1 out of 10 required dependencies. they should not match + // buildA is a subset of buildB. should wait for it 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) + assert.True(t, matches) + assert.Equal(t, buildA.Name, buildMatch.Name) } diff --git a/pkg/apis/camel/v1/build_types_support.go b/pkg/apis/camel/v1/build_types_support.go index 80bba1f0cc..1c01cd5d52 100644 --- a/pkg/apis/camel/v1/build_types_support.go +++ b/pkg/apis/camel/v1/build_types_support.go @@ -288,8 +288,11 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) { } runtimeVersion := build.RuntimeVersion() + var bestBuild Build + bestBuildCommonDependencies := 0 buildLoop: - for _, b := range bl.Items { + for i := range bl.Items { + b := bl.Items[i] if b.Name == build.Name || b.Status.IsFinished() { continue } @@ -323,8 +326,8 @@ buildLoop: } } - if commonDependencies < len(required)/2 { - // few common dependencies + if commonDependencies == 0 { + // no common dependencies continue } @@ -335,23 +338,23 @@ buildLoop: case BuildPhaseInitialization, BuildPhaseScheduling: // handle suitable scheduled build - // the build has at least half the dependencies, maybe all of them - if compareBuilds(&b, build) < 0 { - return true, &b - } if missing == 0 { // seems like both builds require exactly the same list of dependencies // additionally check for the creation timestamp if compareBuilds(&b, build) < 0 { return true, &b } - } else { - // some deps are missing, but this build has at least half the deps we need and no extra dependency. - return true, &b + } else if commonDependencies > bestBuildCommonDependencies { + bestBuildCommonDependencies = commonDependencies + bestBuild = b + continue } } } + if bestBuildCommonDependencies > 0 { + return true, &bestBuild + } return false, nil }