Merge pull request #135 from telekom-mms/feature/move-splitrt-cleanup…

…-to-vpnsetup Move cleanup commands of Split Routing to VPNSetup
telekom-mms · Jan 22, 2025 · c108664 · c108664
2 parents 0949054 + f7d2438
commit c108664
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 67 deletions.
diff --git a/internal/cmdtmpl/command.go b/internal/cmdtmpl/command.go
@@ -217,21 +217,6 @@ add element inet oc-daemon-routing excludes4 { {{.}} }
 			},
 			defaultTemplate: SplitRoutingDefaultTemplate,
 		}
-	case "SplitRoutingCleanup":
-		// Cleanup
-		cl = &CommandList{
-			Name: name,
-			Commands: []*Command{
-				{Line: "{{.Executables.IP}} -4 rule delete pref {{.SplitRouting.RulePriority1}}"},
-				{Line: "{{.Executables.IP}} -4 rule delete pref {{.SplitRouting.RulePriority2}}"},
-				{Line: "{{.Executables.IP}} -6 rule delete pref {{.SplitRouting.RulePriority1}}"},
-				{Line: "{{.Executables.IP}} -6 rule delete pref {{.SplitRouting.RulePriority2}}"},
-				{Line: "{{.Executables.IP}} -4 route flush table {{.SplitRouting.RoutingTable}}"},
-				{Line: "{{.Executables.IP}} -6 route flush table {{.SplitRouting.RoutingTable}}"},
-				{Line: "{{.Executables.Nft}} -f - delete table inet oc-daemon-routing"},
-			},
-			defaultTemplate: SplitRoutingDefaultTemplate,
-		}
 	default:
 		return nil
 
@@ -579,8 +564,18 @@ func getCommandListVPNSetup(name string) *CommandList {
 		cl = &CommandList{
 			Name: name,
 			Commands: []*Command{
+				// DNS cleanup
 				{Line: "{{.Executables.Resolvectl}} revert {{.OpenConnect.VPNDevice}}"},
+				// Device cleanup
 				{Line: "{{.Executables.IP}} link delete {{.OpenConnect.VPNDevice}}"},
+				// Routing cleanup
+				{Line: "{{.Executables.IP}} -4 rule delete pref {{.SplitRouting.RulePriority1}}"},
+				{Line: "{{.Executables.IP}} -4 rule delete pref {{.SplitRouting.RulePriority2}}"},
+				{Line: "{{.Executables.IP}} -6 rule delete pref {{.SplitRouting.RulePriority1}}"},
+				{Line: "{{.Executables.IP}} -6 rule delete pref {{.SplitRouting.RulePriority2}}"},
+				{Line: "{{.Executables.IP}} -4 route flush table {{.SplitRouting.RoutingTable}}"},
+				{Line: "{{.Executables.IP}} -6 route flush table {{.SplitRouting.RoutingTable}}"},
+				{Line: "{{.Executables.Nft}} -f - delete table inet oc-daemon-routing"},
 			},
 			defaultTemplate: "",
 		}

diff --git a/internal/cmdtmpl/command_test.go b/internal/cmdtmpl/command_test.go
@@ -39,7 +39,6 @@ func TestGetCommandList(t *testing.T) {
 		"SplitRoutingSetupRouting",
 		"SplitRoutingTeardownRouting",
 		"SplitRoutingSetExcludes",
-		"SplitRoutingCleanup",
 
 		// Traffic Policing
 		"TrafPolSetFilterRules",
@@ -98,7 +97,6 @@ func TestGetCmds(t *testing.T) {
 		"SplitRoutingSetupRouting",
 		"SplitRoutingTeardownRouting",
 		// "SplitRoutingSetExcludes", // skip, requires excludes
-		"SplitRoutingCleanup",
 
 		// Traffic Policing
 		"TrafPolSetFilterRules",

diff --git a/internal/splitrt/splitrt.go b/internal/splitrt/splitrt.go
@@ -345,20 +345,3 @@ func NewSplitRouting(config *daemoncfg.Config) *SplitRouting {
 		closed:   make(chan struct{}),
 	}
 }
-
-// Cleanup cleans up old configuration after a failed shutdown.
-func Cleanup(ctx context.Context, config *daemoncfg.Config) {
-	cmds, err := cmdtmpl.GetCmds("SplitRoutingCleanup", config)
-	if err != nil {
-		log.WithError(err).Error("SplitRouting could not get cleanup commands")
-	}
-	for _, c := range cmds {
-		if _, _, err := c.Run(ctx); err == nil {
-			log.WithFields(log.Fields{
-				"command": c.Cmd,
-				"args":    c.Args,
-				"stdin":   c.Stdin,
-			}).Debug("SplitRouting cleaned up configuration")
-		}
-	}
-}
diff --git a/internal/splitrt/splitrt_test.go b/internal/splitrt/splitrt_test.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"net/netip"
 	"reflect"
-	"strings"
 	"testing"
 
 	"github.com/telekom-mms/oc-daemon/internal/addrmon"
@@ -350,33 +349,3 @@ func TestNewSplitRouting(t *testing.T) {
 		t.Errorf("got nil, want != nil")
 	}
 }
-
-// TestCleanup tests Cleanup.
-func TestCleanup(t *testing.T) {
-	got := []string{}
-
-	oldRunCmd := execs.RunCmd
-	execs.RunCmd = func(_ context.Context, cmd string, s string, arg ...string) ([]byte, []byte, error) {
-		if s == "" {
-			got = append(got, cmd+" "+strings.Join(arg, " "))
-			return nil, nil, nil
-		}
-		got = append(got, cmd+" "+strings.Join(arg, " ")+" "+s)
-		return nil, nil, nil
-	}
-	defer func() { execs.RunCmd = oldRunCmd }()
-
-	Cleanup(context.Background(), daemoncfg.NewConfig())
-	want := []string{
-		"ip -4 rule delete pref 2111",
-		"ip -4 rule delete pref 2112",
-		"ip -6 rule delete pref 2111",
-		"ip -6 rule delete pref 2112",
-		"ip -4 route flush table 42111",
-		"ip -6 route flush table 42111",
-		"nft -f - delete table inet oc-daemon-routing",
-	}
-	if !reflect.DeepEqual(got, want) {
-		t.Errorf("got %v, want %v", got, want)
-	}
-}
diff --git a/internal/vpnsetup/vpnsetup.go b/internal/vpnsetup/vpnsetup.go
@@ -560,8 +560,7 @@ func Cleanup(ctx context.Context, config *daemoncfg.Config) {
 				"command": c.Cmd,
 				"args":    c.Args,
 				"stdin":   c.Stdin,
-			}).Warn("VPNSetup cleaned up configuration")
+			}).Debug("VPNSetup cleaned up configuration")
 		}
 	}
-	splitrt.Cleanup(ctx, config)
 }