Skip to content

Commit

Permalink
Fixed L2 to L3 transition related bug
Browse files Browse the repository at this point in the history
Signed-off-by: Patryk Strusiewicz-Surmacki <[email protected]>
  • Loading branch information
p-strusiewiczsurmacki-mobica committed Oct 16, 2024
1 parent 6ba54c9 commit 0058989
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
14 changes: 11 additions & 3 deletions pkg/frr/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg *config.Co
for j := range in.VRFs[i].Import {
for k := range in.VRFs[i].Import[j].Items {
if in.VRFs[i].Import[j].Items[k].Action != "deny" {
return false, fmt.Errorf("only deny rules are allowed in import prefix-lists of mgmt VRFs")
return false, &ConfigurationError{fmt.Errorf("only deny rules are allowed in import prefix-lists of mgmt VRFs")}
}
// Swap deny to permit, this will be a prefix-list called from a deny route-map
in.VRFs[i].Import[j].Items[k].Action = "permit"
Expand All @@ -58,7 +58,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg *config.Co

currentConfig, err := os.ReadFile(m.ConfigPath)
if err != nil {
return false, fmt.Errorf("error reading configuration file: %w", err)
return false, &ConfigurationError{fmt.Errorf("error reading configuration file: %w", err)}
}

targetConfig, err := render(m.configTemplate, frrConfig)
Expand All @@ -72,7 +72,7 @@ func (m *Manager) Configure(in Configuration, nm *nl.Manager, nwopCfg *config.Co
if !bytes.Equal(currentConfig, targetConfig) {
err = os.WriteFile(m.ConfigPath, targetConfig, frrPermissions)
if err != nil {
return false, fmt.Errorf("error writing configuration file: %w", err)
return false, &ConfigurationError{fmt.Errorf("error writing configuration file: %w", err)}
}

return true, nil
Expand Down Expand Up @@ -187,3 +187,11 @@ func applyCfgReplacements(frrConfig []byte, replacements []config.Replacement) [
}
return frrConfig
}

type ConfigurationError struct {
err error
}

func (r *ConfigurationError) Error() string {
return fmt.Sprintf("FRR configuration error: %s", r.err.Error())
}
25 changes: 18 additions & 7 deletions pkg/reconciler/layer3.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package reconciler

import (
"context"
"errors"
"fmt"
"net"
"sort"
Expand Down Expand Up @@ -65,10 +66,13 @@ func (r *reconcile) reconcileLayer3(l3vnis []networkv1alpha1.VRFRouteConfigurati
return allConfigs[i].VNI < allConfigs[j].VNI
})

// reconfigure FRR
// Create FRR configuration and reload it
err = r.configureFRR(allConfigs)
if err != nil {
return err
if !errors.Is(err, &frr.ConfigurationError{}) {
return err
}
r.Logger.Error(err, "failed to configure FRR")
}

created, deletedVRF, err := r.reconcileL3Netlink(l3Configs)
Expand All @@ -93,12 +97,9 @@ func (r *reconcile) reconcileLayer3(l3vnis []networkv1alpha1.VRFRouteConfigurati
// We wait here for two seconds to let FRR settle after updating netlink devices
time.Sleep(defaultSleep)

// Make sure that all created netlink VRFs are up after FRR reload
time.Sleep(defaultSleep)
for _, info := range created {
if err := r.netlinkManager.UpL3(info); err != nil {
r.Logger.Error(err, "error setting L3 to state UP")
return fmt.Errorf("error setting L3 to state UP: %w", err)
r.Logger.Error(err, "error setting L3 to state UP", "interface", info)
}
}
return nil
Expand All @@ -110,7 +111,6 @@ func (r *reconcile) configureFRR(vrfConfigs []frr.VRFConfiguration) error {
ASN: r.config.ServerASN,
}, r.netlinkManager, r.config)
if err != nil {
r.Logger.Error(err, "error updating FRR configuration")
return fmt.Errorf("error updating FRR configuration: %w", err)
}

Expand Down Expand Up @@ -245,13 +245,17 @@ func (r *reconcile) reconcileL3Netlink(vrfConfigs []frr.VRFConfiguration) ([]nl.
}

// Check for VRFs that are configured on the host but no longer in Kubernetes
preexisting := []nl.VRFInformation{}
toDelete := []nl.VRFInformation{}
for i := range existing {
stillExists := false
for j := range vrfConfigs {
if vrfConfigs[j].Name == existing[i].Name && vrfConfigs[j].VNI == existing[i].VNI {
stillExists = true
existing[i].MTU = vrfConfigs[j].MTU
if !existing[i].MarkForDelete {
preexisting = append(preexisting, existing[i])
}
break
}
}
Expand All @@ -262,6 +266,13 @@ func (r *reconcile) reconcileL3Netlink(vrfConfigs []frr.VRFConfiguration) ([]nl.
}
}

// Make sure that all previously configured interfaces are up
for _, info := range preexisting {
if err := r.netlinkManager.UpL3(info); err != nil {
r.Logger.Error(err, "failed to set L3 up", "interface", info.Name)
}
}

// Check for VRFs that are in Kubernetes but not yet configured on the host
toCreate := prepareVRFsToCreate(vrfConfigs, existing)

Expand Down

0 comments on commit 0058989

Please sign in to comment.