Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add 'type' arg for antctl get bgproutes to filter bgproutes #6835

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+177 −166
Open

Add 'type' arg for antctl get bgproutes to filter bgproutes #6835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/antctl.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,13 @@ ROUTE TYPE K8S-OBJ-REF
fec0::192:168:77:100/128 EgressIP egress2
fd00:10:244:1::/64 NodeIPAMPodCIDR <NONE>
fec0::10:96:10:10/128 ServiceLoadBalancerIP default/svc2

# Get the list of all advertised routes of a specific type
$ antctl get bgproutes -T EgressIP

ROUTE TYPE K8S-OBJ-REF
172.18.0.3/32 EgressIP egress1
fec0::192:168:77:100/128 EgressIP egress2
```

### Upgrade existing objects of CRDs
Expand Down
32 changes: 26 additions & 6 deletions pkg/agent/apiserver/handlers/bgppeer/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ import (
"errors"
"net"
"net/http"
"net/netip"
"reflect"
"slices"
"strconv"

"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/controller/bgp"
Expand Down Expand Up @@ -59,7 +62,7 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {
return
}

peers, err := bq.GetBGPPeerStatus(r.Context(), !ipv6Only, !ipv4Only)
peers, err := bq.GetBGPPeerStatus(r.Context())
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 All @@ -72,12 +75,29 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {

var bgpPeersResp []apis.BGPPeerResponse
for _, peer := range peers {
bgpPeersResp = append(bgpPeersResp, apis.BGPPeerResponse{
Peer: net.JoinHostPort(peer.Address, strconv.Itoa(int(peer.Port))),
ASN: peer.ASN,
State: string(peer.SessionState),
})
if (!ipv4Only && !ipv6Only) ||
(ipv4Only && utilnet.IsIPv4String(peer.Address)) ||
(ipv6Only && utilnet.IsIPv6String(peer.Address)) {
Comment on lines +78 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may be more readable to test the opposite:

if ipv4Only && !utilnet.IsIPv4String(peer.Address)) {
        continue
}
if ipv6Only && !utilnet.IsIPv6String(peer.Address)) {
        continue
}
// add to response

bgpPeersResp = append(bgpPeersResp, apis.BGPPeerResponse{
Peer: net.JoinHostPort(peer.Address, strconv.Itoa(int(peer.Port))),
ASN: peer.ASN,
State: string(peer.SessionState),
})
}
}
// make sure that we provide a stable order for the API response
slices.SortFunc(bgpPeersResp, func(a, b apis.BGPPeerResponse) int {
addrPortA, _ := netip.ParseAddrPort(a.Peer)
addrPortB, _ := netip.ParseAddrPort(b.Peer)
// IPv4 routes first, then IPv6 routes
if addrPortA.Addr().Is4() && !addrPortB.Addr().Is4() {
return -1
}
if !addrPortA.Addr().Is4() && addrPortB.Addr().Is4() {
return 1
}
return addrPortA.Compare(addrPortB)
})

if err := json.NewEncoder(w).Encode(bgpPeersResp); err != nil {
w.WriteHeader(http.StatusInternalServerError)
Expand Down
89 changes: 43 additions & 46 deletions pkg/agent/apiserver/handlers/bgppeer/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,35 @@ import (
queriertest "antrea.io/antrea/pkg/querier/testing"
)

var (
bgpPeerStatus = []bgp.PeerStatus{
{
Address: "192.168.77.201",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
},
{
Address: "fec0::196:168:77:252",
Port: 179,
ASN: 65002,
SessionState: bgp.SessionActive,
},
{
Address: "192.168.77.200",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
{
Address: "fec0::196:168:77:251",
Port: 179,
ASN: 65001,
SessionState: bgp.SessionEstablished,
},
}
)

func TestBGPPeerQuery(t *testing.T) {
ctx := context.Background()
tests := []struct {
Expand All @@ -44,21 +73,7 @@ func TestBGPPeerQuery(t *testing.T) {
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)
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx).Return(bgpPeerStatus, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
Expand All @@ -78,21 +93,7 @@ func TestBGPPeerQuery(t *testing.T) {
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)
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx).Return(bgpPeerStatus, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
Expand All @@ -111,21 +112,7 @@ func TestBGPPeerQuery(t *testing.T) {
{
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)
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx).Return(bgpPeerStatus, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPPeerResponse{
Expand All @@ -134,8 +121,18 @@ func TestBGPPeerQuery(t *testing.T) {
ASN: 65001,
State: "Established",
},
{
Peer: "192.168.77.201:179",
ASN: 65002,
State: "Active",
},
{
Peer: "[fec0::196:168:77:251]:179",
ASN: 65001,
State: "Established",
},
{
Peer: "[fec0::196:168:77:252]:179",
ASN: 65002,
State: "Active",
},
Expand All @@ -144,7 +141,7 @@ func TestBGPPeerQuery(t *testing.T) {
{
name: "bgpPolicyState does not exist",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx, true, true).Return(nil, bgpcontroller.ErrBGPPolicyNotFound)
mockBGPServer.EXPECT().GetBGPPeerStatus(ctx).Return(nil, bgpcontroller.ErrBGPPolicyNotFound)
},
expectedStatus: http.StatusNotFound,
},
Expand Down
21 changes: 14 additions & 7 deletions pkg/agent/apiserver/handlers/bgproute/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

"k8s.io/klog/v2"
"k8s.io/utils/net"

"antrea.io/antrea/pkg/agent/apis"
"antrea.io/antrea/pkg/agent/controller/bgp"
Expand All @@ -40,6 +41,7 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {
}

values := r.URL.Query()
bgpRouteType := values.Get("type")
var ipv4Only, ipv6Only bool
if values.Has("ipv4-only") {
if values.Get("ipv4-only") != "" {
Expand All @@ -60,7 +62,7 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {
return
}

bgpRoutes, err := bq.GetBGPRoutes(r.Context(), !ipv6Only, !ipv4Only)
bgpRoutes, err := bq.GetBGPRoutes(r.Context())
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 All @@ -71,12 +73,17 @@ func HandleFunc(bq querier.AgentBGPPolicyInfoQuerier) http.HandlerFunc {
}

var bgpRoutesResp []apis.BGPRouteResponse
for bgpRoute := range bgpRoutes {
bgpRoutesResp = append(bgpRoutesResp, apis.BGPRouteResponse{
Route: bgpRoute.Prefix,
Type: string(bgpRoutes[bgpRoute].Type),
K8sObjRef: bgpRoutes[bgpRoute].K8sObjRef,
})
for bgpRoute, routeMetadata := range bgpRoutes {
if ((!ipv4Only && !ipv6Only) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto:

if ipv4Only && !utilnet.IsIPv4String(peer.Address)) {
        continue
}
if ipv6Only && !utilnet.IsIPv6String(peer.Address)) {
        continue
}
if bgpRouteType != "" && bgpRouteType != string(routeMetadata.Type) {
        continue
}
// add to response

(ipv4Only && net.IsIPv4CIDRString(bgpRoute.Prefix)) ||
(ipv6Only && net.IsIPv6CIDRString(bgpRoute.Prefix))) &&
(bgpRouteType == "" || bgpRouteType == string(routeMetadata.Type)) {
bgpRoutesResp = append(bgpRoutesResp, apis.BGPRouteResponse{
Route: bgpRoute.Prefix,
Type: string(routeMetadata.Type),
K8sObjRef: routeMetadata.K8sObjRef,
})
}
}
// make sure that we provide a stable order for the API response
slices.SortFunc(bgpRoutesResp, func(a, b apis.BGPRouteResponse) int {
Expand Down
75 changes: 58 additions & 17 deletions pkg/agent/apiserver/handlers/bgproute/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,21 @@ var (
podIPv4CIDRRoute = bgp.Route{Prefix: podIPv4CIDR}
clusterIPv4 = "10.96.10.10"
clusterIPv4Route = bgp.Route{Prefix: ipStrToPrefix(clusterIPv4)}
egressIPv4 = "10.96.11.10"
egressIPv4Route = bgp.Route{Prefix: ipStrToPrefix(egressIPv4)}
loadBalancerIPv6 = "fec0::192:168:77:150"
loadBalancerIPv6Route = bgp.Route{Prefix: ipStrToPrefix(loadBalancerIPv6)}
egressIPv6 = "fec0::192:168:77:200"
egressIPv6Route = bgp.Route{Prefix: ipStrToPrefix(egressIPv6)}

ipv4ClusterIPName = "clusterip-4"
ipv4EgressName = "egress-4"
ipv6LoadBalancerName = "loadbalancer-6"
ipv6EgressName = "egress-6"

allRoutes = map[bgp.Route]bgpcontroller.RouteMetadata{
clusterIPv4Route: {Type: bgpcontroller.ServiceClusterIP, K8sObjRef: getServiceName(ipv4ClusterIPName)},
egressIPv4Route: {Type: bgpcontroller.EgressIP, K8sObjRef: ipv4EgressName},
loadBalancerIPv6Route: {Type: bgpcontroller.ServiceLoadBalancerIP, K8sObjRef: getServiceName(ipv6LoadBalancerName)},
egressIPv6Route: {Type: bgpcontroller.EgressIP, K8sObjRef: ipv6EgressName},
podIPv4CIDRRoute: {Type: bgpcontroller.NodeIPAMPodCIDR},
Expand All @@ -72,22 +76,22 @@ func TestBGPRouteQuery(t *testing.T) {
{
name: "bgpPolicyState does not exist",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(context.Background(), true, true).Return(nil, bgpcontroller.ErrBGPPolicyNotFound)
mockBGPServer.EXPECT().GetBGPRoutes(context.Background()).Return(nil, bgpcontroller.ErrBGPPolicyNotFound)
},
expectedStatus: http.StatusNotFound,
},
{
name: "get all advertised routes",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(ctx, true, true).Return(
map[bgp.Route]bgpcontroller.RouteMetadata{
clusterIPv4Route: allRoutes[clusterIPv4Route],
podIPv4CIDRRoute: allRoutes[podIPv4CIDRRoute],
egressIPv6Route: allRoutes[egressIPv6Route],
}, nil)
mockBGPServer.EXPECT().GetBGPRoutes(ctx).Return(allRoutes, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPRouteResponse{
{
Route: egressIPv4Route.Prefix,
Type: string(allRoutes[egressIPv4Route].Type),
K8sObjRef: allRoutes[egressIPv4Route].K8sObjRef,
},
{
Route: podIPv4CIDR,
Type: string(bgpcontroller.NodeIPAMPodCIDR),
Expand All @@ -102,20 +106,26 @@ func TestBGPRouteQuery(t *testing.T) {
Type: string(allRoutes[egressIPv6Route].Type),
K8sObjRef: allRoutes[egressIPv6Route].K8sObjRef,
},
{
Route: loadBalancerIPv6Route.Prefix,
Type: string(allRoutes[loadBalancerIPv6Route].Type),
K8sObjRef: allRoutes[loadBalancerIPv6Route].K8sObjRef,
},
},
},
{
name: "get advertised ipv4 routes only",
url: "?ipv4-only",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(ctx, true, false).Return(
map[bgp.Route]bgpcontroller.RouteMetadata{
clusterIPv4Route: allRoutes[clusterIPv4Route],
podIPv4CIDRRoute: allRoutes[podIPv4CIDRRoute],
}, nil)
mockBGPServer.EXPECT().GetBGPRoutes(ctx).Return(allRoutes, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPRouteResponse{
{
Route: egressIPv4Route.Prefix,
Type: string(allRoutes[egressIPv4Route].Type),
K8sObjRef: allRoutes[egressIPv4Route].K8sObjRef,
},
{
Route: podIPv4CIDRRoute.Prefix,
Type: string(allRoutes[podIPv4CIDRRoute].Type),
Expand All @@ -131,11 +141,7 @@ func TestBGPRouteQuery(t *testing.T) {
name: "get advertised ipv6 routes only",
url: "?ipv6-only=",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(ctx, false, true).Return(
map[bgp.Route]bgpcontroller.RouteMetadata{
loadBalancerIPv6Route: allRoutes[loadBalancerIPv6Route],
egressIPv6Route: allRoutes[egressIPv6Route],
}, nil)
mockBGPServer.EXPECT().GetBGPRoutes(ctx).Return(allRoutes, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPRouteResponse{
Expand All @@ -161,6 +167,41 @@ func TestBGPRouteQuery(t *testing.T) {
url: "?ipv4-only&ipv6-only",
expectedStatus: http.StatusBadRequest,
},
{
name: "get all advertised egressIP routes",
url: "?type=EgressIP",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(ctx).Return(allRoutes, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPRouteResponse{
{
Route: egressIPv4Route.Prefix,
Type: string(allRoutes[egressIPv4Route].Type),
K8sObjRef: allRoutes[egressIPv4Route].K8sObjRef,
},
{
Route: egressIPv6Route.Prefix,
Type: string(allRoutes[egressIPv6Route].Type),
K8sObjRef: allRoutes[egressIPv6Route].K8sObjRef,
},
},
},
{
name: "get advertised egress ipv4 routes only",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually for consistency with the name of the previous sub-test: "get advertised IPv4 egressIP routes"

url: "?ipv4-only&type=EgressIP",
expectedCalls: func(mockBGPServer *queriertest.MockAgentBGPPolicyInfoQuerier) {
mockBGPServer.EXPECT().GetBGPRoutes(ctx).Return(allRoutes, nil)
},
expectedStatus: http.StatusOK,
expectedResponse: []apis.BGPRouteResponse{
{
Route: egressIPv4Route.Prefix,
Type: string(allRoutes[egressIPv4Route].Type),
K8sObjRef: allRoutes[egressIPv4Route].K8sObjRef,
},
},
},
}

for _, tt := range tests {
Expand Down
Loading
Loading