diff --git a/Makefile b/Makefile index df9841f739..2d00a7b887 100644 --- a/Makefile +++ b/Makefile @@ -64,6 +64,10 @@ format: check: @$(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/check.sh ./cmd/... ./pkg/... +.PHONY: test +test: + @go test -mod=vendor $(REPO_ROOT)/cmd/... $(REPO_ROOT)/pkg/... + .PHONY: install install: @./hack/install diff --git a/pkg/testmachinery/testdefinition/types.go b/pkg/testmachinery/testdefinition/types.go index 5fa9113fc8..6eec2a0a09 100644 --- a/pkg/testmachinery/testdefinition/types.go +++ b/pkg/testmachinery/testdefinition/types.go @@ -13,7 +13,6 @@ import ( // TestDefinition represents a TestDefinition which was fetched from locations. type TestDefinition struct { Info *tmv1beta1.TestDefinition - TaskName string Location Location FileName string Template *argov1.Template diff --git a/pkg/testmachinery/testflow/flow_operations.go b/pkg/testmachinery/testflow/flow_operations.go index 0cd14870a1..f5e6528616 100644 --- a/pkg/testmachinery/testflow/flow_operations.go +++ b/pkg/testmachinery/testflow/flow_operations.go @@ -74,10 +74,9 @@ func ReorderChildrenOfNodes(list *node.Set) *node.Set { // reorderSerialNodes reorders all children of a node so that serial steps run in serial after parallel nodes. // The functions returns the new Children func reorderChildrenOfNode(root *node.Node) *node.Set { - grandChildren := root.Children.GetChildren() - // directly return if there is only one node in the pool - if root.Children.Len() == 1 { + // directly return if there are no nodes or only one node in the pool + if root.Children.Len() <= 1 { // todo: write test for special case return root.Children } @@ -101,33 +100,48 @@ func reorderChildrenOfNode(root *node.Node) *node.Set { root.ClearChildren() root.AddChildren(parallelNodes.List()...) + previousChildren := parallelNodes.GetChildren() + // remove children from parallel node and the root note + parallelNodes.Remove(previousChildren.List()...) + root.RemoveChildren(previousChildren.List()...) for i, serialNode := range serialNodes.List() { + // remove the current node from the list of previous children + // and add all children of the current node + previousChildren.Remove(serialNode) + previousChildren.Add(serialNode.Children.List()...) + // remove the serial as possible parent from all previous children + previousChildren.RemoveParents(serialNode) + serialNode.ClearChildren() if i == 0 { parallelNodes.ClearChildren() parallelNodes.AddChildren(serialNode) - serialNode.ClearParents() + // only remove the root parent as otherwise other parent information will get lost + serialNode.RemoveParent(root) serialNode.AddParents(parallelNodes.List()...) } else { prevNode := serialNodes.List()[i-1] - prevNode.ClearChildren() + //prevNode.ClearChildren() prevNode.AddChildren(serialNode) - serialNode.ClearParents() + // only remove the root parent as otherwise other parent information will get lost + serialNode.RemoveParent(root) serialNode.AddParents(prevNode) } + // last node if i == serialNodes.Len()-1 { serialNode.ClearChildren() - serialNode.AddChildren(grandChildren.List()...) + serialNode.AddChildren(previousChildren.List()...) - grandChildren.ClearParents() - grandChildren.AddParents(serialNode) + previousChildren.AddParents(serialNode) } } - return grandChildren + // return last node + // the list cannot be empty as this case is already checked above. + return node.NewSet(serialNodes.List()[serialNodes.Len()-1]) } // ApplyOutputScope defines the artifact scopes for outputs. @@ -282,6 +296,9 @@ func getJointNodes(nodes []*node.Node, branches []*node.Set, getNext func(*node. // Note the order of the node sets are essential. func findJointNode(nodeSets []*node.Set) *node.Node { if len(nodeSets) == 1 { + if nodeSets[0].Len() == 0 { + return nil + } nodeList := nodeSets[0].List() return nodeList[len(nodeList)-1] } diff --git a/pkg/testmachinery/testflow/flow_operations_test.go b/pkg/testmachinery/testflow/flow_operations_test.go index bad521090e..f1fb3dd962 100644 --- a/pkg/testmachinery/testflow/flow_operations_test.go +++ b/pkg/testmachinery/testflow/flow_operations_test.go @@ -703,6 +703,7 @@ var serialTestDef = func() testdefinition.TestDefinition { Template: &argov1.Template{}, } }() + var defaultTestDef = testdefinition.NewEmpty() func createStepsFromNodes(nodes ...*node.Node) map[string]*testflow.Step { diff --git a/pkg/testmachinery/testflow/flow_test.go b/pkg/testmachinery/testflow/flow_test.go new file mode 100644 index 0000000000..c61ead2a28 --- /dev/null +++ b/pkg/testmachinery/testflow/flow_test.go @@ -0,0 +1,67 @@ +package testflow_test + +import ( + tmv1beta1 "github.com/gardener/test-infra/pkg/apis/testmachinery/v1beta1" + "github.com/gardener/test-infra/pkg/testmachinery/testdefinition" + "github.com/gardener/test-infra/pkg/testmachinery/testflow" + testutils "github.com/gardener/test-infra/test/utils" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("flow", func() { + Context("init", func() { + It("should set the root node as parent of all steps with no dependencies", func() { + rootNode := testNode("root", nil, defaultTestDef, nil) + locs := &testutils.LocationsMock{ + StepToTestDefinitions: map[string][]*testdefinition.TestDefinition{ + "create": {testutils.TestDef("create")}, + "delete": {testutils.TestDef("delete")}, + "tests-beta": {testutils.TestDef("default"), testutils.SerialTestDef("serial"), testutils.DisruptiveTestDef("disruptive")}, + "tests-release": {testutils.TestDef("default"), testutils.SerialTestDef("serial"), testutils.DisruptiveTestDef("disruptive")}, + }, + } + tf := tmv1beta1.TestFlow{} + Expect(testutils.ReadYAMLFile("./testdata/testflow-00.yaml", &tf)).To(Succeed()) + + _, err := testflow.NewFlow("testid", rootNode, tf, locs, nil) + Expect(err).ToNot(HaveOccurred()) + + Expect(rootNode.Children.Len()).To(Equal(1), "the root node should have one create child") + createNode := rootNode.Children.List()[0] + Expect(createNode.TestDefinition.Info.Name).To(Equal("create")) + + // beta step + Expect(createNode.Children.Len()).To(Equal(1), "the create node should have 1 test child") + defaultBetaNode := createNode.Children.List()[0] + Expect(defaultBetaNode.TestDefinition.Info.Name).To(Equal("default")) + + Expect(defaultBetaNode.Children.Len()).To(Equal(1), "the disruptive beta node should have 1 test child") + serialBetaNode := defaultBetaNode.Children.List()[0] + Expect(serialBetaNode.TestDefinition.Info.Name).To(Equal("serial")) + + Expect(serialBetaNode.Children.Len()).To(Equal(1), "the default beta node should have 1 test child") + disruptiveBetaNode := serialBetaNode.Children.List()[0] + Expect(disruptiveBetaNode.TestDefinition.Info.Name).To(Equal("disruptive")) + + // release step + Expect(disruptiveBetaNode.Children.Len()).To(Equal(1), "the disruptive beta node should have 1 test child") + defaultReleaseNode := disruptiveBetaNode.Children.List()[0] + Expect(defaultReleaseNode.TestDefinition.Info.Name).To(Equal("default")) + + Expect(defaultReleaseNode.Children.Len()).To(Equal(1), "the default release node should have 1 test child") + serialReleaseNode := defaultReleaseNode.Children.List()[0] + Expect(serialReleaseNode.TestDefinition.Info.Name).To(Equal("serial")) + + Expect(serialReleaseNode.Children.Len()).To(Equal(1), "the serial release node should have 1 test child") + disruptiveReleaseNode := serialReleaseNode.Children.List()[0] + Expect(disruptiveReleaseNode.TestDefinition.Info.Name).To(Equal("disruptive")) + + // delete + Expect(disruptiveReleaseNode.Children.Len()).To(Equal(1), "the disruptive release node should have 1 test child") + deleteNode := disruptiveReleaseNode.Children.List()[0] + Expect(deleteNode.TestDefinition.Info.Name).To(Equal("delete")) + }) + }) +}) diff --git a/pkg/testmachinery/testflow/node/node.go b/pkg/testmachinery/testflow/node/node.go index 59c3d6e7b7..7c07d60306 100644 --- a/pkg/testmachinery/testflow/node/node.go +++ b/pkg/testmachinery/testflow/node/node.go @@ -79,13 +79,24 @@ func (n *Node) AddChildren(children ...*Node) { n.Children.Add(children...) } -// ClearParent removes a node from the current node's children +// RemoveChild removes a node from the current node's children func (n *Node) RemoveChild(child *Node) { + child.RemoveParent(n) n.Children.Remove(child) } +// RemoveChildren removes all nodes from the current node's children +func (n *Node) RemoveChildren(children ...*Node) { + for _, child := range children { + n.RemoveChild(child) + } +} + // ClearChildren removes all children from the current node func (n *Node) ClearChildren() { + for child := range n.Children.Iterate() { + child.RemoveParent(n) + } n.Children = NewSet() } diff --git a/pkg/testmachinery/testflow/node/node_suite_test.go b/pkg/testmachinery/testflow/node/node_suite_test.go index 1029e17029..fabe812df2 100644 --- a/pkg/testmachinery/testflow/node/node_suite_test.go +++ b/pkg/testmachinery/testflow/node/node_suite_test.go @@ -3,8 +3,6 @@ package node import ( "testing" - argov1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1" - tmv1beta1 "github.com/gardener/test-infra/pkg/apis/testmachinery/v1beta1" "github.com/gardener/test-infra/pkg/testmachinery/testdefinition" testutils "github.com/gardener/test-infra/test/utils" @@ -25,9 +23,9 @@ var _ = Describe("node operations", func() { step.Definition.ContinueOnError = true locs := &testutils.LocationsMock{ TestDefinitions: []*testdefinition.TestDefinition{ - defaultTestDef, - serialTestDef(), - disruptiveTestDef(), + testutils.TestDef("default"), + testutils.SerialTestDef("serial"), + testutils.DisruptiveTestDef("disruptive"), }, } nodes, err := CreateNodesFromStep(step, locs, nil, "") @@ -46,27 +44,3 @@ var _ = Describe("node operations", func() { }) }) - -func serialTestDef() *testdefinition.TestDefinition { - return &testdefinition.TestDefinition{ - Info: &tmv1beta1.TestDefinition{ - Spec: tmv1beta1.TestDefSpec{ - Behavior: []string{tmv1beta1.SerialBehavior}, - }, - }, - Template: &argov1.Template{}, - } -} - -func disruptiveTestDef() *testdefinition.TestDefinition { - return &testdefinition.TestDefinition{ - Info: &tmv1beta1.TestDefinition{ - Spec: tmv1beta1.TestDefSpec{ - Behavior: []string{tmv1beta1.DisruptiveBehavior}, - }, - }, - Template: &argov1.Template{}, - } -} - -var defaultTestDef = testdefinition.NewEmpty() diff --git a/pkg/testmachinery/testflow/node/nodelist.go b/pkg/testmachinery/testflow/node/nodelist.go index c3218f277e..4903939275 100644 --- a/pkg/testmachinery/testflow/node/nodelist.go +++ b/pkg/testmachinery/testflow/node/nodelist.go @@ -1,10 +1,11 @@ package node +import "k8s.io/apimachinery/pkg/util/sets" + // NewSet creates a new set of nodes func NewSet(nodes ...*Node) *Set { set := Set{ - set: make(map[*Node]int), - list: make([]*Node, 0), + set: make(map[*Node]sets.Empty), } set.Add(nodes...) return &set @@ -12,26 +13,48 @@ func NewSet(nodes ...*Node) *Set { // New creates a deep copy of the set func (s *Set) Copy() *Set { - newSet := make(map[*Node]int, len(s.set)) + newIntSet := make(map[*Node]sets.Empty, len(s.set)) for key, val := range s.set { - newSet[key] = val + newIntSet[key] = val } - newList := make([]*Node, len(s.list)) - for i, v := range s.list { - newList[i] = v + set := &Set{ + set: newIntSet, } + set.Add(s.List()...) + return set +} - return &Set{ - set: newSet, - list: newList, +// is a internal iteration through the list items +func (s *Set) iterateList() chan *listItem { + c := make(chan *listItem) + go func() { + n := s.listStart + for n != nil { + c <- n + n = n.next + } + close(c) + }() + return c +} + +// returns the list item for a node +func (s *Set) getListItem(node *Node) *listItem { + for item := range s.iterateList() { + if item.node == node { + return item + } } + return nil } func (s *Set) Iterate() chan *Node { c := make(chan *Node) go func() { - for _, n := range s.list { - c <- n + n := s.listStart + for n != nil { + c <- n.node + n = n.next } close(c) }() @@ -41,8 +64,10 @@ func (s *Set) Iterate() chan *Node { func (s *Set) IterateInverse() chan *Node { c := make(chan *Node) go func() { - for i := len(s.list) - 1; i >= 0; i-- { - c <- s.list[i] + n := s.listEnd + for n != nil { + c <- n.node + n = n.previous } close(c) }() @@ -61,17 +86,48 @@ func (s *Set) Has(n *Node) bool { // Add adds nodes to the set func (s *Set) Add(nodes ...*Node) { for _, n := range nodes { - s.set[n] = len(s.list) - s.list = append(s.list, n) + if _, ok := s.set[n]; ok { + continue + } + s.set[n] = sets.Empty{} + newItem := &listItem{ + node: n, + previous: s.listEnd, + } + if s.listEnd != nil { + newItem.previous.next = newItem + } + if s.listStart == nil { + s.listStart = newItem + } + s.listEnd = newItem } } // Removes nodes form the set func (s *Set) Remove(nodes ...*Node) { for _, n := range nodes { - i := s.set[n] + if _, ok := s.set[n]; !ok { + continue + } delete(s.set, n) - s.list = append(s.list[:i], s.list[i+1:]...) + item := s.getListItem(n) + if item == nil { + // this should never happen as we will have a inconsistency in the list + // but for now we expect that the item is then already deleted + continue + } + if item.previous != nil { + item.previous.next = item.next + } else { + s.listStart = item.next + } + if item.next != nil { + item.next.previous = item.previous + } else { + s.listEnd = item.previous + } + // let go garbage collect the item } } @@ -85,7 +141,7 @@ func (s *Set) AddParents(parents ...*Node) { // RemoveParents removes nodes from parents. func (s *Set) RemoveParents(parents ...*Node) { for n := range s.Iterate() { - for parent := range n.Parents.set { + for _, parent := range parents { n.RemoveParent(parent) } } @@ -108,7 +164,7 @@ func (s *Set) AddChildren(children ...*Node) { // RemoveChildren removes nodes from children func (s *Set) RemoveChildren(children ...*Node) { for n := range s.Iterate() { - for child := range n.Children.Iterate() { + for _, child := range children { n.RemoveChild(child) } } @@ -149,10 +205,18 @@ func (s *Set) GetParents() *Set { // List return all nodes of the set as an array func (s Set) List() []*Node { - return s.list + var ( + list = make([]*Node, len(s.set)) + i = 0 + ) + for node := range s.Iterate() { + list[i] = node + i++ + } + return list } // Set returns map of the set -func (s Set) Set() map[*Node]int { +func (s Set) Set() map[*Node]sets.Empty { return s.set } diff --git a/pkg/testmachinery/testflow/node/types.go b/pkg/testmachinery/testflow/node/types.go index 627cd75e39..30eff3e0dd 100644 --- a/pkg/testmachinery/testflow/node/types.go +++ b/pkg/testmachinery/testflow/node/types.go @@ -2,13 +2,25 @@ package node import ( argov1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1" + "k8s.io/apimachinery/pkg/util/sets" + tmv1beta1 "github.com/gardener/test-infra/pkg/apis/testmachinery/v1beta1" "github.com/gardener/test-infra/pkg/testmachinery/testdefinition" ) type Set struct { - list []*Node - set map[*Node]int + set map[*Node]sets.Empty + // first item of the linked list + listStart *listItem + // last item of the linked list + listEnd *listItem +} + +// listItem is a internal structure for a linked list of nodes +type listItem struct { + node *Node + previous *listItem + next *listItem } // Node is an object that represents a node of the internal DAG representation diff --git a/pkg/testmachinery/testflow/testdata/testflow-00.yaml b/pkg/testmachinery/testflow/testdata/testflow-00.yaml new file mode 100644 index 0000000000..e03068c005 --- /dev/null +++ b/pkg/testmachinery/testflow/testdata/testflow-00.yaml @@ -0,0 +1,20 @@ +- name: create + definition: + name: create + +- name: tests-beta + dependsOn: [ create ] + definition: + name: tests-beta + continueOnError: true + +- name: tests-release + dependsOn: [ tests-beta ] + definition: + name: tests-release + continueOnError: true + +- name: delete + dependsOn: [ tests-release, tests-beta ] + definition: + name: delete \ No newline at end of file diff --git a/test/utils/mocks.go b/test/utils/mocks.go index d874ef081e..e768ac482f 100644 --- a/test/utils/mocks.go +++ b/test/utils/mocks.go @@ -15,13 +15,17 @@ package utils import ( + "fmt" + tmv1beta1 "github.com/gardener/test-infra/pkg/apis/testmachinery/v1beta1" + "github.com/gardener/test-infra/pkg/testmachinery/config" "github.com/gardener/test-infra/pkg/testmachinery/locations" "github.com/gardener/test-infra/pkg/testmachinery/testdefinition" ) type LocationsMock struct { TestDefinitions []*testdefinition.TestDefinition + StepToTestDefinitions map[string][]*testdefinition.TestDefinition GetTestDefinitionsFunc func(step tmv1beta1.StepDefinition) ([]*testdefinition.TestDefinition, error) } @@ -31,6 +35,13 @@ func (t *LocationsMock) GetTestDefinitions(step tmv1beta1.StepDefinition) ([]*te if t.GetTestDefinitionsFunc != nil { return t.GetTestDefinitionsFunc(step) } + if t.StepToTestDefinitions != nil { + td, ok := t.StepToTestDefinitions[step.Name] + if !ok { + return nil, fmt.Errorf("no testdefinitions found for step %q", step.Name) + } + return td, nil + } if t.TestDefinitions != nil { return t.TestDefinitions, nil } @@ -66,3 +77,34 @@ func (l *TDLocationMock) SetTestDefs(_ map[string]*testdefinition.TestDefinition func (l *TDLocationMock) GetLocation() *tmv1beta1.TestLocation { return nil } + +/** +TestDefinition helper +*/ + +// TestDef creates a new empty testdefinition with a step name +func TestDef(name string) *testdefinition.TestDefinition { + td := testdefinition.NewEmpty() + td.Info.Name = name + return td +} + +// SerialTestDef creates a new serial testdefinition with a name +func SerialTestDef(name string) *testdefinition.TestDefinition { + td := TestDef(name) + td.Info.Spec.Behavior = []string{tmv1beta1.SerialBehavior} + return td +} + +// DisruptiveTestDef creates a new disruptive testdefinition with a name +func DisruptiveTestDef(name string) *testdefinition.TestDefinition { + td := TestDef(name) + td.Info.Spec.Behavior = []string{tmv1beta1.DisruptiveBehavior} + return td +} + +// TestDefWithConfig adds a config to the given testdefinition and returns it +func TestDefWithConfig(td *testdefinition.TestDefinition, cfgs []tmv1beta1.ConfigElement) *testdefinition.TestDefinition { + td.AddConfig(config.New(cfgs, config.LevelTestDefinition)) + return td +} diff --git a/test/utils/utils.go b/test/utils/utils.go index ef48018a72..41cb590c5c 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -16,6 +16,7 @@ package utils import ( "context" + "encoding/json" "fmt" "github.com/gardener/gardener-resource-manager/pkg/apis/resources/v1alpha1" mrhealth "github.com/gardener/gardener-resource-manager/pkg/health" @@ -25,6 +26,9 @@ import ( "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/util/wait" + "sigs.k8s.io/yaml" + + "io/ioutil" "net/http" "reflect" "strings" @@ -262,3 +266,21 @@ func HTTPGet(url string) (*http.Response, error) { return response, nil } + +// ReadJSONFile reads a file and deserializes the json into the given object +func ReadJSONFile(path string, obj interface{}) error { + data, err := ioutil.ReadFile(path) + if err != nil { + return err + } + return json.Unmarshal(data, obj) +} + +// ReadYAMLFile reads a file and deserializes the yaml into the given object +func ReadYAMLFile(path string, obj interface{}) error { + data, err := ioutil.ReadFile(path) + if err != nil { + return err + } + return yaml.Unmarshal(data, obj) +}