Skip to content

Commit

Permalink
bgp: Fixed broken bgp speaker unit tests
Browse files Browse the repository at this point in the history
A previous commit broke the bgp speaker unit tests, which this commit
fixes.

Fixes: cilium#20508
Signed-off-by: Dylan Reimerink <[email protected]>
  • Loading branch information
dylandreimerink authored and borkmann committed Jul 14, 2022
1 parent 87b4fd4 commit 7550088
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 27 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ replace (
github.com/miekg/dns => github.com/cilium/dns v1.1.4-0.20190417235132-8e25ec9a0ff3
github.com/optiopay/kafka => github.com/cilium/kafka v0.0.0-20180809090225-01ce283b732b

go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20220628131316-3c8101856f47
go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20220713202445-9066eee9e0be

// Using private fork of controller-tools. See commit msg for more context
// as to why we are using a private fork.
Expand Down
4 changes: 2 additions & 2 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 29 additions & 2 deletions pkg/bgp/speaker/speaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,16 @@ func TestSpeakerOnUpdateNode(t *testing.T) {
atomic.AddInt32(&callCount, 1)
return types.SyncStateSuccess
},
PeerSession_: func() []metallbspr.Session {
GetBGPController_: func() *metallbspr.BGPController {
atomic.AddInt32(&callCount, 1)
return []metallbspr.Session{mockSession}
return &metallbspr.BGPController{
SvcAds: make(map[string][]*metallbbgp.Advertisement),
Peers: []*metallbspr.Peer{
{
BGP: mockSession,
},
},
}
},
}

Expand Down Expand Up @@ -449,6 +456,16 @@ func TestSpeakerOnDeleteNode(t *testing.T) {
atomic.AddInt32(&callCount, 1)
return []metallbspr.Session{mockSession}
},
GetBGPController_: func() *metallbspr.BGPController {
return &metallbspr.BGPController{
SvcAds: make(map[string][]*metallbbgp.Advertisement),
Peers: []*metallbspr.Peer{
{
BGP: mockSession,
},
},
}
},
}

spkr := &MetalLBSpeaker{
Expand Down Expand Up @@ -532,6 +549,16 @@ func TestSpeakerOnUpdateAndDeleteCiliumNode(t *testing.T) {
PeerSession_: func() []metallbspr.Session {
return []metallbspr.Session{mockSession}
},
GetBGPController_: func() *metallbspr.BGPController {
return &metallbspr.BGPController{
SvcAds: make(map[string][]*metallbbgp.Advertisement),
Peers: []*metallbspr.Peer{
{
BGP: mockSession,
},
},
}
},
}

spkr := &MetalLBSpeaker{
Expand Down
40 changes: 20 additions & 20 deletions vendor/go.universe.tf/metallb/pkg/speaker/bgp_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,49 +32,49 @@ import (
"github.com/go-kit/kit/log"
)

type peer struct {
type Peer struct {
cfg *config.Peer
bgp Session
BGP Session
}

type BGPController struct {
Logger log.Logger
MyNode string
nodeLabels labels.Set
peers []*peer
Peers []*Peer
SvcAds map[string][]*bgp.Advertisement
}

func (c *BGPController) SetConfig(l log.Logger, cfg *config.Config) error {
newPeers := make([]*peer, 0, len(cfg.Peers))
newPeers := make([]*Peer, 0, len(cfg.Peers))
newPeers:
for _, p := range cfg.Peers {
for i, ep := range c.peers {
for i, ep := range c.Peers {
if ep == nil {
continue
}
if reflect.DeepEqual(p, ep.cfg) {
newPeers = append(newPeers, ep)
c.peers[i] = nil
c.Peers[i] = nil
continue newPeers
}
}
// No existing peers match, create a new one.
newPeers = append(newPeers, &peer{
newPeers = append(newPeers, &Peer{
cfg: p,
})
}

oldPeers := c.peers
c.peers = newPeers
oldPeers := c.Peers
c.Peers = newPeers

for _, p := range oldPeers {
if p == nil {
continue
}
l.Log("event", "peerRemoved", "peer", p.cfg.Addr, "reason", "removedFromConfig", "msg", "peer deconfigured, closing BGP session")
if p.bgp != nil {
if err := p.bgp.Close(); err != nil {
if p.BGP != nil {
if err := p.BGP.Close(); err != nil {
l.Log("op", "setConfig", "error", err, "peer", p.cfg.Addr, "msg", "failed to shut down BGP session")
}
}
Expand Down Expand Up @@ -139,7 +139,7 @@ func (c *BGPController) syncPeers(l log.Logger) error {
errs int
needUpdateAds bool
)
for _, p := range c.peers {
for _, p := range c.Peers {
// First, determine if the peering should be active for this
// node.
shouldRun := false
Expand All @@ -151,14 +151,14 @@ func (c *BGPController) syncPeers(l log.Logger) error {
}

// Now, compare current state to intended state, and correct.
if p.bgp != nil && !shouldRun {
if p.BGP != nil && !shouldRun {
// Oops, session is running but shouldn't be. Shut it down.
l.Log("event", "peerRemoved", "peer", p.cfg.Addr, "reason", "filteredByNodeSelector", "msg", "peer deconfigured, closing BGP session")
if err := p.bgp.Close(); err != nil {
if err := p.BGP.Close(); err != nil {
l.Log("op", "syncPeers", "error", err, "peer", p.cfg.Addr, "msg", "failed to shut down BGP session")
}
p.bgp = nil
} else if p.bgp == nil && shouldRun {
p.BGP = nil
} else if p.BGP == nil && shouldRun {
// Session doesn't exist, but should be running. Create
// it.
l.Log("event", "peerAdded", "peer", p.cfg.Addr, "msg", "peer configured, starting BGP session")
Expand All @@ -171,7 +171,7 @@ func (c *BGPController) syncPeers(l log.Logger) error {
l.Log("op", "syncPeers", "error", err, "peer", p.cfg.Addr, "msg", "failed to create BGP session")
errs++
} else {
p.bgp = s
p.BGP = s
needUpdateAds = true
}
}
Expand Down Expand Up @@ -268,9 +268,9 @@ func (c *BGPController) SetNodeLabels(l log.Logger, lbls map[string]string) erro

// PeerSessions returns the underlying BGP sessions for direct use.
func (c *BGPController) PeerSessions() []Session {
s := make([]Session, len(c.peers))
for i, peer := range c.peers {
s[i] = peer.bgp
s := make([]Session, len(c.Peers))
for i, peer := range c.Peers {
s[i] = peer.BGP
}
return s
}
Expand Down
4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ go.uber.org/zap/internal/color
go.uber.org/zap/internal/exit
go.uber.org/zap/zapcore
go.uber.org/zap/zapgrpc
# go.universe.tf/metallb v0.11.0 => github.com/cilium/metallb v0.1.1-0.20220628131316-3c8101856f47
# go.universe.tf/metallb v0.11.0 => github.com/cilium/metallb v0.1.1-0.20220713202445-9066eee9e0be
## explicit; go 1.17
go.universe.tf/metallb/pkg/allocator
go.universe.tf/metallb/pkg/allocator/k8salloc
Expand Down Expand Up @@ -1658,5 +1658,5 @@ sigs.k8s.io/structured-merge-diff/v4/value
sigs.k8s.io/yaml
# github.com/miekg/dns => github.com/cilium/dns v1.1.4-0.20190417235132-8e25ec9a0ff3
# github.com/optiopay/kafka => github.com/cilium/kafka v0.0.0-20180809090225-01ce283b732b
# go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20220628131316-3c8101856f47
# go.universe.tf/metallb => github.com/cilium/metallb v0.1.1-0.20220713202445-9066eee9e0be
# sigs.k8s.io/controller-tools => github.com/cilium/controller-tools v0.6.2

0 comments on commit 7550088

Please sign in to comment.