diff --git a/cmd/topo.go b/cmd/topo.go index 8657664..16bd464 100644 --- a/cmd/topo.go +++ b/cmd/topo.go @@ -35,15 +35,11 @@ func topoFunc(cmd *cobra.Command, args []string) { switch args[0] { case "apply": - if err := topo.Apply(&topoStruct); err != nil { - errPrint(err) - } else { + if err := topo.Apply(&topoStruct); err == nil { fmt.Printf("Topology applied successfully from file: %s\n", args[1]) } case "delete": - if err := topo.Delete(&topoStruct); err != nil { - errPrint(err) - } else { + if err := topo.Delete(&topoStruct); err == nil { fmt.Printf("Topology deleted successfully from file: %s\n", args[1]) } default: diff --git a/integration-test/topo/test.sh b/integration-test/topo/test.sh index 8d4f8fd..7a6ba69 100755 --- a/integration-test/topo/test.sh +++ b/integration-test/topo/test.sh @@ -42,6 +42,13 @@ assert_count() { echo "[+][PASS] $1" } +topo_apply_cycle() { + output=$(../nsctl topo apply topo_template_deps_cycle.yaml) + expect="" + + diff "Topo apply cycle" "$output" "$expect" +} + topo_apply() { output=$(../nsctl topo apply topo_template.yaml) expect=$(cat topo_apply.txt) @@ -66,6 +73,7 @@ topo_delete() { } main() { + topo_apply_cycle topo_apply exec_ping_test topo_delete diff --git a/integration-test/topo/topo_template.yaml b/integration-test/topo/topo_template.yaml index b0060db..eb83ecb 100644 --- a/integration-test/topo/topo_template.yaml +++ b/integration-test/topo/topo_template.yaml @@ -6,6 +6,8 @@ namespaces: - name: br-test-1 bridge: br-test-1-2 ipv4: 10.0.0.1/24 + depends_on: + - test-2 - name: test-2 commands: diff --git a/integration-test/topo/topo_template_deps_cycle.yaml b/integration-test/topo/topo_template_deps_cycle.yaml new file mode 100644 index 0000000..1290849 --- /dev/null +++ b/integration-test/topo/topo_template_deps_cycle.yaml @@ -0,0 +1,24 @@ +namespaces: + - name: test-1 + commands: + - ls + networks: + - name: br-test-1 + bridge: br-test-1-2 + ipv4: 10.0.0.1/24 + depends_on: + - test-2 + + - name: test-2 + commands: + - ls + networks: + - name: br-test-2 + bridge: br-test-1-2 + ipv4: 10.0.0.2/24 + depends_on: + - test-1 + +networks: + - name: br-test-1-2 + subnet: 10.0.0.0/24 \ No newline at end of file diff --git a/internal/topo/apply.go b/internal/topo/apply.go index 8906cd4..f025808 100644 --- a/internal/topo/apply.go +++ b/internal/topo/apply.go @@ -8,7 +8,8 @@ import ( ) func Apply(topo *Topology) error { - if err := checkTopo(topo); err != nil { + g, err := checkTopo(topo) + if err != nil { return err } @@ -20,7 +21,7 @@ func Apply(topo *Topology) error { return err } - if err := addNamespaces(topo.Namespaces); err != nil { + if err := addNamespaces(g.getSortedNamespaces(topo.Namespaces, false)); err != nil { return err } diff --git a/internal/topo/delete.go b/internal/topo/delete.go index 8392bb1..fce7b67 100644 --- a/internal/topo/delete.go +++ b/internal/topo/delete.go @@ -1,6 +1,8 @@ package topo import ( + "errors" + "github.com/Alonza0314/nsctl/internal/namespace" "github.com/Alonza0314/nsctl/internal/veth" "github.com/pterm/pterm" @@ -9,48 +11,60 @@ import ( ) func Delete(topo *Topology) error { - if err := checkTopo(topo); err != nil { + g, err := checkTopo(topo) + if err != nil { return err } - if err := deleteNamespaces(topo.Namespaces); err != nil { - return err + errFlag := false + if err := deleteNamespaces(g.getSortedNamespaces(topo.Namespaces, true)); err != nil { + errFlag = true } if err := deleteBridges(topo.Networks); err != nil { - return err + errFlag = true } + if errFlag { + return errors.New("Error") + } return nil } func deleteNamespaces(nss []Namespace) error { + errFlag := false + for _, ns := range nss { spinner, err := pterm.DefaultSpinner.Start("Deleting namespace " + ns.Name + "...") if err != nil { spinner.Fail("Failed to start spinner for namespace " + ns.Name + ": " + err.Error()) + errFlag = true continue } found, err := namespace.GetNs(ns.Name) if err != nil { spinner.Fail("Failed to get namespace " + ns.Name + ": " + err.Error()) + errFlag = true continue } if !found { spinner.Fail("Namespace " + ns.Name + " does not exist") + errFlag = true continue } _, originCloseFunc, err := namespace.GetOriginNs() if err != nil { spinner.Fail("Failed to get origin namespace file descriptor: " + err.Error()) + errFlag = true continue } nsFd, nsCloseFunc, err := namespace.GetNsFd(ns.Name) if err != nil { spinner.Fail("Failed to get namespace " + ns.Name + " file descriptor: " + err.Error()) originCloseFunc() + errFlag = true continue } @@ -58,22 +72,26 @@ func deleteNamespaces(nss []Namespace) error { spinner.Fail("Failed to set namespace " + ns.Name + ": " + err.Error()) nsCloseFunc() originCloseFunc() + errFlag = true continue } for _, network := range ns.Networks { if err := veth.UpDown(ns.Name, network.Name, false); err != nil { spinner.Fail("Failed to bring down veth for namespace " + ns.Name + " and network " + network.Name + ": " + err.Error()) + errFlag = true continue } link, err := netlink.LinkByName(network.Name) if err != nil { spinner.Fail("Failed to get link " + network.Name + " in namespace " + ns.Name + ": " + err.Error()) + errFlag = true continue } if err := netlink.LinkDel(link); err != nil { spinner.Fail("Failed to delete link " + network.Name + " in namespace " + ns.Name + ": " + err.Error()) + errFlag = true } } @@ -82,31 +100,43 @@ func deleteNamespaces(nss []Namespace) error { if err := namespace.Delete(ns.Name); err != nil { spinner.Fail("Failed to delete namespace " + ns.Name + ": " + err.Error()) + errFlag = true } spinner.Success("Namespace " + ns.Name + " deleted") } + if errFlag { + return errors.New("Error") + } return nil } func deleteBridges(networks []Network) error { + errFlag := false + for _, network := range networks { spinner, err := pterm.DefaultSpinner.Start("Deleting bridge " + network.Name + "...") if err != nil { spinner.Fail("Failed to start spinner for bridge " + network.Name + ": " + err.Error()) + errFlag = true continue } link, err := netlink.LinkByName(network.Name) if err != nil { spinner.Fail("Failed to get bridge " + network.Name + ": " + err.Error()) + errFlag = true continue } if err := netlink.LinkDel(link); err != nil { spinner.Fail("Failed to delete bridge " + network.Name + ": " + err.Error()) + errFlag = true } spinner.Success("Bridge " + network.Name + " deleted") } + if errFlag { + return errors.New("Error") + } return nil } diff --git a/internal/topo/model.go b/internal/topo/model.go index 829b0a8..f4a550e 100644 --- a/internal/topo/model.go +++ b/internal/topo/model.go @@ -7,8 +7,8 @@ type Topology struct { type Namespace struct { Name string `yaml:"name" validate:"required"` - Commands []string `yaml:"commands"` Networks []Net `yaml:"networks"` + DependsOn []string `yaml:"depends_on"` } type Network struct { diff --git a/internal/topo/topo.go b/internal/topo/topo.go index a0c4653..269ff77 100644 --- a/internal/topo/topo.go +++ b/internal/topo/topo.go @@ -8,44 +8,59 @@ import ( "github.com/pterm/pterm" ) -func checkTopo(topo *Topology) error { +func checkTopo(topo *Topology) (*graph, error) { spinner, err := pterm.DefaultSpinner.Start("Checking topology namespaces...") if err != nil { - return err + return nil, err } - if err := checkTopoNamespace(topo); err != nil { - return err + g, err := checkTopoNamespace(topo) + if err != nil { + spinner.Fail("Topology namespaces check failed: " + err.Error()) + return nil, err } spinner.Success("Topology namespaces check passed") spinner, err = pterm.DefaultSpinner.Start("Checking topology networks...") if err != nil { - return err + return nil, err } if err := checkTopoNetwork(topo); err != nil { - return err + spinner.Fail("Topology networks check failed: " + err.Error()) + return nil, err } spinner.Success("Topology networks check passed") - return nil + return g, nil } -func checkTopoNamespace(topo *Topology) error { +func checkTopoNamespace(topo *Topology) (*graph, error) { nsNames := make(map[string]struct{}) for _, ns := range topo.Namespaces { if _, exists := nsNames[ns.Name]; exists { - return fmt.Errorf("duplicate namespace name: %s", ns.Name) + return nil, fmt.Errorf("duplicate namespace name: %s", ns.Name) } nsNames[ns.Name] = struct{}{} if err := checkNamespaceNetwork(&ns); err != nil { - return err + return nil, err } + } + for _, ns := range topo.Namespaces { + for _, dep := range ns.DependsOn { + if _, exists := nsNames[dep]; !exists { + return nil, fmt.Errorf("namespace %s depends on non-existent namespace: %s", ns.Name, dep) + } + } } - return nil + g, err := existCycle(topo.Namespaces) + if err != nil { + return nil, err + } + + return g, nil } func checkNamespaceNetwork(ns *Namespace) error { @@ -145,3 +160,88 @@ func checkNamespaceExist(nss []Namespace) error { return nil } + +type graph struct { + aTob map[string][]string + inDegree map[string]int + index map[string]int + sorted []string +} + +func newGraph() *graph { + return &graph{ + aTob: make(map[string][]string), + inDegree: make(map[string]int), + index: make(map[string]int), + sorted: nil, + } +} + +func (g *graph) make(nss []Namespace) { + for i, ns := range nss { + g.aTob[ns.Name], g.inDegree[ns.Name], g.index[ns.Name] = make([]string, 0), 0, i + } + + for _, ns := range nss { + for _, dep := range ns.DependsOn { + g.aTob[dep] = append(g.aTob[dep], ns.Name) + g.inDegree[ns.Name] += 1 + } + } + +} + +func (g *graph) getNeighbors(ns string) []string { + return g.aTob[ns] +} + +func (g *graph) topologicalSort() { + queue := make([]string, 0) + for ns, degree := range g.inDegree { + if degree == 0 { + queue = append(queue, ns) + } + } + + sorted := make([]string, 0) + for len(queue) > 0 { + ns := queue[0] + queue, sorted = queue[1:], append(sorted, ns) + + for _, neighbor := range g.getNeighbors(ns) { + g.inDegree[neighbor] -= 1 + if g.inDegree[neighbor] == 0 { + queue = append(queue, neighbor) + } + } + } + + g.sorted = sorted +} + +func (g *graph) getSortedNamespaces(nss []Namespace, reverse bool) []Namespace { + nsList := make([]Namespace, len(nss)) + for i, ns := range g.sorted { + nsList[i] = nss[g.index[ns]] + } + + if reverse { + for i, j := 0, len(nsList)-1; i < j; i, j = i+1, j-1 { + nsList[i], nsList[j] = nsList[j], nsList[i] + } + } + + return nsList +} + +func existCycle(nss []Namespace) (*graph, error) { + g := newGraph() + g.make(nss) + + g.topologicalSort() + if len(g.sorted) != len(nss) { + return nil, fmt.Errorf("circular dependency detected among namespaces") + } + + return g, nil +} diff --git a/internal/topo/topo_test.go b/internal/topo/topo_test.go index d0e6250..6a7a6d2 100644 --- a/internal/topo/topo_test.go +++ b/internal/topo/topo_test.go @@ -14,7 +14,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -22,10 +21,10 @@ var testCheckTopoCases = []struct { Ipv4: "10.0.0.1/24", }, }, + DependsOn: []string{"test-2"}, }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -51,7 +50,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -62,7 +60,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -88,7 +85,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -104,7 +100,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -130,7 +125,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -146,7 +140,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -172,7 +165,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -183,7 +175,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -213,7 +204,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -224,7 +214,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -250,7 +239,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -261,7 +249,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -291,7 +278,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -302,7 +288,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -328,7 +313,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -339,7 +323,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -365,7 +348,6 @@ var testCheckTopoCases = []struct { Namespaces: []Namespace{ { Name: "test-1", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-1", @@ -376,7 +358,6 @@ var testCheckTopoCases = []struct { }, { Name: "test-2", - Commands: []string{"echo"}, Networks: []Net{ { Name: "br-test-2", @@ -396,12 +377,48 @@ var testCheckTopoCases = []struct { expectedErr: true, expectedErrDetail: "invalid IP address 10.0.0.999/24 for bridge br-test-1-2: invalid IP address: 10.0.0.999/24", }, + { + name: "depends on non-existent namespace", + topo: &Topology{ + Namespaces: []Namespace{ + { + Name: "test-1", + Networks: []Net{ + { + Name: "br-test-1", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.1/24", + }, + }, + DependsOn: []string{"test-3"}, + }, + { + Name: "test-2", + Networks: []Net{ + { + Name: "br-test-2", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.2/24", + }, + }, + }, + }, + Networks: []Network{ + { + Name: "br-test-1-2", + Subnet: "10.0.0.0/24", + }, + }, + }, + expectedErr: true, + expectedErrDetail: "namespace test-1 depends on non-existent namespace: test-3", + }, } func TestCheckTopo(t *testing.T) { for _, tc := range testCheckTopoCases { t.Run(tc.name, func(t *testing.T) { - err := checkTopo(tc.topo) + _, err := checkTopo(tc.topo) if tc.expectedErr { if err == nil { t.Errorf("expected error but got nil") @@ -416,3 +433,133 @@ func TestCheckTopo(t *testing.T) { }) } } + +var testTopologicalSortCases = []struct { + name string + namespaces []Namespace + reverse bool + epectedErr bool + expectedOrder []string + expectedErrDetail string +}{ + { + name: "valid topo", + namespaces: []Namespace{ + { + Name: "test-1", + Networks: []Net{ + { + Name: "br-test-1", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.1/24", + }, + }, + DependsOn: []string{"test-2"}, + }, + { + Name: "test-2", + Networks: []Net{ + { + Name: "br-test-2", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.2/24", + }, + }, + }, + }, + reverse: false, + epectedErr: false, + expectedOrder: []string{"test-2", "test-1"}, + expectedErrDetail: "", + }, + { + name: "valid topo reverse", + namespaces: []Namespace{ + { + Name: "test-1", + Networks: []Net{ + { + Name: "br-test-1", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.1/24", + }, + }, + DependsOn: []string{"test-2"}, + }, + { + Name: "test-2", + Networks: []Net{ + { + Name: "br-test-2", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.2/24", + }, + }, + }, + }, + reverse: true, + epectedErr: false, + expectedOrder: []string{"test-1", "test-2"}, + expectedErrDetail: "", + }, + { + name: "circular dependency", + namespaces: []Namespace{ + { + Name: "test-1", + Networks: []Net{ + { + Name: "br-test-1", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.1/24", + }, + }, + DependsOn: []string{"test-2"}, + }, + { + Name: "test-2", + Networks: []Net{ + { + Name: "br-test-2", + Bridge: "br-test-1-2", + Ipv4: "10.0.0.2/24", + }, + }, + DependsOn: []string{"test-1"}, + }, + }, + reverse: false, + epectedErr: true, + expectedOrder: nil, + expectedErrDetail: "circular dependency detected among namespaces", + }, +} + +func TestTopologicalSort(t *testing.T) { + for _, tc := range testTopologicalSortCases { + t.Run(tc.name, func(t *testing.T) { + g, err := existCycle(tc.namespaces) + if err != nil { + if tc.epectedErr { + if err.Error() != tc.expectedErrDetail { + t.Errorf("expected error detail '%s' but got '%s'", tc.expectedErrDetail, err.Error()) + } + } else { + t.Errorf("expected no error but got: %v", err) + } + return + } + sorted := g.getSortedNamespaces(tc.namespaces, tc.reverse) + sortedNames := make([]string, len(sorted)) + for i, ns := range sorted { + sortedNames[i] = ns.Name + } + for i := range sortedNames { + if sortedNames[i] != tc.expectedOrder[i] { + t.Errorf("expected order %v but got %v", tc.expectedOrder, sortedNames) + break + } + } + }) + } +}