You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by pc...@apache.org on 2024/08/19 12:13:20 UTC
(camel-k) branch main updated (b8e42f3a6 -> 74860143a)
This is an automated email from the ASF dual-hosted git repository.
pcongiusti pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git
from b8e42f3a6 fix(doc): registry link
new 69c0eb170 bugfix(#5758): changing incremental image build to not allow extra dependencies
new d66be5ed4 bugfix(#5758): changing dependencies build order strategy to be compatible with the incremental image build algorithm
new 74860143a bugfix(#5758): incremental image build should not consider the application jar when matching dependencies
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
pkg/apis/camel/v1/build_type_support_test.go | 189 +++++++++--
pkg/apis/camel/v1/build_types_support.go | 39 ++-
pkg/builder/image.go | 23 +-
pkg/builder/image_test.go | 449 +++++++++++++++++++++++++++
pkg/controller/build/build_monitor_test.go | 4 +-
5 files changed, 664 insertions(+), 40 deletions(-)
(camel-k) 01/03: bugfix(#5758): changing incremental image build to not allow extra dependencies
Posted by pc...@apache.org.
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 69c0eb170a08c313f9cd98df36375d1b02f15ced
Author: Luis Sergio Carneiro <lu...@sensedia.com>
AuthorDate: Tue Aug 13 20:29:53 2024 -0300
bugfix(#5758): changing incremental image build to not allow extra dependencies
---
pkg/apis/camel/v1/build_type_support_test.go | 87 +++++--
pkg/apis/camel/v1/build_types_support.go | 34 ++-
pkg/builder/image.go | 11 +-
pkg/builder/image_test.go | 362 +++++++++++++++++++++++++++
pkg/controller/build/build_monitor_test.go | 4 +-
5 files changed, 464 insertions(+), 34 deletions(-)
diff --git a/pkg/apis/camel/v1/build_type_support_test.go b/pkg/apis/camel/v1/build_type_support_test.go
index 01a635310..39e9a3816 100644
--- a/pkg/apis/camel/v1/build_type_support_test.go
+++ b/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)
diff --git a/pkg/apis/camel/v1/build_types_support.go b/pkg/apis/camel/v1/build_types_support.go
index c66930f72..80bba1f0c 100644
--- a/pkg/apis/camel/v1/build_types_support.go
+++ b/pkg/apis/camel/v1/build_types_support.go
@@ -282,8 +282,13 @@ 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()
+buildLoop:
for _, b := range bl.Items {
if b.Name == build.Name || b.Status.IsFinished() {
continue
@@ -300,12 +305,18 @@ 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++
@@ -313,6 +324,7 @@ func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) {
}
if commonDependencies < len(required)/2 {
+ // few common dependencies
continue
}
@@ -322,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
}
}
}
diff --git a/pkg/builder/image.go b/pkg/builder/image.go
index 25d7de5b4..852326218 100644
--- a/pkg/builder/image.go
+++ 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
}
}
diff --git a/pkg/builder/image_test.go b/pkg/builder/image_test.go
index e0fce1e5f..7fcfa7eb9 100644
--- a/pkg/builder/image_test.go
+++ b/pkg/builder/image_test.go
@@ -93,3 +93,365 @@ func TestListPublishedImages(t *testing.T) {
assert.Len(t, i, 1)
assert.Equal(t, "image-2", i[0].Image)
}
+
+func TestFindBestImageExactMatch(t *testing.T) {
+ requiredArtifacts := []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ }
+ iks := []v1.IntegrationKitStatus{
+ {
+ // missing dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ },
+ },
+ {
+ // exact match
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ },
+ },
+ {
+ // extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ },
+ {
+ // missing and extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ }}
+
+ bestImage, commonLibs := findBestImage(iks, requiredArtifacts)
+ assert.NotNil(t, bestImage)
+ assert.Equal(t, iks[1], bestImage) // exact match
+
+ assert.NotNil(t, commonLibs)
+ assert.Equal(t, 3, len(commonLibs))
+ assert.True(t, commonLibs["artifact-1"])
+ assert.True(t, commonLibs["artifact-2"])
+ assert.True(t, commonLibs["artifact-3"])
+
+}
+
+func TestFindBestImageNoExactMatch(t *testing.T) {
+ requiredArtifacts := []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ }
+ iks := []v1.IntegrationKitStatus{
+ {
+ // missing 2 dependencies
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ },
+ },
+ {
+ // missing dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ },
+ },
+ {
+ // extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ },
+ {
+ // missing and extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ }}
+
+ bestImage, commonLibs := findBestImage(iks, requiredArtifacts)
+ assert.NotNil(t, bestImage)
+ assert.Equal(t, iks[1], bestImage) // missing only 1 dependency and no surplus
+
+ assert.NotNil(t, commonLibs)
+ assert.Equal(t, 2, len(commonLibs))
+ assert.True(t, commonLibs["artifact-1"])
+ assert.True(t, commonLibs["artifact-3"])
+
+}
+
+func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
+ requiredArtifacts := []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ }
+ iks := []v1.IntegrationKitStatus{
+ {
+ // missing 2 dependencies
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ },
+ },
+ {
+ // missing dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "bad-checksum",
+ ID: "artifact-3",
+ },
+ },
+ },
+ {
+ // extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ },
+ {
+ // missing and extra dependency
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ }}
+
+ bestImage, commonLibs := findBestImage(iks, requiredArtifacts)
+ assert.NotNil(t, bestImage)
+ assert.Equal(t, iks[0], bestImage) // 2 missing dependencies and no surplus
+
+ assert.NotNil(t, commonLibs)
+ assert.Equal(t, 1, len(commonLibs))
+ assert.True(t, commonLibs["artifact-1"])
+
+}
+
+func TestFindBestImageAllImagesWithSurplus(t *testing.T) {
+ requiredArtifacts := []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ }
+ iks := []v1.IntegrationKitStatus{
+ {
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "5",
+ ID: "artifact-5",
+ },
+ },
+ },
+ {
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ },
+ },
+ {
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "3",
+ ID: "artifact-3",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ {
+ Checksum: "5",
+ ID: "artifact-5",
+ },
+ },
+ },
+ {
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "1",
+ ID: "artifact-1",
+ },
+ {
+ Checksum: "2",
+ ID: "artifact-2",
+ },
+ {
+ Checksum: "4",
+ ID: "artifact-4",
+ },
+ {
+ Checksum: "6",
+ ID: "artifact-6",
+ },
+ },
+ }}
+
+ bestImage, commonLibs := findBestImage(iks, requiredArtifacts)
+ assert.Equal(t, "", bestImage.Image)
+ assert.Empty(t, commonLibs)
+}
diff --git a/pkg/controller/build/build_monitor_test.go b/pkg/controller/build/build_monitor_test.go
index 9b75eed48..d6b9b1b8f 100644
--- a/pkg/controller/build/build_monitor_test.go
+++ b/pkg/controller/build/build_monitor_test.go
@@ -555,14 +555,14 @@ func TestMonitorDependencyMatchingBuilds(t *testing.T) {
"Waiting for build (my-build-pending) to finish in order to use incremental image builds - the build (my-build) gets enqueued"),
},
{
- name: "queueBuildWhenSuitableBuildWithSupersetDependenciesIsAlreadyRunning",
+ name: "queueBuildWhenSuitableBuildWithSubsetDependenciesIsAlreadyRunning",
running: []*v1.Build{
newBuild("some-ns", "my-build-1", deps...),
newBuild("other-ns", "my-build-2", deps...),
},
builds: []*v1.Build{
newBuildInPhase("ns", "my-build-x", v1.BuildPhaseSucceeded, deps...),
- newBuildInPhase("ns", "my-build-running", v1.BuildPhaseRunning, append(deps, "camel:test")...),
+ newBuildInPhase("ns", "my-build-running", v1.BuildPhaseRunning, deps[0:len(deps)-1]...),
},
build: newBuild("ns", "my-build", deps...),
allowed: false,
(camel-k) 03/03: bugfix(#5758): incremental image build should not consider the application jar when matching dependencies
Posted by pc...@apache.org.
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 74860143a7eb8b63bea247b62065d5e541cb2cd3
Author: Luis Sergio Carneiro <lu...@sensedia.com>
AuthorDate: Fri Aug 16 18:50:14 2024 -0300
bugfix(#5758): incremental image build should not consider the application jar when matching dependencies
---
pkg/builder/image.go | 12 ++++++-
pkg/builder/image_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/pkg/builder/image.go b/pkg/builder/image.go
index 852326218..af79056ae 100644
--- a/pkg/builder/image.go
+++ b/pkg/builder/image.go
@@ -21,6 +21,7 @@ import (
"os"
"path"
"path/filepath"
+ "strings"
"github.com/apache/camel-k/v2/pkg/util/io"
"github.com/apache/camel-k/v2/pkg/util/log"
@@ -238,8 +239,17 @@ func findBestImage(images []v1.IntegrationKitStatus, artifacts []v1.Artifact) (v
bestImageCommonLibs := make(map[string]bool)
for _, image := range images {
+ nonLibArtifacts := 0
common := make(map[string]bool)
+
for _, artifact := range image.Artifacts {
+ // the application artifacts should not be considered as dependencies for image reuse
+ // otherwise, checksums would never match and we would always use the root image
+ if !strings.HasPrefix(artifact.Target, "dependencies/lib") {
+ nonLibArtifacts++
+ continue
+ }
+
// If the Artifact's checksum is not defined we can't reliably determine if for some
// reason the artifact has been changed but not the ID (as example for snapshots or
// other generated jar) thus we do not take this artifact into account.
@@ -252,7 +262,7 @@ func findBestImage(images []v1.IntegrationKitStatus, artifacts []v1.Artifact) (v
}
numCommonLibs := len(common)
- surplus := len(image.Artifacts) - numCommonLibs
+ surplus := len(image.Artifacts) - numCommonLibs - nonLibArtifacts
if surplus > 0 {
// the base image cannot have extra libs that we don't need
diff --git a/pkg/builder/image_test.go b/pkg/builder/image_test.go
index 7fcfa7eb9..0014c48dc 100644
--- a/pkg/builder/image_test.go
+++ b/pkg/builder/image_test.go
@@ -99,14 +99,17 @@ func TestFindBestImageExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
}
iks := []v1.IntegrationKitStatus{
@@ -116,10 +119,12 @@ func TestFindBestImageExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
},
},
@@ -129,14 +134,17 @@ func TestFindBestImageExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
},
},
@@ -146,18 +154,22 @@ func TestFindBestImageExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
},
@@ -167,14 +179,17 @@ func TestFindBestImageExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
}}
@@ -196,14 +211,17 @@ func TestFindBestImageNoExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
}
iks := []v1.IntegrationKitStatus{
@@ -213,6 +231,7 @@ func TestFindBestImageNoExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
},
},
@@ -222,10 +241,12 @@ func TestFindBestImageNoExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
},
},
@@ -235,18 +256,22 @@ func TestFindBestImageNoExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
},
@@ -256,14 +281,17 @@ func TestFindBestImageNoExactMatch(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
}}
@@ -284,14 +312,17 @@ func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
}
iks := []v1.IntegrationKitStatus{
@@ -301,6 +332,7 @@ func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
},
},
@@ -310,10 +342,12 @@ func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "bad-checksum",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
},
},
@@ -323,18 +357,22 @@ func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "3",
ID: "artifact-3",
+ Target: "dependencies/lib/main/artifact-3.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
},
@@ -344,14 +382,17 @@ func TestFindBestImageNoExactMatchBadChecksum(t *testing.T) {
{
Checksum: "1",
ID: "artifact-1",
+ Target: "dependencies/lib/main/artifact-1.jar",
},
{
Checksum: "2",
ID: "artifact-2",
+ Target: "dependencies/lib/main/artifact-2.jar",
},
{
Checksum: "4",
ID: "artifact-4",
+ Target: "dependencies/lib/main/artifact-4.jar",
},
},
}}
@@ -455,3 +496,49 @@ func TestFindBestImageAllImagesWithSurplus(t *testing.T) {
assert.Equal(t, "", bestImage.Image)
assert.Empty(t, commonLibs)
}
+
+func TestFindBestImageShouldIgnoreIntegrationArfifact(t *testing.T) {
+ requiredArtifacts := []v1.Artifact{
+ {
+ Checksum: "sha1:eMo2eeyP9wfBixeIXvWJRneAxIY=",
+ ID: "io.github.crac.org-crac-0.1.3.jar",
+ Target: "dependencies/lib/boot/io.github.crac.org-crac-0.1.3.jar",
+ },
+ {
+ Checksum: "sha1:bOYm5KjEIJKipCZTSMpiGReLm80=",
+ ID: "io.quarkus.quarkus-development-mode-spi-3.8.3.jar",
+ Target: "dependencies/lib/boot/io.quarkus.quarkus-development-mode-spi-3.8.3.jar",
+ },
+ }
+ iks := []v1.IntegrationKitStatus{
+ {
+ // the integration jar file should not be taken into account, so this should be a good match
+ Artifacts: []v1.Artifact{
+ {
+ Checksum: "sha1:Nca4e5f6WohYvjBCigLINWbigAo=",
+ ID: "camel-k-integration-2.5.0-SNAPSHOT.jar",
+ Target: "dependencies/app/camel-k-integration-2.5.0-SNAPSHOT.jar",
+ },
+ {
+ Checksum: "sha1:eMo2eeyP9wfBixeIXvWJRneAxIY=",
+ ID: "io.github.crac.org-crac-0.1.3.jar",
+ Target: "dependencies/lib/boot/io.github.crac.org-crac-0.1.3.jar",
+ },
+ {
+ Checksum: "sha1:bOYm5KjEIJKipCZTSMpiGReLm80=",
+ ID: "io.quarkus.quarkus-development-mode-spi-3.8.3.jar",
+ Target: "dependencies/lib/boot/io.quarkus.quarkus-development-mode-spi-3.8.3.jar",
+ },
+ },
+ },
+ }
+
+ bestImage, commonLibs := findBestImage(iks, requiredArtifacts)
+
+ assert.NotNil(t, bestImage)
+ assert.Equal(t, iks[0], bestImage)
+ assert.NotNil(t, commonLibs)
+ assert.Equal(t, 2, len(commonLibs))
+ assert.True(t, commonLibs["io.github.crac.org-crac-0.1.3.jar"])
+ assert.True(t, commonLibs["io.quarkus.quarkus-development-mode-spi-3.8.3.jar"])
+}
(camel-k) 02/03: bugfix(#5758): changing dependencies build order strategy to be compatible with the incremental image build algorithm
Posted by pc...@apache.org.
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 d66be5ed4c38deea46f09073d80ee25fa360a83c
Author: Luis Sergio Carneiro <lu...@sensedia.com>
AuthorDate: Wed Aug 14 14:47:37 2024 -0300
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 39e9a3816..36ac9b378 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 80bba1f0c..1c01cd5d5 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
}