bugfix(#5758): changing dependencies build order strategy to be compa…

…tible with the incremental image build algorithm
apache · Aug 16, 2024 · db8c0fa · db8c0fa
1 parent 0b8f885
commit db8c0fa
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 15 deletions.
diff --git 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
@@ -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
 }