Skip to content

Commit

Permalink
Revert "support for switch port toggle on/off (#119)"
Browse files Browse the repository at this point in the history
This reverts commit 782fd9d.
  • Loading branch information
robertvolkmann committed Jun 18, 2024
1 parent 784be7b commit 94f5801
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 345 deletions.
67 changes: 8 additions & 59 deletions cmd/internal/core/reconfigure-switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package core

import (
"fmt"
"net"
"os"
"slices"
"strconv"
Expand All @@ -24,7 +23,7 @@ func (c *Core) ReconfigureSwitch() {
for range t.C {
c.log.Info("trigger reconfiguration")
start := time.Now()
s, err := c.reconfigureSwitch(host)
err := c.reconfigureSwitch(host)
elapsed := time.Since(start)
c.log.Info("reconfiguration took", "elapsed", elapsed)

Expand All @@ -33,7 +32,6 @@ func (c *Core) ReconfigureSwitch() {
ns := elapsed.Nanoseconds()
nr := &models.V1SwitchNotifyRequest{
SyncDuration: &ns,
PortStates: make(map[string]string),
}
if err != nil {
errStr := err.Error()
Expand All @@ -44,30 +42,6 @@ func (c *Core) ReconfigureSwitch() {
c.log.Info("reconfiguration succeeded")
}

// fill the port states of the switch
for _, n := range s.Nics {
if n == nil || n.Name == nil {
// lets log the whole nic because the name could be empty; lets hope there is some useful information
// in the nic
c.log.Error("could not check if link is up", "nic", n)
c.metrics.CountError("switch-reconfiguration")
continue
}
isup, err := isLinkUp(*n.Name)
if err != nil {
c.log.Error("could not check if link is up", "error", err, "nicname", *n.Name)
nr.PortStates[*n.Name] = models.V1SwitchNicActualUNKNOWN
c.metrics.CountError("switch-reconfiguration")
continue
}
if isup {
nr.PortStates[*n.Name] = models.V1SwitchNicActualUP
} else {
nr.PortStates[*n.Name] = models.V1SwitchNicActualDOWN
}

}

params.Body = nr
_, err = c.driver.SwitchOperations().NotifySwitch(params, nil)
if err != nil {
Expand All @@ -77,37 +51,37 @@ func (c *Core) ReconfigureSwitch() {
}
}

func (c *Core) reconfigureSwitch(switchName string) (*models.V1SwitchResponse, error) {
func (c *Core) reconfigureSwitch(switchName string) error {
params := sw.NewFindSwitchParams()
params.ID = switchName
fsr, err := c.driver.SwitchOperations().FindSwitch(params, nil)
if err != nil {
return nil, fmt.Errorf("could not fetch switch from metal-api: %w", err)
return fmt.Errorf("could not fetch switch from metal-api: %w", err)
}

s := fsr.Payload
switchConfig, err := c.buildSwitcherConfig(s)
if err != nil {
return nil, fmt.Errorf("could not build switcher config: %w", err)
return fmt.Errorf("could not build switcher config: %w", err)
}

err = fillEth0Info(switchConfig, c.managementGateway)
if err != nil {
return nil, fmt.Errorf("could not gather information about eth0 nic: %w", err)
return fmt.Errorf("could not gather information about eth0 nic: %w", err)
}

c.log.Info("assembled new config for switch", "config", switchConfig)
if !c.enableReconfigureSwitch {
c.log.Debug("skip config application because of environment setting")
return s, nil
return nil
}

err = c.nos.Apply(switchConfig)
if err != nil {
return nil, fmt.Errorf("could not apply switch config: %w", err)
return fmt.Errorf("could not apply switch config: %w", err)
}

return s, nil
return nil
}

func (c *Core) buildSwitcherConfig(s *models.V1SwitchResponse) (*types.Conf, error) {
Expand All @@ -130,18 +104,10 @@ 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{},
}
p.BladePorts = c.additionalBridgePorts
for _, nic := range s.Nics {
port := *nic.Name

if isPortStatusEqual(models.V1SwitchNicActualDOWN, nic.Actual) {
if has := p.DownPorts[port]; !has {
p.DownPorts[port] = true
}
}

if slices.Contains(p.Underlay, port) {
continue
}
Expand Down Expand Up @@ -236,20 +202,3 @@ func fillEth0Info(c *types.Conf, gw string) error {
c.Ports.Eth0.Gateway = gw
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)
}
1 change: 0 additions & 1 deletion cmd/internal/core/reconfigure-switch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func TestBuildSwitcherConfig(t *testing.T) {
MetalCoreCIDR: "10.255.255.2/24",
ASN: 420000001,
Ports: types.Ports{
DownPorts: map[string]bool{},
Underlay: []string{"swp31", "swp32"},
Unprovisioned: []string{"swp1"},
Firewalls: map[string]*types.Firewall{
Expand Down
5 changes: 2 additions & 3 deletions cmd/internal/switcher/cumulus/cumulus.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ func New(log *slog.Logger, frrTplFile, interfacesTplFile string) *Cumulus {
}

func (c *Cumulus) Apply(cfg *types.Conf) error {
withoutDownPorts := cfg.NewWithoutDownPorts()
err := c.interfacesApplier.Apply(withoutDownPorts)
err := c.interfacesApplier.Apply(cfg)
if err != nil {
return err
}

return c.frrApplier.Apply(withoutDownPorts)
return c.frrApplier.Apply(cfg)
}

func (c *Cumulus) IsInitialized() (initialized bool, err error) {
Expand Down
9 changes: 2 additions & 7 deletions cmd/internal/switcher/sonic/db/configdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const (
portTable = "PORT"
adminStatus = "admin_status"
adminStatusUp = "up"
adminStatusDown = "down"
mtu = "mtu"
fec = "fec"
fecRS = "rs"
Expand Down Expand Up @@ -232,12 +231,8 @@ 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) SetAdminStatusUp(ctx context.Context, interfaceName string) error {
key := Key{portTable, interfaceName}

status := adminStatusUp
if !up {
status = adminStatusDown
}
return d.c.HSet(ctx, key, Val{adminStatus: status})
return d.c.HSet(ctx, key, Val{adminStatus: adminStatusUp})
}
27 changes: 12 additions & 15 deletions cmd/internal/switcher/sonic/redis/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ func (a *Applier) Apply(cfg *types.Conf) error {
}

for _, interfaceName := range cfg.Ports.Underlay {
if err := a.configureUnderlayPort(interfaceName, !cfg.Ports.DownPorts[interfaceName]); err != nil {
if err := a.configureUnderlayPort(interfaceName); err != nil {
errs = append(errs, err)
}
}

for _, interfaceName := range cfg.Ports.Unprovisioned {
if err := a.configureUnprovisionedPort(interfaceName, !cfg.Ports.DownPorts[interfaceName]); err != nil {
if err := a.configureUnprovisionedPort(interfaceName); err != nil {
errs = append(errs, err)
}
}

for interfaceName := range cfg.Ports.Firewalls {
if err := a.configureFirewallPort(interfaceName, !cfg.Ports.DownPorts[interfaceName]); err != nil {
if err := a.configureFirewallPort(interfaceName); err != nil {
errs = append(errs, err)
}
}
Expand All @@ -71,7 +71,7 @@ func (a *Applier) Apply(cfg *types.Conf) error {
errs = append(errs, err)
}
for _, interfaceName := range vrf.Neighbors {
if err := a.configureVrfNeighbor(interfaceName, vrfName, !cfg.Ports.DownPorts[interfaceName]); err != nil {
if err := a.configureVrfNeighbor(interfaceName, vrfName); err != nil {
errs = append(errs, err)
}
}
Expand Down Expand Up @@ -115,7 +115,7 @@ func (a *Applier) refreshOidMaps() error {
return nil
}

func (a *Applier) configureUnprovisionedPort(interfaceName string, isUp bool) error {
func (a *Applier) configureUnprovisionedPort(interfaceName string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

Expand All @@ -124,15 +124,14 @@ func (a *Applier) configureUnprovisionedPort(interfaceName string, isUp bool) er
return err
}

// unprovisioned ports should be up
if err := a.ensurePortConfiguration(ctx, interfaceName, "9000", true, isUp); err != nil {
if err := a.ensurePortConfiguration(ctx, interfaceName, "9000", true); err != nil {
return fmt.Errorf("failed to update Port info for interface %s: %w", interfaceName, err)
}

return a.ensureInterfaceIsVlanMember(ctx, interfaceName, "Vlan4000")
}

func (a *Applier) configureFirewallPort(interfaceName string, isUp bool) error {
func (a *Applier) configureFirewallPort(interfaceName string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

Expand All @@ -141,26 +140,24 @@ func (a *Applier) configureFirewallPort(interfaceName string, isUp bool) error {
return err
}

// a firewall port should always be up
if err := a.ensurePortConfiguration(ctx, interfaceName, "9216", true, isUp); err != nil {
if err := a.ensurePortConfiguration(ctx, interfaceName, "9216", true); err != nil {
return fmt.Errorf("failed to update Port info for interface %s: %w", interfaceName, err)
}

return a.ensureLinkLocalOnlyIsEnabled(ctx, interfaceName)
}

func (a *Applier) configureUnderlayPort(interfaceName string, isUp bool) error {
func (a *Applier) configureUnderlayPort(interfaceName string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

// underlay ports should be up
if err := a.ensurePortConfiguration(ctx, interfaceName, "9216", false, isUp); err != nil {
if err := a.ensurePortConfiguration(ctx, interfaceName, "9216", false); err != nil {
return fmt.Errorf("failed to update Port info for interface %s: %w", interfaceName, err)
}
return a.ensureLinkLocalOnlyIsEnabled(ctx, interfaceName)
}

func (a *Applier) configureVrfNeighbor(interfaceName, vrfName string, isUp bool) error {
func (a *Applier) configureVrfNeighbor(interfaceName, vrfName string) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

Expand All @@ -174,7 +171,7 @@ func (a *Applier) configureVrfNeighbor(interfaceName, vrfName string, isUp bool)
return err
}

if err := a.ensurePortConfiguration(ctx, interfaceName, "9000", true, isUp); err != nil {
if err := a.ensurePortConfiguration(ctx, interfaceName, "9000", true); err != nil {
return fmt.Errorf("failed to update Port info for interface %s: %w", interfaceName, err)
}

Expand Down
8 changes: 4 additions & 4 deletions cmd/internal/switcher/sonic/redis/port.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/avast/retry-go/v4"
)

func (a *Applier) ensurePortConfiguration(ctx context.Context, portName, mtu string, isFecRs, isUp bool) error {
func (a *Applier) ensurePortConfiguration(ctx context.Context, portName, mtu string, isFecRs bool) error {
p, err := a.db.Config.GetPort(ctx, portName)
if err != nil {
return fmt.Errorf("could not retrieve port info for %s from redis: %w", portName, err)
Expand All @@ -29,9 +29,9 @@ func (a *Applier) ensurePortConfiguration(ctx context.Context, portName, mtu str
}
}

if p.AdminStatus != isUp {
a.log.Debug("set admin status to", "port", portName, "admin_status_up", isUp)
return a.db.Config.SetAdminStatusUp(ctx, portName, isUp)
if !p.AdminStatus {
a.log.Debug("set admin status to", "port", portName, "admin_status", "up")
return a.db.Config.SetAdminStatusUp(ctx, portName)
}

return nil
Expand Down
1 change: 0 additions & 1 deletion cmd/internal/switcher/sonic/sonic.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ func getPortsConfig(filepath string) (map[string]PortInfo, error) {
config := struct {
Ports map[string]PortInfo `json:"PORT"`
}{}
//nolint:musttag
err = json.Unmarshal(byteValue, &config)

return config.Ports, err
Expand Down
9 changes: 1 addition & 8 deletions cmd/internal/switcher/templates/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ func TestInterfacesTemplate(t *testing.T) {
}
}

func TestInterfacesTemplateWithDownPorts(t *testing.T) {
c := readConf(t, path.Join("test_down_interfaces", "conf_with_downports.yaml"))
c = *c.NewWithoutDownPorts()
tpl := InterfacesTemplate("")
verifyTemplate(t, tpl, &c, path.Join("test_down_interfaces", "interfaces_with_downports"))
}

func TestCumulusFrrTemplate(t *testing.T) {
tests := listTestCases()
for i := range tests {
Expand Down Expand Up @@ -83,7 +76,7 @@ func TestCustomSonicFrrTemplate(t *testing.T) {
func verifyTemplate(t *testing.T, tpl *template.Template, c *types.Conf, expectedFilename string) {
actual := renderToString(t, tpl, c)
expected := readExpected(t, expectedFilename)
require.Equal(t, expected, actual, "Wanted: %s\nGot: %s", expected, actual)
require.Equal(t, expected, actual, "Wanted: %s, Got: %s", expected, actual)
}

func renderToString(t *testing.T, tpl *template.Template, c *types.Conf) string {
Expand Down

This file was deleted.

Loading

0 comments on commit 94f5801

Please sign in to comment.