From aa383f060d4db609df64787fcf52a569df4c8071 Mon Sep 17 00:00:00 2001 From: Kumar Atish Date: Fri, 18 Oct 2024 14:19:30 +0530 Subject: [PATCH] Add "ipv4-only" and "ipv6-only" flags for "antctl get bgppeers" Signed-off-by: Kumar Atish --- docs/antctl.md | 15 ++ .../apiserver/handlers/bgppeer/handler.go | 27 +++- .../handlers/bgppeer/handler_test.go | 131 +++++++++++++++--- pkg/agent/controller/bgp/controller.go | 22 ++- pkg/agent/controller/bgp/controller_test.go | 90 ++++++++++-- pkg/antctl/antctl.go | 24 +++- pkg/querier/querier.go | 4 +- pkg/querier/testing/mock_querier.go | 8 +- 8 files changed, 272 insertions(+), 49 deletions(-) diff --git a/docs/antctl.md b/docs/antctl.md index 82f4722e33b..bd94cc4c20d 100644 --- a/docs/antctl.md +++ b/docs/antctl.md @@ -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 ``` ### Upgrade existing objects of CRDs diff --git a/pkg/agent/apiserver/handlers/bgppeer/handler.go b/pkg/agent/apiserver/handlers/bgppeer/handler.go index 521ea206965..5164499f7fd 100644 --- a/pkg/agent/apiserver/handlers/bgppeer/handler.go +++ b/pkg/agent/apiserver/handlers/bgppeer/handler.go @@ -38,7 +38,32 @@ 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 + } + if !ipv4Only && !ipv6Only { + ipv4Only = true + ipv6Only = true + } + + peers, err := bq.GetBGPPeerStatus(r.Context(), ipv4Only, ipv6Only) if err != nil { if errors.Is(err, bgp.ErrBGPPolicyNotFound) { http.Error(w, "there is no effective bgp policy applied to the Node", http.StatusNotFound) diff --git a/pkg/agent/apiserver/handlers/bgppeer/handler_test.go b/pkg/agent/apiserver/handlers/bgppeer/handler_test.go index d9c9ca4153e..aab1846ddde 100644 --- a/pkg/agent/apiserver/handlers/bgppeer/handler_test.go +++ b/pkg/agent/apiserver/handlers/bgppeer/handler_test.go @@ -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{ { @@ -63,17 +135,28 @@ 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, }, } @@ -81,10 +164,12 @@ func TestBGPPeerQuery(t *testing.T) { 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() @@ -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) } }) } diff --git a/pkg/agent/controller/bgp/controller.go b/pkg/agent/controller/bgp/controller.go index 888cb4a77ba..58448423d4a 100644 --- a/pkg/agent/controller/bgp/controller.go +++ b/pkg/agent/controller/bgp/controller.go @@ -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() @@ -985,9 +985,25 @@ 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 } diff --git a/pkg/agent/controller/bgp/controller_test.go b/pkg/agent/controller/bgp/controller_test.go index 60a9eb91d37..6dc6bd5b01f 100644 --- a/pkg/agent/controller/bgp/controller_test.go +++ b/pkg/agent/controller/bgp/controller_test.go @@ -2166,37 +2166,103 @@ 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, + Address: ipv4Peer1Addr, + ASN: peer1ASN, SessionState: bgp.SessionEstablished, }, { - Address: "192.168.77.201", - ASN: 65002, + Address: ipv4Peer2Addr, + ASN: peer2ASN, SessionState: bgp.SessionActive, }, }, nil) }, expectedBgpPeerStatus: []bgp.PeerStatus{ { - Address: "192.168.77.200", - ASN: 65001, + Address: ipv4Peer1Addr, + ASN: peer1ASN, SessionState: bgp.SessionEstablished, }, { - Address: "192.168.77.201", - ASN: 65002, + Address: ipv4Peer2Addr, + ASN: peer2ASN, + SessionState: bgp.SessionActive, + }, + }, + }, + { + name: "get ipv6 bgp peers", + ipv6Peers: true, + existingState: &bgpPolicyState{}, + expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) { + mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{ + { + Address: ipv6Peer1Addr, + ASN: peer1ASN, + SessionState: bgp.SessionEstablished, + }, + { + Address: ipv6Peer2Addr, + ASN: peer2ASN, + SessionState: bgp.SessionActive, + }, + }, nil) + }, + expectedBgpPeerStatus: []bgp.PeerStatus{ + { + Address: ipv6Peer1Addr, + ASN: peer1ASN, + SessionState: bgp.SessionEstablished, + }, + { + Address: ipv6Peer2Addr, + ASN: peer2ASN, + SessionState: bgp.SessionActive, + }, + }, + }, + { + name: "get all bgp peers", + ipv4Peers: true, + ipv6Peers: true, + existingState: &bgpPolicyState{}, + expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) { + mockBGPServer.GetPeers(ctx).Return([]bgp.PeerStatus{ + { + Address: ipv4Peer1Addr, + ASN: peer1ASN, + SessionState: bgp.SessionEstablished, + }, + { + Address: ipv6Peer2Addr, + ASN: peer2ASN, + SessionState: bgp.SessionActive, + }, + }, nil) + }, + expectedBgpPeerStatus: []bgp.PeerStatus{ + { + Address: ipv4Peer1Addr, + ASN: peer1ASN, + SessionState: bgp.SessionEstablished, + }, + { + Address: ipv6Peer2Addr, + ASN: peer2ASN, SessionState: bgp.SessionActive, }, }, @@ -2208,7 +2274,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 @@ -2220,12 +2286,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) } }) } diff --git a/pkg/antctl/antctl.go b/pkg/antctl/antctl.go index 33aee26b692..f10c4c9e9b0 100644 --- a/pkg/antctl/antctl.go +++ b/pkg/antctl/antctl.go @@ -655,14 +655,30 @@ $ antctl get podmulticaststats pod -n namespace`, { use: "bgppeers", aliases: []string{"bgppeer"}, - short: "Print the current status of all bgp peers of effective bgppolicy", - long: "Print the current status of all bgp peers of effective bgppolicy which includes peer IP address with port, asn and state", - example: ` Get the list of bgppeers with their current status + short: "Print the current status of bgp peers of effective bgppolicy", + long: "Print the current status of bgp peers of effective bgppolicy which includes peer IP address with port, asn and state", + example: ` Get the list of all bgp peers with their current status $ antctl get bgppeers + Get the list of IPv4 bgp peers with their current status + $ antctl get bgppeers --ipv4-only + Get the list of IPv6 bgp peers with their current status + $ antctl get bgppeers --ipv6-only `, agentEndpoint: &endpoint{ nonResourceEndpoint: &nonResourceEndpoint{ - path: "/bgppeers", + path: "/bgppeers", + params: []flagInfo{ + { + name: "ipv4-only", + usage: "Get IPv4 bgp peers only", + isBool: true, + }, + { + name: "ipv6-only", + usage: "Get IPv6 bgp peers only", + isBool: true, + }, + }, outputType: multiple, }, }, diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index f64fdc924fb..3c7e94da4af 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -145,6 +145,6 @@ type ServiceExternalIPStatusQuerier interface { type AgentBGPPolicyInfoQuerier interface { // GetBGPPolicyInfo returns Name, RouterID, LocalASN and ListenPort of effective BGP Policy applied on the Node. GetBGPPolicyInfo() (string, string, int32, int32) - // GetBGPPeerStatus returns current status of all BGP Peers of effective BGP Policy applied on the Node. - GetBGPPeerStatus(ctx context.Context) ([]bgp.PeerStatus, error) + // GetBGPPeerStatus returns current status of BGP Peers of effective BGP Policy applied on the Node. + GetBGPPeerStatus(ctx context.Context, ipv4Peers, ipv6Peers bool) ([]bgp.PeerStatus, error) } diff --git a/pkg/querier/testing/mock_querier.go b/pkg/querier/testing/mock_querier.go index 71f44342bfa..d0def4ce9b9 100644 --- a/pkg/querier/testing/mock_querier.go +++ b/pkg/querier/testing/mock_querier.go @@ -360,18 +360,18 @@ func (m *MockAgentBGPPolicyInfoQuerier) EXPECT() *MockAgentBGPPolicyInfoQuerierM } // GetBGPPeerStatus mocks base method. -func (m *MockAgentBGPPolicyInfoQuerier) GetBGPPeerStatus(arg0 context.Context) ([]bgp.PeerStatus, error) { +func (m *MockAgentBGPPolicyInfoQuerier) GetBGPPeerStatus(arg0 context.Context, arg1, arg2 bool) ([]bgp.PeerStatus, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBGPPeerStatus", arg0) + ret := m.ctrl.Call(m, "GetBGPPeerStatus", arg0, arg1, arg2) ret0, _ := ret[0].([]bgp.PeerStatus) ret1, _ := ret[1].(error) return ret0, ret1 } // GetBGPPeerStatus indicates an expected call of GetBGPPeerStatus. -func (mr *MockAgentBGPPolicyInfoQuerierMockRecorder) GetBGPPeerStatus(arg0 any) *gomock.Call { +func (mr *MockAgentBGPPolicyInfoQuerierMockRecorder) GetBGPPeerStatus(arg0, arg1, arg2 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBGPPeerStatus", reflect.TypeOf((*MockAgentBGPPolicyInfoQuerier)(nil).GetBGPPeerStatus), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBGPPeerStatus", reflect.TypeOf((*MockAgentBGPPolicyInfoQuerier)(nil).GetBGPPeerStatus), arg0, arg1, arg2) } // GetBGPPolicyInfo mocks base method.