Skip to content

Commit

Permalink
Add "ipv4-only" and "ipv6-only" flags for "antctl get bgppeers" (#6755)
Browse files Browse the repository at this point in the history 
Signed-off-by: Kumar Atish <[email protected]>
  • Loading branch information
@Atish-iaf
Atish-iaf authored Oct 25, 2024
1 parent 38b3394 commit 71d57f1
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 64 deletions.
15 changes: 15 additions & 0 deletions docs/antctl.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -774,11 +774,26 @@ of effective BGP policy applied on the local Node. It includes Peer IP address w
ASN, and State of the BGP Peers.

```bash
# Get the list of all bgp peers
$ antctl get bgppeers

PEER ASN STATE
192.168.77.200:179 65001 Established
[fec0::196:168:77:251]:179 65002 Active

# Get the list of IPv4 bgp peers only
$ antctl get bgppeers --ipv4-only

PEER ASN STATE
192.168.77.200:179 65001 Established
192.168.77.201:179 65002 Active

# Get the list of IPv6 bgp peers only
$ antctl get bgppeers --ipv6-only

PEER ASN STATE
[fec0::196:168:77:251]:179 65001 Established
[fec0::196:168:77:252]:179 65002 Active
```

`antctl` agent command `get bgproutes` prints the advertised BGP routes on the local Node.
Expand Down
23 changes: 22 additions & 1 deletion pkg/agent/apiserver/handlers/bgppeer/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,28 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {
return
}

peers, err := bq.GetBGPPeerStatus(r.Context())
values := r.URL.Query()
var ipv4Only, ipv6Only bool
if values.Has("ipv4-only") {
if values.Get("ipv4-only") != "" {
http.Error(w, "invalid query", http.StatusBadRequest)
return
}
ipv4Only = true
}
if values.Has("ipv6-only") {
if values.Get("ipv6-only") != "" {
http.Error(w, "invalid query", http.StatusBadRequest)
return
}
ipv6Only = true
}
if ipv4Only && ipv6Only {
http.Error(w, "invalid query", http.StatusBadRequest)
return
}

peers, err := bq.GetBGPPeerStatus(r.Context(), !ipv6Only, !ipv4Only)
if err != nil {
if errors.Is(err, bgp.ErrBGPPolicyNotFound) {
http.Error(w, "there is no effective bgp policy applied to the Node", http.StatusNotFound)
Expand Down
131 changes: 108 additions & 23 deletions pkg/agent/apiserver/handlers/bgppeer/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,101 @@ import (
)

func TestBGPPeerQuery(t *testing.T) {
ctx := context.Background()
tests := []struct {
name string
fakeBGPPeerStatus []bgp.PeerStatus
expectedStatus int
expectedResponse []apis.BGPPeerResponse
fakeErr error
name string
url string
expectedCalls func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier)
expectedStatus int
expectedResponse []apis.BGPPeerResponse
}{
{
name: "bgpPolicyState exists",
fakeBGPPeerStatus: []bgp.PeerStatus{
name: "get ipv4 bgp peers only",
url: "?ipv4-only",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx, true, false).Return(
[]bgp.PeerStatus{
{
Address: "192.168.77.200",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "192.168.77.201",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
},
}, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
{
Address: "192.168.77.200",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
Peer: "192.168.77.200:179",
ASN: 65001,
State: "Established",
},
{
Address: "192.168.77.201",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
Peer: "192.168.77.201:179",
ASN: 65002,
State: "Active",
},
},
},
{
name: "get ipv6 bgp peers only",
url: "?ipv6-only=",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx, false, true).Return(
[]bgp.PeerStatus{
{
Address: "fec0::196:168:77:251",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "fec0::196:168:77:252",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
},
}, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
{
Peer: "[fec0::196:168:77:251]:179",
ASN: 65001,
State: "Established",
},
{
Peer: "[fec0::196:168:77:252]:179",
ASN: 65002,
State: "Active",
},
},
},
{
name: "get all bgp peers",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx, true, true).Return(
[]bgp.PeerStatus{
{
Address: "192.168.77.200",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "fec0::196:168:77:251",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
},
}, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
{
Expand All @@ -63,28 +135,41 @@ func TestBGPPeerQuery(t *testing.T) {
State: "Established",
},
{
Peer: "192.168.77.201:179",
Peer: "[fec0::196:168:77:251]:179",
ASN: 65002,
State: "Active",
},
},
},
{
name: "bgpPolicyState does not exist",
fakeBGPPeerStatus: nil,
expectedStatus: http.StatusNotFound,
fakeErr: bgpcontroller.ErrBGPPolicyNotFound,
name: "bgpPolicyState does not exist",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx, true, true).Return(nil, bgpcontroller.ErrBGPPolicyNotFound)
},
expectedStatus: http.StatusNotFound,
},
{
name: "flag with value",
url: "?ipv4-only=true",
expectedStatus: http.StatusBadRequest,
},
{
name: "both flags are passed",
url: "?ipv4-only&ipv6-only",
expectedStatus: http.StatusBadRequest,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
q := queriertest.NewMockAgentBGPPolicyInfoQuerier(ctrl)
q.EXPECT().GetBGPPeerStatus(context.Background()).Return(tt.fakeBGPPeerStatus, tt.fakeErr)
if tt.expectedCalls != nil {
tt.expectedCalls(q)
}
handler := HandleFunc(q)

req, err := http.NewRequest(http.MethodGet, "", nil)
req, err := http.NewRequest(http.MethodGet, tt.url, nil)
require.NoError(t, err)

recorder := httptest.NewRecorder()
Expand All @@ -95,7 +180,7 @@ func TestBGPPeerQuery(t *testing.T) {
var received []apis.BGPPeerResponse
err = json.Unmarshal(recorder.Body.Bytes(), &received)
require.NoError(t, err)
assert.ElementsMatch(t, tt.expectedResponse, received)
assert.Equal(t, tt.expectedResponse, received)
}
})
}
Expand Down
22 changes: 19 additions & 3 deletions pkg/agent/controller/bgp/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -970,8 +970,8 @@ func (c *Controller) GetBGPPolicyInfo() (string, string, int32, int32) {
return name, routerID, localASN, listenPort
}

// GetBGPPeerStatus returns current status of all BGP Peers of effective BGP Policy applied on the Node.
func (c *Controller) GetBGPPeerStatus(ctx context.Context) ([]bgp.PeerStatus, error) {
// GetBGPPeerStatus returns current status of BGP Peers of effective BGP Policy applied on the Node.
func (c *Controller) GetBGPPeerStatus(ctx context.Context, ipv4Peers, ipv6Peers bool) ([]bgp.PeerStatus, error) {
getBgpServer := func() bgp.Interface {
c.bgpPolicyStateMutex.RLock()
defer c.bgpPolicyStateMutex.RUnlock()
Expand All @@ -985,10 +985,26 @@ func (c *Controller) GetBGPPeerStatus(ctx context.Context) ([]bgp.PeerStatus, er
if bgpServer == nil {
return nil, ErrBGPPolicyNotFound
}
peers, err := bgpServer.GetPeers(ctx)
allPeers, err := bgpServer.GetPeers(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get bgp peers: %w", err)
}

peers := make([]bgp.PeerStatus, 0, len(allPeers))
if ipv4Peers { // insert IPv4 peers
for _, peer := range allPeers {
if utilnet.IsIPv4String(peer.Address) {
peers = append(peers, peer)
}
}
}
if ipv6Peers { // insert IPv6 peers
for _, peer := range allPeers {
if utilnet.IsIPv6String(peer.Address) {
peers = append(peers, peer)
}
}
}
return peers, nil
}

Expand Down
77 changes: 50 additions & 27 deletions pkg/agent/controller/bgp/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ var (
ipv6Peer1 = generateBGPPeer(ipv6Peer1Addr, peer1ASN, 179, 120)
ipv4Peer1Config = generateBGPPeerConfig(&ipv4Peer1, peer1AuthPassword)
ipv6Peer1Config = generateBGPPeerConfig(&ipv6Peer1, peer1AuthPassword)
ipv4Peer1Status = bgp.PeerStatus{
Address: ipv4Peer1Addr,
ASN: peer1ASN,
SessionState: bgp.SessionActive,
}
ipv6Peer1Status = bgp.PeerStatus{
Address: ipv6Peer1Addr,
ASN: peer1ASN,
SessionState: bgp.SessionActive,
}

peer2ASN = int32(65532)
peer2AuthPassword = "bgp-peer2" // #nosec G101
Expand All @@ -83,6 +93,16 @@ var (
ipv6Peer2 = generateBGPPeer(ipv6Peer2Addr, peer2ASN, 179, 120)
ipv4Peer2Config = generateBGPPeerConfig(&ipv4Peer2, peer2AuthPassword)
ipv6Peer2Config = generateBGPPeerConfig(&ipv6Peer2, peer2AuthPassword)
ipv4Peer2Status = bgp.PeerStatus{
Address: ipv4Peer2Addr,
ASN: peer2ASN,
SessionState: bgp.SessionActive,
}
ipv6Peer2Status = bgp.PeerStatus{
Address: ipv6Peer2Addr,
ASN: peer2ASN,
SessionState: bgp.SessionActive,
}

updatedIPv4Peer2 = generateBGPPeer(ipv4Peer2Addr, peer2ASN, 179, 60)
updatedIPv6Peer2 = generateBGPPeer(ipv6Peer2Addr, peer2ASN, 179, 60)
Expand Down Expand Up @@ -2166,40 +2186,43 @@ func TestGetBGPPeerStatus(t *testing.T) {
ctx := context.Background()
testCases := []struct {
name string
ipv4Peers bool
ipv6Peers bool
existingState *bgpPolicyState
expectedCalls func(mockBGPServer *bgptest.MockInterfaceMockRecorder)
expectedBgpPeerStatus []bgp.PeerStatus
expectedErr error
}{
{
name: "bgpPolicyState exists",
name: "get ipv4 bgp peers",
ipv4Peers: true,
existingState: &bgpPolicyState{},
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{
{
Address: "192.168.77.200",
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "192.168.77.201",
ASN: 65002,
SessionState: bgp.SessionActive,
},
}, nil)
mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{ipv4Peer1Status, ipv4Peer2Status,
ipv6Peer1Status, ipv6Peer2Status}, nil)
},
expectedBgpPeerStatus: []bgp.PeerStatus{
{
Address: "192.168.77.200",
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "192.168.77.201",
ASN: 65002,
SessionState: bgp.SessionActive,
},
expectedBgpPeerStatus: []bgp.PeerStatus{ipv4Peer1Status, ipv4Peer2Status},
},
{
name: "get ipv6 bgp peers",
ipv6Peers: true,
existingState: &bgpPolicyState{},
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{ipv4Peer1Status, ipv4Peer2Status,
ipv6Peer1Status, ipv6Peer2Status}, nil)
},
expectedBgpPeerStatus: []bgp.PeerStatus{ipv6Peer1Status, ipv6Peer2Status},
},
{
name: "get all bgp peers",
ipv4Peers: true,
ipv6Peers: true,
existingState: &bgpPolicyState{},
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{ipv4Peer1Status, ipv4Peer2Status,
ipv6Peer1Status, ipv6Peer2Status}, nil)
},
expectedBgpPeerStatus: []bgp.PeerStatus{ipv4Peer1Status, ipv4Peer2Status, ipv6Peer1Status, ipv6Peer2Status},
},
{
name: "bgpPolicyState does not exist",
Expand All @@ -2208,7 +2231,7 @@ func TestGetBGPPeerStatus(t *testing.T) {
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
c := newFakeController(t, nil, nil, true, false)
c := newFakeController(t, nil, nil, tt.ipv4Peers, tt.ipv6Peers)

// Fake the BGPPolicy state.
c.bgpPolicyState = tt.existingState
Expand All @@ -2220,12 +2243,12 @@ func TestGetBGPPeerStatus(t *testing.T) {
tt.expectedCalls(c.mockBGPServer.EXPECT())
}

actualBgpPeerStatus, err := c.GetBGPPeerStatus(ctx)
actualBgpPeerStatus, err := c.GetBGPPeerStatus(ctx, tt.ipv4Peers, tt.ipv6Peers)
if tt.expectedErr != nil {
assert.ErrorIs(t, err, tt.expectedErr)
} else {
require.NoError(t, err)
assert.ElementsMatch(t, actualBgpPeerStatus, tt.expectedBgpPeerStatus)
assert.Equal(t, tt.expectedBgpPeerStatus, actualBgpPeerStatus)
}
})
}
Expand Down
Loading

0 comments on commit 71d57f1

Please sign in to comment.