Skip to content

Commit

Permalink
bugfix(#5755): avoiding deadlock between builds with dependencies bui…
Browse files Browse the repository at this point in the history 
…ld order strategy when the builds share enough dependencies
  • Loading branch information
@lsergio
lsergio committed Aug 9, 2024
1 parent cd3fbf2 commit 2c68f13
Show file tree
Hide file tree
Showing 2 changed files with 264 additions and 6 deletions.
220 changes: 220 additions & 0 deletions pkg/apis/camel/v1/build_type_support_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -37,6 +38,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:timer",
"camel:log",
},
Runtime: RuntimeSpec{
Version: "3.8.1",
},
},
},
},
Expand All @@ -58,6 +62,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:log",
"camel:bean",
},
Runtime: RuntimeSpec{
Version: "3.8.1",
},
},
},
},
Expand All @@ -80,6 +87,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:bean",
"camel:zipfile",
},
Runtime: RuntimeSpec{
Version: "3.8.1",
},
},
},
},
Expand All @@ -101,6 +111,9 @@ func TestMatchingBuildsPending(t *testing.T) {
"camel:component-a",
"camel:component-b",
},
Runtime: RuntimeSpec{
Version: "3.8.1",
},
},
},
},
Expand All @@ -126,3 +139,210 @@ 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)
}
50 changes: 44 additions & 6 deletions pkg/apis/camel/v1/build_types_support.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -269,14 +279,24 @@ func (bl BuildList) HasScheduledBuildsBefore(build *Build) (bool, *Build) {
// It returns the first matching build found regardless it may have any one more appropriate.
func (bl BuildList) HasMatchingBuild(build *Build) (bool, *Build) {
required := build.BuilderDependencies()
requiredMap := make(map[string]int, len(required))
for i, item := range required {
requiredMap[item] = i
}
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))
Expand All @@ -286,16 +306,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 == 0 {
continue
}

Expand All @@ -311,10 +332,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
}
}
}
Expand Down

0 comments on commit 2c68f13

Please sign in to comment.