Skip to content

Commit

Permalink
Fix non-determinism with visualizer yamls
Browse files Browse the repository at this point in the history
  • Loading branch information
gordon-klotho committed Dec 5, 2023
1 parent 7a22b97 commit e2bf53c
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 63 deletions.
15 changes: 13 additions & 2 deletions pkg/construct2/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"math"
"sort"
"strings"

"github.com/dominikbraun/graph"
Expand Down Expand Up @@ -119,8 +120,17 @@ func bellmanFord(g Graph, source ResourceId, skipEdge func(Edge) bool) (*bellman
}
dist[source] = 0

// Sort the keys to ensure deterministic results. It adds +O(N) to the runtime, but
// when it's already O(N * E), it doesn't matter.
sortedKeys := make([]ResourceId, 0, len(adjacencyMap))
for key := range adjacencyMap {
sortedKeys = append(sortedKeys, key)
}
sort.Sort(sortedIds(sortedKeys))

for i := 0; i < len(adjacencyMap)-1; i++ {
for key, edges := range adjacencyMap {
for _, key := range sortedKeys {
edges := adjacencyMap[key]
for _, edge := range edges {
if skipEdge(edge) {
continue
Expand All @@ -134,7 +144,8 @@ func bellmanFord(g Graph, source ResourceId, skipEdge func(Edge) bool) (*bellman
}
}

for _, edges := range adjacencyMap {
for _, key := range sortedKeys {
edges := adjacencyMap[key]
for _, edge := range edges {
if skipEdge(edge) {
continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine2/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func assertYamlMatches(t *testing.T, expectStr, actualStr string, name string) {
case diff.DELETE:
t.Errorf("[%s] %s %s: %v", name, c.Type, path, c.From)
case diff.UPDATE:
t.Errorf("[%s] %s %s: %v -> %v", name, c.Type, path, c.From, c.To)
t.Errorf("[%s] %s %s: %v to %v", name, c.Type, path, c.From, c.To)
}
}
}
2 changes: 1 addition & 1 deletion pkg/engine2/testdata/ecs_rds.dataflow-viz.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ resources:
parent: vpc/vpc-0

ecs_service/ecs_service_0 -> rds_instance/rds-instance-2:
path: aws:subnet:vpc-0:subnet-1,aws:security_group:vpc-0:security_group-rds-instance-2-1
path: aws:ecs_task_definition:ecs_service_0,aws:iam_role:ecs_service_0-execution-role


10 changes: 5 additions & 5 deletions pkg/engine2/testdata/ecs_rds.expect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ resources:
Protocol: "-1"
ToPort: 0
IngressRules:
- Description: Allow ingress traffic from within the same security group
FromPort: 0
Protocol: "-1"
Self: true
ToPort: 0
- CidrBlocks:
- 10.0.128.0/18
Description: Allow ingress traffic from ip addresses within the subnet subnet-0
Expand All @@ -187,11 +192,6 @@ resources:
FromPort: 0
Protocol: "-1"
ToPort: 0
- Description: Allow ingress traffic from within the same security group
FromPort: 0
Protocol: "-1"
Self: true
ToPort: 0
Vpc: aws:vpc:vpc-0
aws:route_table:route_table-subnet-0-0:
Routes:
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine2/testdata/k8s_api.expect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ resources:
aws:load_balancer:rest-api-4-integbcc77100:
Scheme: internal
Subnets:
- aws:subnet:vpc-0:subnet-0
- aws:subnet:vpc-0:subnet-1
- aws:subnet:vpc-0:subnet-0
Type: network
aws:load_balancer_listener:rest_api_4_integration_0-pod2:
DefaultActions:
Expand Down
42 changes: 27 additions & 15 deletions pkg/engine2/visualizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,18 @@ func (e *Engine) GetViewsDag(view View, ctx solution_context.SolutionContext) (c
}
zap.S().Debugf("%s paths: %d", src, len(deps.Paths))
sort.Slice(deps.Paths, func(i, j int) bool {
return len(deps.Paths[i]) < len(deps.Paths[j])
li, lj := len(deps.Paths[i]), len(deps.Paths[j])
if li != lj {
return li < lj
}
for pathIdx := 0; pathIdx < li; pathIdx++ {
resI := deps.Paths[i][pathIdx]
resJ := deps.Paths[j][pathIdx]
if resI != resJ {
return construct.ResourceIdLess(resI, resJ)
}
}
return false
})
seenSmall := make(set.Set[construct.ResourceId])
for _, path := range deps.Paths {
Expand All @@ -127,24 +138,25 @@ func (e *Engine) GetViewsDag(view View, ctx solution_context.SolutionContext) (c
dstTag := e.GetResourceVizTag(string(DataflowView), dst)
switch dstTag {
case ParentIconTag:
if node.Parent.IsZero() {
template, err := e.Kb.GetResourceTemplate(dst)
if !node.Parent.IsZero() {
continue
}
template, err := e.Kb.GetResourceTemplate(dst)
if err != nil {
errs = errors.Join(errs, err)
continue
}
if collectionutil.Contains(template.Classification.Is, "cluster") ||
collectionutil.Contains(template.Classification.Is, "network") {
hasPath, err := HasParent(topo, ctx, src, dst)
if err != nil {
errs = errors.Join(errs, err)
continue
}
if collectionutil.Contains(template.Classification.Is, "cluster") ||
collectionutil.Contains(template.Classification.Is, "network") {
hasPath, err := HasParent(topo, ctx, src, dst)
if err != nil {
errs = errors.Join(errs, err)
}
if node.Parent.IsZero() && hasPath {
node.Parent = dst
}
} else {
errs = errors.Join(errs, createEdgeIfPath(topo, ctx, src, dst, path))
if node.Parent.IsZero() && hasPath {
node.Parent = dst
}
} else {
errs = errors.Join(errs, createEdgeIfPath(topo, ctx, src, dst, path))
}
case BigIconTag:
errs = errors.Join(errs, createEdgeIfPath(topo, ctx, src, dst, path))
Expand Down
41 changes: 3 additions & 38 deletions pkg/graph_addons/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@ import (
)

type LoggingGraph[K comparable, T any] struct {
Log *zap.SugaredLogger
Graph graph.Graph[K, T]
Hash func(T) K
}
graph.Graph[K, T]

func (g LoggingGraph[K, T]) Traits() *graph.Traits {
return g.Graph.Traits()
Log *zap.SugaredLogger
Hash func(T) K
}

func (g LoggingGraph[K, T]) AddVertex(value T, options ...func(*graph.VertexProperties)) error {
Expand All @@ -30,14 +27,6 @@ func (g LoggingGraph[K, T]) AddVerticesFrom(other graph.Graph[K, T]) error {
return g.Graph.AddVerticesFrom(other)
}

func (g LoggingGraph[K, T]) Vertex(hash K) (T, error) {
return g.Graph.Vertex(hash)
}

func (g LoggingGraph[K, T]) VertexWithProperties(hash K) (T, graph.VertexProperties, error) {
return g.Graph.VertexWithProperties(hash)
}

func (g LoggingGraph[K, T]) RemoveVertex(hash K) error {
err := g.Graph.RemoveVertex(hash)
if err != nil {
Expand All @@ -63,14 +52,6 @@ func (g LoggingGraph[K, T]) AddEdgesFrom(other graph.Graph[K, T]) error {
return g.Graph.AddEdgesFrom(other)
}

func (g LoggingGraph[K, T]) Edge(sourceHash K, targetHash K) (graph.Edge[T], error) {
return g.Graph.Edge(sourceHash, targetHash)
}

func (g LoggingGraph[K, T]) Edges() ([]graph.Edge[K], error) {
return g.Graph.Edges()
}

func (g LoggingGraph[K, T]) UpdateEdge(source K, target K, options ...func(properties *graph.EdgeProperties)) error {
err := g.Graph.UpdateEdge(source, target, options...)
if err != nil {
Expand All @@ -91,26 +72,10 @@ func (g LoggingGraph[K, T]) RemoveEdge(source K, target K) error {
return err
}

func (g LoggingGraph[K, T]) AdjacencyMap() (map[K]map[K]graph.Edge[K], error) {
return g.Graph.AdjacencyMap()
}

func (g LoggingGraph[K, T]) PredecessorMap() (map[K]map[K]graph.Edge[K], error) {
return g.Graph.PredecessorMap()
}

func (g LoggingGraph[K, T]) Clone() (graph.Graph[K, T], error) {
cloned, err := g.Graph.Clone()
if err != nil {
return nil, err
}
return LoggingGraph[K, T]{Log: g.Log, Graph: cloned}, nil
}

func (g LoggingGraph[K, T]) Order() (int, error) {
return g.Graph.Order()
}

func (g LoggingGraph[K, T]) Size() (int, error) {
return g.Graph.Size()
}

0 comments on commit e2bf53c

Please sign in to comment.