Skip to content

Commit

Permalink
Merge pull request #671 from joulaud/fjo/fix-rebuild-errors
Browse files Browse the repository at this point in the history
fix: RebuildGraph is more reliable for child images
  • Loading branch information
antoinegelloz authored Jan 14, 2025
2 parents ad452c5 + 3e56470 commit a47e45d
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 82 deletions.
52 changes: 0 additions & 52 deletions pkg/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,58 +107,6 @@ func (d *DAG) WalkParallel(visitor NodeVisitorFunc) {
waitGroup.Wait()
}

// Filter creates a new DAG only populated by nodes that fulfil the condition checked by filterFunc.
// All children nodes that no longer have any parents in the resulting graph will become a root node of the DAG.
func (d *DAG) Filter(filterFunc func(*Node) bool) *DAG {
filteredGraph := DAG{}
replacements := make(map[*Node]*Node)
var orphans []*Node

d.Walk(func(node *Node) {
if !filterFunc(node) {
// The node does not fulfil the condition, skip it.
return
}

// Create a replacement node containing the same Image.
newNode := NewNode(node.Image)
replacements[node] = newNode
// Check if the node is orphan
for _, parent := range node.Parents() {
if _, ok := replacements[parent]; ok {
// The node has at least one parent, nothing to do.
return
}
}
// The node no longer has parents, we add it to the list of orphans.
orphans = append(orphans, node)
})

for _, orphan := range orphans {
orphan.walkInDepth(createUniqueVisitor(func(node *Node) {
replacement, ok := replacements[node]
if !ok {
// This node on the source graph should not be in the filtered graph.
return
}

// Add new child nodes
for _, child := range node.Children() {
childReplacement, ok := replacements[child]
if !ok {
// This child node on the source graph should not be in the filtered graph.
continue
}
replacement.AddChild(childReplacement)
}
}))
// Add the orphan node to the root nodes of the graph.
filteredGraph.AddNode(replacements[orphan])
}

return &filteredGraph
}

//nolint:musttag
func (d *DAG) ListImage() string {
imagesList := make(map[string]Image)
Expand Down
26 changes: 0 additions & 26 deletions pkg/dag/dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,32 +214,6 @@ func Test_WalkParallel_RunsAllNodes(t *testing.T) {
assert.Equal(t, 6, length) // Total number of nodes is 6
}

func Test_Filter(t *testing.T) {
t.Parallel()

DAG := createDAG()

withoutParents := DAG.Filter(func(node *dag.Node) bool {
return len(node.Parents()) > 0 // Filter out nodes having parents.
})
assert.Len(t, withoutParents.Nodes(), 3)

withParents := DAG.Filter(func(node *dag.Node) bool {
return len(node.Parents()) == 0 // Filter out nodes having no parent.
})
assert.Len(t, withParents.Nodes(), 2)

withoutChildren := DAG.Filter(func(node *dag.Node) bool {
return len(node.Children()) > 0 // Filter out nodes having children.
})
assert.Len(t, withoutChildren.Nodes(), 2)

withChildren := DAG.Filter(func(node *dag.Node) bool {
return len(node.Children()) == 0 // Filter out nodes having no children.
})
assert.Len(t, withChildren.Nodes(), 3)
}

func Test_ListImage(t *testing.T) {
t.Parallel()

Expand Down
8 changes: 4 additions & 4 deletions pkg/dib/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,13 @@ func (p *Builder) rebuildGraph(
buildArgs map[string]string,
) {
p.Graph.
Filter(
func(node *dag.Node) bool {
return node.Image.NeedsRebuild || node.Image.NeedsTests
}).
WalkParallel(
func(node *dag.Node) {
img := node.Image
if !(img.NeedsRebuild || img.NeedsTests) {
img.RebuildFailed = false
return
}
buildReport := report.BuildReport{Image: *img}

// Return if any parent build failed
Expand Down

0 comments on commit a47e45d

Please sign in to comment.