check admin status instead of actual status

cmd/internal/core/reconfigure-switch.go

@@ -17,6 +17,7 @@ import (
 	"github.com/metal-stack/metal-core/cmd/internal/vlan"
 	sw "github.com/metal-stack/metal-go/api/client/switch_operations"
 	"github.com/metal-stack/metal-go/api/models"
+	"github.com/metal-stack/metal-lib/pkg/pointer"
 )
 
 // ConstantlyReconfigureSwitch reconfigures the switch.
@@ -162,20 +163,22 @@ func (c *Core) buildSwitcherConfig(s *models.V1SwitchResponse) (*types.Conf, err
 		Unprovisioned: []string{},
 		Vrfs:          map[string]*types.Vrf{},
 		Firewalls:     map[string]*types.Firewall{},
-		DownPorts:     map[string]bool{},
+		AdminStatus:   map[string]types.PortStatus{},
 	}
 
 	for _, nic := range s.Nics {
-		port := *nic.Name
+		if nic == nil {
+			continue
+		}
 
+		port := pointer.SafeDeref(nic.Name)
 		if slices.Contains(p.Underlay, port) {
 			continue
 		}
 
-		if isPortStatusEqual(models.V1SwitchNicActualDOWN, nic.Actual) {
-			if has := p.DownPorts[port]; !has {
-				p.DownPorts[port] = true
-			}
+		adminStatus := types.PortStatus(strings.ToLower(nic.AdminStatus))
+		if adminStatus == types.PortStatusDown || adminStatus == types.PortStatusUp {
+			p.AdminStatus[port] = types.PortStatus(adminStatus)
 		}
 
 		if slices.Contains(c.additionalBridgePorts, port) {
@@ -275,19 +278,10 @@ func fillEth0Info(c *types.Conf, gw string) error {
 	return nil
 }
 
-// isLinkUp checks if the interface with the given name is up.
-// It returns a boolean indicating if the interface is up, and an error if there was a problem checking the interface.
 func isLinkUp(nicname string) (bool, error) {
 	nic, err := net.InterfaceByName(nicname)
 	if err != nil {
 		return false, fmt.Errorf("cannot query interface %q : %w", nicname, err)
 	}
-	return nic.Flags&net.FlagUp != 0, nil
-}
-
-func isPortStatusEqual(stat string, other *string) bool {
-	if other == nil {
-		return false
-	}
-	return strings.EqualFold(stat, *other)
+	return nic.Flags&net.FlagRunning != 0, nil
 }

cmd/internal/core/reconfigure-switch_test.go

@@ -63,7 +63,7 @@ func TestBuildSwitcherConfig(t *testing.T) {
 		MetalCoreCIDR: "10.255.255.2/24",
 		ASN:           420000001,
 		Ports: types.Ports{
-			DownPorts:     map[string]bool{},
+			AdminStatus:   map[string]types.PortStatus{},
 			Underlay:      []string{"swp31", "swp32"},
 			Unprovisioned: []string{"swp1"},
 			Firewalls: map[string]*types.Firewall{

cmd/internal/switcher/sonic/db/configdb.go

@@ -4,13 +4,12 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/metal-stack/metal-core/cmd/internal/switcher/types"
 	"github.com/valkey-io/valkey-go"
 )
 
 const (
-	adminStatus         = "admin_status"
-	adminStatusUp       = "up"
-	adminStatusDown     = "down"
+	adminStatusField    = "admin_status"
 	alias               = "alias"
 	enable              = "enable"
 	interfaceTable      = "INTERFACE"
@@ -35,7 +34,7 @@ type ConfigDB struct {
 type Port struct {
 	Name        string
 	Alias       string
-	AdminStatus bool
+	AdminStatus string
 	Mtu         string
 }
 
@@ -303,7 +302,7 @@ func (d *ConfigDB) GetPort(ctx context.Context, interfaceName string) (*Port, er
 	return &Port{
 		Name:        interfaceName,
 		Alias:       result[alias],
-		AdminStatus: result[adminStatus] == adminStatusUp,
+		AdminStatus: result[adminStatusField],
 		Mtu:         result[mtu],
 	}, nil
 }
@@ -333,7 +332,7 @@ func (d *ConfigDB) GetPorts(ctx context.Context) ([]*Port, error) {
 		ports = append(ports, &Port{
 			Name:        p,
 			Alias:       result[alias],
-			AdminStatus: result[adminStatus] == adminStatusUp,
+			AdminStatus: result[adminStatusField],
 			Mtu:         result[mtu],
 		})
 	}
@@ -347,12 +346,21 @@ func (d *ConfigDB) SetPortMtu(ctx context.Context, interfaceName string, val str
 	return d.c.HSet(ctx, key, Val{mtu: val})
 }
 
-func (d *ConfigDB) SetAdminStatusUp(ctx context.Context, interfaceName string, up bool) error {
+func (d *ConfigDB) GetAdminStatus(ctx context.Context, interfaceName string) (types.PortStatus, error) {
 	key := Key{portTable, interfaceName}
 
-	status := adminStatusUp
-	if !up {
-		status = adminStatusDown
+	status, err := d.c.HGet(ctx, key, adminStatusField)
+	if err != nil {
+		return "", err
 	}
-	return d.c.HSet(ctx, key, Val{adminStatus: status})
+
+	return types.PortStatus(status), nil
+}
+
+func (d *ConfigDB) SetAdminStatus(ctx context.Context, interfaceName string, adminStatus types.PortStatus) error {
+	if adminStatus != types.PortStatusDown && adminStatus != types.PortStatusUp {
+		return fmt.Errorf("unknown admin status %s", adminStatus)
+	}
+	key := Key{portTable, interfaceName}
+	return d.c.HSet(ctx, key, Val{adminStatusField: string(adminStatus)})
 }

cmd/internal/switcher/sonic/db/configdb_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/metal-stack/metal-core/cmd/internal/switcher/sonic/db/test"
+	"github.com/metal-stack/metal-core/cmd/internal/switcher/types"
 	"github.com/stretchr/testify/require"
 )
 
@@ -891,6 +892,7 @@ func TestConfigDB_GetVrfs(t *testing.T) {
 				c: c,
 			}
 			got, err := d.GetVrfs(ctx)
+			slices.Sort(got)
 			require.NoError(t, err)
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Errorf("ConfigDB.GetVrfs() diff = %s", diff)
@@ -1651,7 +1653,7 @@ func TestConfigDB_GetPort(t *testing.T) {
 			want: &Port{
 				Name:        "Ethernet1",
 				Alias:       "Eth1/2",
-				AdminStatus: true,
+				AdminStatus: string(types.PortStatusUp),
 				Mtu:         "9000",
 			},
 		},
@@ -1697,13 +1699,13 @@ func TestConfigDB_GetPorts(t *testing.T) {
 				{
 					Name:        "Ethernet0",
 					Alias:       "Eth1/1",
-					AdminStatus: true,
+					AdminStatus: string(types.PortStatusUp),
 					Mtu:         "9216",
 				},
 				{
 					Name:        "Ethernet1",
 					Alias:       "Eth1/2",
-					AdminStatus: true,
+					AdminStatus: string(types.PortStatusUp),
 					Mtu:         "9000",
 				},
 			},
@@ -1816,14 +1818,15 @@ func TestConfigDB_SetAdminStatusUp(t *testing.T) {
 		name          string
 		data          test.StringMap
 		interfaceName string
-		up            bool
+		status        types.PortStatus
 		mods          func(test.HashMap)
+		wantErr       bool
 	}{
 		{
 			name:          "set on non-existing",
 			data:          configDBTestData,
 			interfaceName: "Ethernet2",
-			up:            false,
+			status:        types.PortStatusDown,
 			mods: func(data test.HashMap) {
 				data["PORT|Ethernet2"] = map[string]string{
 					"admin_status": "down",
@@ -1834,18 +1837,25 @@ func TestConfigDB_SetAdminStatusUp(t *testing.T) {
 			name:          "set same as existing",
 			data:          configDBTestData,
 			interfaceName: "Ethernet1",
-			up:            true,
+			status:        types.PortStatusUp,
 			mods:          func(data test.HashMap) {},
 		},
 		{
 			name:          "change existing",
 			data:          configDBTestData,
 			interfaceName: "Ethernet1",
-			up:            false,
+			status:        types.PortStatusDown,
 			mods: func(data test.HashMap) {
 				data["PORT|Ethernet1"]["admin_status"] = "down"
 			},
 		},
+		{
+			name:          "invalid input",
+			data:          configDBTestData,
+			interfaceName: "Ethernet1",
+			status:        "unknown",
+			wantErr:       true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1871,8 +1881,10 @@ func TestConfigDB_SetAdminStatusUp(t *testing.T) {
 			d := &ConfigDB{
 				c: c,
 			}
-			err = d.SetAdminStatusUp(ctx, tt.interfaceName, tt.up)
-			require.NoError(t, err)
+			err = d.SetAdminStatus(ctx, tt.interfaceName, tt.status)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ConfigDB.SetAdminStatusUp() err = %v, want err %v", err, tt.wantErr)
+			}
 			data, err := test.GetData(ctx, vc, sep)
 			require.NoError(t, err)
 			if diff := cmp.Diff(initData, data); diff != "" {
@@ -1881,3 +1893,76 @@ func TestConfigDB_SetAdminStatusUp(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigDB_GetAdminStatus(t *testing.T) {
+	data := test.StringMap{
+		"PORT": test.StringMap{
+			"Ethernet0": test.StringMap{
+				"admin_status": "up",
+				"alias":        "Eth1/1",
+				"mtu":          "9216",
+			},
+			"Ethernet1": test.StringMap{
+				"alias": "Eth1/2",
+				"mtu":   "9000",
+			},
+			"Ethernet2": test.StringMap{
+				"admin_status": "down",
+				"alias":        "Eth1/3",
+				"mtu":          "9000",
+			},
+		},
+	}
+
+	tests := []struct {
+		name          string
+		data          test.StringMap
+		interfaceName string
+		want          types.PortStatus
+	}{
+		{
+			name:          "interface is up",
+			data:          data,
+			interfaceName: "Ethernet0",
+			want:          types.PortStatusUp,
+		},
+		{
+			name:          "interface is down",
+			data:          data,
+			interfaceName: "Ethernet2",
+			want:          types.PortStatusDown,
+		},
+		{
+			name:          "no admin_status defined",
+			data:          data,
+			interfaceName: "Ethernet1",
+			want:          "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var (
+				ctx = t.Context()
+				sep = "|"
+				vc  = test.StartValkey(t)
+			)
+			defer vc.Close()
+
+			err := test.LoadData(ctx, vc, tt.data, sep)
+			require.NoError(t, err)
+
+			c := &Client{
+				rdb: vc,
+				sep: sep,
+			}
+			d := &ConfigDB{
+				c: c,
+			}
+			got, err := d.GetAdminStatus(ctx, tt.interfaceName)
+			require.NoError(t, err)
+			if got != tt.want {
+				t.Errorf("ConfigDB.GetAdminStatus() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}