bugfix(#5758): changing incremental image build to not allow extra de…

…pendencies

pkg/apis/camel/v1/build_type_support_test.go

@@ -178,8 +178,75 @@ func TestMatchingBuildsSchedulingSharedDependencies(t *testing.T) {
 				{
 					Builder: &BuilderTask{
 						Dependencies: []string{
+							"camel:core",
+							"camel:rest",
+						},
+						Runtime: RuntimeSpec{
+							Version: "3.8.1",
+						}},
+				},
+			},
+		},
+		Status: BuildStatus{
+			Phase: BuildPhaseScheduling,
+		},
+	}
+
+	buildList := BuildList{
+		Items: []Build{buildA, buildB},
+	}
+
+	// buildB contains a subset of buildA dependencies
+	// buildA should wait for it
+
+	matches, buildMatch := buildList.HasMatchingBuild(&buildA)
+	assert.True(t, matches)
+	assert.True(t, buildMatch.Name == buildB.Name)
+	matches, buildMatch = buildList.HasMatchingBuild(&buildB)
+	assert.False(t, matches)
+	assert.Nil(t, buildMatch)
+}
+
+func TestMatchingBuildsSchedulingSharedDependenciesWithSurplus(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:core",
 							"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",
 						},
@@ -198,15 +265,14 @@ func TestMatchingBuildsSchedulingSharedDependencies(t *testing.T) {
 		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
+	// no build is a subset of the other
 
 	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.False(t, matches)
+	assert.Nil(t, buildMatch)
 }
 
 func TestMatchingBuildsSchedulingSameDependenciesDIfferentRuntimes(t *testing.T) {
@@ -360,15 +426,6 @@ func TestMatchingBuildsSchedulingFewCommonDependencies(t *testing.T) {
 					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",
@@ -418,7 +475,7 @@ func TestMatchingBuildsSchedulingFewCommonDependencies(t *testing.T) {
 		Items: []Build{buildA, buildB},
 	}
 
-	// builds have only 1 out of 10 shared dependencies. they should not match
+	// builds have only 1 out of 10 required dependencies. they should not match
 
 	matches, buildMatch := buildList.HasMatchingBuild(&buildA)
 	assert.False(t, matches)

pkg/apis/camel/v1/build_types_support.go

@@ -282,10 +282,14 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) {
 	if len(required) == 0 {
 		return false, nil
 	}
+	requiredMap := make(map[string]int, len(required))
+	for i, item := range required {
+		requiredMap[item] = i
+	}
 	runtimeVersion := build.RuntimeVersion()
 
-	for i := range bl.Items {
-		b := bl.Items[i]
+buildLoop:
+	for _, b := range bl.Items {
 		if b.Name == build.Name || b.Status.IsFinished() {
 			continue
 		}
@@ -301,19 +305,26 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) {
 			dependencyMap[item] = i
 		}
 
-		allMatching := true
+		// check if this build has any extra dependency. If it does, we can't use it
+		for _, item := range dependencies {
+			if _, ok := requiredMap[item]; !ok {
+				continue buildLoop
+			}
+		}
+
+		// now check how many common dependencies this build has
 		missing := 0
 		commonDependencies := 0
 		for _, item := range required {
 			if _, ok := dependencyMap[item]; !ok {
-				allMatching = false
 				missing++
 			} else {
 				commonDependencies++
 			}
 		}
 
 		if commonDependencies < len(required)/2 {
+			// few common dependencies
 			continue
 		}
 
@@ -323,20 +334,20 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) {
 			return true, &b
 		case BuildPhaseInitialization, BuildPhaseScheduling:
 			// handle suitable scheduled build
-			if allMatching && len(required) == len(dependencies) {
+
+			// 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 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) ||
-					len(dependencies) == len(required) && compareBuilds(&b, build) < 0 {
-					return true, &b
-				}
-				continue
+			} else {
+				// some deps are missing, but this build has at least half the deps we need and no extra dependency.
+				return true, &b
 			}
 		}
 	}

pkg/builder/image.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 
 	"github.com/apache/camel-k/v2/pkg/util/io"
+	"github.com/apache/camel-k/v2/pkg/util/log"
 
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
@@ -134,6 +135,7 @@ func incrementalImageContext(ctx *builderContext) error {
 		bestImage, commonLibs := findBestImage(images, ctx.Artifacts)
 		if bestImage.Image != "" {
 
+			log.Infof("Selected %s as base image for %s", bestImage.Image, ctx.Build.Name)
 			ctx.BaseImage = bestImage.Image
 			ctx.SelectedArtifacts = make([]v1.Artifact, 0)
 
@@ -234,7 +236,6 @@ func findBestImage(images []v1.IntegrationKitStatus, artifacts []v1.Artifact) (v
 	}
 
 	bestImageCommonLibs := make(map[string]bool)
-	bestImageSurplusLibs := 0
 
 	for _, image := range images {
 		common := make(map[string]bool)
@@ -253,16 +254,14 @@ func findBestImage(images []v1.IntegrationKitStatus, artifacts []v1.Artifact) (v
 		numCommonLibs := len(common)
 		surplus := len(image.Artifacts) - numCommonLibs
 
-		if numCommonLibs != len(image.Artifacts) && surplus >= numCommonLibs/3 {
-			// Heuristic approach: if there are too many unrelated libraries then this image is
-			// not suitable to be used as base image
+		if surplus > 0 {
+			// the base image cannot have extra libs that we don't need
 			continue
 		}
 
-		if numCommonLibs > len(bestImageCommonLibs) || (numCommonLibs == len(bestImageCommonLibs) && surplus < bestImageSurplusLibs) {
+		if numCommonLibs >= len(bestImageCommonLibs) {
 			bestImage = image
 			bestImageCommonLibs = common
-			bestImageSurplusLibs = surplus
 		}
 	}