diff --git a/docs/antctl.md b/docs/antctl.md index 027c3bd8591..354cd08b32f 100644 --- a/docs/antctl.md +++ b/docs/antctl.md @@ -237,7 +237,7 @@ Starting from version 0.6.0, Antrea Agent supports dumping Antrea OVS flows. The `antctl` `get ovsflows` (or `get of`) command can dump all OVS flows, flows added for a specified Pod, or flows added for Service load-balancing of a specified Service, or flows added to realize a specified NetworkPolicy, or flows -in a specified OVS flow table. +in the specified OVS flow tables, or all or the specified OVS groups. ```bash antctl get ovsflows @@ -246,7 +246,8 @@ antctl get ovsflows -S SERVICE -n NAMESPACE antctl get ovsflows -N NETWORKPOLICY -n NAMESPACE antctl get ovsflows -T TABLE_A,TABLE_B antctl get ovsflows -T TABLE_A,TABLE_B_NUM -antctl get ovsflows -T TABLE_A_NUM,TABLE_B_NUM +antctl get ovsflows -G all +antctl get ovsflows -G GROUP_ID1,GROUP_ID2 ``` OVS flow tables can be specified using table names, or the table numbers. diff --git a/pkg/agent/apiserver/handlers/ovsflows/handler.go b/pkg/agent/apiserver/handlers/ovsflows/handler.go index c8ff3540e69..6332e267c39 100644 --- a/pkg/agent/apiserver/handlers/ovsflows/handler.go +++ b/pkg/agent/apiserver/handlers/ovsflows/handler.go @@ -71,7 +71,7 @@ func dumpFlows(aq agentquerier.AgentQuerier, table binding.TableIDType) ([]Respo func dumpMatchedGroups(aq agentquerier.AgentQuerier, groupIDs []binding.GroupIDType) ([]Response, error) { resps := []Response{} for _, g := range groupIDs { - groupStr, err := aq.GetOVSCtlClient().DumpGroup(int(g)) + groupStr, err := aq.GetOVSCtlClient().DumpGroup(uint32(g)) if err != nil { klog.Errorf("Failed to dump group %d: %v", g, err) return nil, err @@ -87,7 +87,7 @@ func dumpMatchedGroups(aq agentquerier.AgentQuerier, groupIDs []binding.GroupIDT // number is invalid). func getTableFlows(aq agentquerier.AgentQuerier, table string) ([]Response, error) { var resps []Response - for _, tableSeg := range strings.Split(strings.TrimSpace(table), ",") { + for _, tableSeg := range strings.Split(table, ",") { tableSeg = strings.TrimSpace(tableSeg) var tableNumber binding.TableIDType // Table nubmer is a 8-bit unsigned integer. @@ -112,6 +112,36 @@ func getTableFlows(aq agentquerier.AgentQuerier, table string) ([]Response, erro return resps, nil } +// nil is returned if the passed group IDs are invalid. +func getGroups(aq agentquerier.AgentQuerier, groups string) ([]Response, error) { + if strings.EqualFold(groups, "all") { + groupStrs, err := aq.GetOVSCtlClient().DumpGroups() + if err != nil { + return nil, err + } + resps := make([]Response, 0, len(groupStrs)) + for _, s := range groupStrs { + resps = append(resps, Response{s}) + } + return resps, nil + } + + var groupIDs []binding.GroupIDType + for _, id := range strings.Split(groups, ",") { + id = strings.TrimSpace(id) + // Group ID is a 32-bit unsigned integer. + n, err := strconv.ParseUint(id, 10, 32) + if err != nil { + return nil, nil + } + groupIDs = append(groupIDs, binding.GroupIDType(n)) + } + if groupIDs == nil { + return nil, nil + } + return dumpMatchedGroups(aq, groupIDs) +} + func getPodFlows(aq agentquerier.AgentQuerier, podName, namespace string) ([]Response, error) { interfaces := aq.GetInterfaceStore().GetContainerInterfacesByPod(podName, namespace) if len(interfaces) == 0 { @@ -159,13 +189,14 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc { networkPolicy := r.URL.Query().Get("networkpolicy") namespace := r.URL.Query().Get("namespace") table := r.URL.Query().Get("table") + groups := r.URL.Query().Get("groups") if (pod != "" || service != "" || networkPolicy != "") && namespace == "" { http.Error(w, "namespace must be provided", http.StatusBadRequest) return } - if pod == "" && service == "" && networkPolicy == "" && namespace == "" && table == "" { + if pod == "" && service == "" && networkPolicy == "" && namespace == "" && table == "" && groups == "" { resps, err = dumpFlows(aq, binding.TableIDAll) } else if pod != "" { // Pod Namespace must be provided to dump flows of a Pod. @@ -184,6 +215,12 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc { http.Error(w, "invalid table name or number", http.StatusBadRequest) return } + } else if groups != "" { + resps, err = getGroups(aq, groups) + if err == nil && resps == nil { + http.Error(w, "invalid group ID", http.StatusBadRequest) + return + } } else { http.Error(w, "unsupported parameter combination", http.StatusBadRequest) return diff --git a/pkg/agent/apiserver/handlers/ovsflows/handler_test.go b/pkg/agent/apiserver/handlers/ovsflows/handler_test.go index 3128f559666..e9c5f5b52be 100644 --- a/pkg/agent/apiserver/handlers/ovsflows/handler_test.go +++ b/pkg/agent/apiserver/handlers/ovsflows/handler_test.go @@ -51,7 +51,7 @@ type testCase struct { namespace string query string expectedStatus int - dumpGroups bool + resps []Response } func TestBadRequests(t *testing.T) { @@ -61,12 +61,15 @@ func TestBadRequests(t *testing.T) { "NetworkPolicy only": "?networkpolicy=np1", "Namespace only": "?namespace=ns1", "Pod and NetworkPolicy": "?pod=pod1&&networkpolicy=np1", - "Pod and Table": "?pod=pod1&&table=0", + "Pod and table": "?pod=pod1&&table=0", "Non-existing table number": "?table=123", "Non-existing table name": "?table=notexist", "Too big table number": "?table=256", "Invalid table number": "?table=0classification", "Invalid table name": "?table=classification0", + "Invalid group IDs": "?groups=all,0", + "Too big group ID": "?groups=123,4294967296", + "Negative group ID": "?groups=-1", } handler := HandleFunc(nil) @@ -135,7 +138,7 @@ func TestServiceFlows(t *testing.T) { namespace: "ns1", query: "?service=svc1&&namespace=ns1", expectedStatus: http.StatusOK, - dumpGroups: true, + resps: append(testResponses, testGroupResponses...), }, { test: "Non-existing Service", @@ -159,7 +162,7 @@ func TestServiceFlows(t *testing.T) { ovsctl.EXPECT().DumpMatchedFlow(f).Return(testDumpFlows[i], nil).Times(1) } for i, g := range testGroupIDs { - ovsctl.EXPECT().DumpGroup(int(g)).Return(testDumpGroups[i], nil).Times(1) + ovsctl.EXPECT().DumpGroup(uint32(g)).Return(testDumpGroups[i], nil).Times(1) } } else { p.EXPECT().GetServiceFlowKeys(tc.name, tc.namespace).Return(nil, nil, false).Times(1) @@ -243,6 +246,74 @@ func TestTableFlows(t *testing.T) { } +func TestGroups(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + testcases := []struct { + testCase + groupIDs []uint32 + dumpedGroups []string + }{ + { + testCase: testCase{ + test: "All groups", + query: "?groups=all", + expectedStatus: http.StatusOK, + resps: testGroupResponses, + }, + dumpedGroups: testDumpGroups, + }, + { + testCase: testCase{ + test: "Group 1234", + query: "?groups=1234", + expectedStatus: http.StatusOK, + resps: []Response{{"group1234"}}, + }, + groupIDs: []uint32{1234}, + dumpedGroups: []string{"group1234"}, + }, + { + testCase: testCase{ + test: "Non-existing group 1234", + query: "?groups=1234", + expectedStatus: http.StatusOK, + resps: []Response{}, + }, + groupIDs: []uint32{1234}, + dumpedGroups: []string{""}, + }, + { + testCase: testCase{ + test: "Group 10, 100, and 1000", + query: "?groups=10,100,1000", + expectedStatus: http.StatusOK, + resps: []Response{{"group10"}, {"group1000"}}, + }, + groupIDs: []uint32{10, 100, 1000}, + dumpedGroups: []string{"group10", "", "group1000"}, + }, + } + for _, tc := range testcases { + ovsctl := ovsctltest.NewMockOVSCtlClient(ctrl) + q := aqtest.NewMockAgentQuerier(ctrl) + if tc.groupIDs == nil { + // Get all. + q.EXPECT().GetOVSCtlClient().Return(ovsctl).Times(1) + ovsctl.EXPECT().DumpGroups().Return(tc.dumpedGroups, nil).Times(1) + } else { + // Get all. + q.EXPECT().GetOVSCtlClient().Return(ovsctl).Times(len(tc.groupIDs)) + for i, id := range tc.groupIDs { + ovsctl.EXPECT().DumpGroup(id).Return(tc.dumpedGroups[i], nil).Times(1) + } + } + + runHTTPTest(t, &tc.testCase, q) + } +} + func runHTTPTest(t *testing.T, tc *testCase, aq agentquerier.AgentQuerier) { handler := HandleFunc(aq) req, err := http.NewRequest(http.MethodGet, tc.query, nil) @@ -256,8 +327,8 @@ func runHTTPTest(t *testing.T, tc *testCase, aq agentquerier.AgentQuerier) { var received []Response err = json.Unmarshal(recorder.Body.Bytes(), &received) assert.Nil(t, err) - if tc.dumpGroups { - assert.Equal(t, append(testResponses, testGroupResponses...), received) + if tc.resps != nil { + assert.Equal(t, tc.resps, received) } else { assert.Equal(t, testResponses, received) } diff --git a/pkg/antctl/antctl.go b/pkg/antctl/antctl.go index 0938a3cd72c..f4d100652e1 100644 --- a/pkg/antctl/antctl.go +++ b/pkg/antctl/antctl.go @@ -299,6 +299,10 @@ var CommandList = &commandList{ $ antctl get ovsflows -N np1 -n ns1 Dump OVS flows of a flow Table $ antctl get ovsflows -T IngressRule + Dump OVS groups + $ antctl get ovsflows -G 10,20 + Dump all OVS groups + $ antctl get ovsflows -G all Antrea OVS Flow Tables:` + generateFlowTableHelpMsg(), agentEndpoint: &endpoint{ @@ -330,6 +334,11 @@ var CommandList = &commandList{ usage: "Comma separated Antrea OVS flow table names or numbers", shorthand: "T", }, + { + name: "groups", + usage: "Comma separated OVS group IDs. Use 'all' for dumping all groups", + shorthand: "G", + }, }, outputType: multiple, }, diff --git a/pkg/ovs/ovsctl/interface.go b/pkg/ovs/ovsctl/interface.go index 4226517a640..590624f81c0 100644 --- a/pkg/ovs/ovsctl/interface.go +++ b/pkg/ovs/ovsctl/interface.go @@ -40,9 +40,9 @@ type OVSCtlClient interface { // DumpTableFlows returns all flows in the table. DumpTableFlows(table uint8) ([]string, error) // DumpGroup returns the OpenFlow group if it exists on the bridge. - DumpGroup(groupID int) (string, error) + DumpGroup(groupID uint32) (string, error) // DumpGroups returns OpenFlow groups of the bridge. - DumpGroups(args ...string) ([][]string, error) + DumpGroups() ([]string, error) // DumpPortsDesc returns OpenFlow ports descriptions of the bridge. DumpPortsDesc() ([][]string, error) // RunOfctlCmd executes "ovs-ofctl" command and returns the outputs. diff --git a/pkg/ovs/ovsctl/ofctl.go b/pkg/ovs/ovsctl/ofctl.go index 4ead6264b4e..d7f01c14467 100644 --- a/pkg/ovs/ovsctl/ofctl.go +++ b/pkg/ovs/ovsctl/ofctl.go @@ -36,7 +36,6 @@ func (c *ovsCtlClient) DumpFlows(args ...string) ([]string, error) { flowList = append(flowList, trimFlowStr(scanner.Text())) } return flowList, nil - } func (c *ovsCtlClient) DumpMatchedFlow(matchStr string) (string, error) { @@ -64,11 +63,11 @@ func (c *ovsCtlClient) DumpTableFlows(table uint8) ([]string, error) { return c.DumpFlows(fmt.Sprintf("table=%d", table)) } -func (c *ovsCtlClient) DumpGroup(groupID int) (string, error) { +func (c *ovsCtlClient) DumpGroup(groupID uint32) (string, error) { // There seems a bug in ovs-ofctl that dump-groups always returns all // the groups when using Openflow13, even when the group ID is provided. // As a workaround, we do not specify Openflow13 to run the command. - groupDump, err := c.runOfctlCmd(false, "dump-groups", strconv.Itoa(groupID)) + groupDump, err := c.runOfctlCmd(false, "dump-groups", strconv.FormatUint(uint64(groupID), 10)) if err != nil { return "", err } @@ -85,27 +84,18 @@ func (c *ovsCtlClient) DumpGroup(groupID int) (string, error) { return strings.TrimSpace(scanner.Text()), nil } -func (c *ovsCtlClient) DumpGroups(args ...string) ([][]string, error) { - groupsDump, err := c.RunOfctlCmd("dump-groups", args...) +func (c *ovsCtlClient) DumpGroups() ([]string, error) { + groupsDump, err := c.RunOfctlCmd("dump-groups") if err != nil { return nil, err } - groupsDumpStr := strings.TrimSpace(string(groupsDump)) - - scanner := bufio.NewScanner(strings.NewReader(groupsDumpStr)) + scanner := bufio.NewScanner(strings.NewReader(string(groupsDump))) scanner.Split(bufio.ScanLines) // Skip the first line. scanner.Scan() - rawGroupItems := []string{} + groupList := []string{} for scanner.Scan() { - rawGroupItems = append(rawGroupItems, scanner.Text()) - } - - var groupList [][]string - for _, rawGroupItem := range rawGroupItems { - rawGroupItem = strings.TrimSpace(rawGroupItem) - elems := strings.Split(rawGroupItem, ",bucket=") - groupList = append(groupList, elems) + groupList = append(groupList, strings.TrimSpace(scanner.Text())) } return groupList, nil } diff --git a/pkg/ovs/ovsctl/testing/mock_ovsctl.go b/pkg/ovs/ovsctl/testing/mock_ovsctl.go index f0f88a34a71..ece557fac9d 100644 --- a/pkg/ovs/ovsctl/testing/mock_ovsctl.go +++ b/pkg/ovs/ovsctl/testing/mock_ovsctl.go @@ -68,7 +68,7 @@ func (mr *MockOVSCtlClientMockRecorder) DumpFlows(arg0 ...interface{}) *gomock.C } // DumpGroup mocks base method -func (m *MockOVSCtlClient) DumpGroup(arg0 int) (string, error) { +func (m *MockOVSCtlClient) DumpGroup(arg0 uint32) (string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DumpGroup", arg0) ret0, _ := ret[0].(string) @@ -83,22 +83,18 @@ func (mr *MockOVSCtlClientMockRecorder) DumpGroup(arg0 interface{}) *gomock.Call } // DumpGroups mocks base method -func (m *MockOVSCtlClient) DumpGroups(arg0 ...string) ([][]string, error) { +func (m *MockOVSCtlClient) DumpGroups() ([]string, error) { m.ctrl.T.Helper() - varargs := []interface{}{} - for _, a := range arg0 { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "DumpGroups", varargs...) - ret0, _ := ret[0].([][]string) + ret := m.ctrl.Call(m, "DumpGroups") + ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } // DumpGroups indicates an expected call of DumpGroups -func (mr *MockOVSCtlClientMockRecorder) DumpGroups(arg0 ...interface{}) *gomock.Call { +func (mr *MockOVSCtlClientMockRecorder) DumpGroups() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DumpGroups", reflect.TypeOf((*MockOVSCtlClient)(nil).DumpGroups), arg0...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DumpGroups", reflect.TypeOf((*MockOVSCtlClient)(nil).DumpGroups)) } // DumpMatchedFlow mocks base method diff --git a/test/integration/ovs/ofctrl_test.go b/test/integration/ovs/ofctrl_test.go index 4ab743d0b3d..39a74702eb1 100644 --- a/test/integration/ovs/ofctrl_test.go +++ b/test/integration/ovs/ofctrl_test.go @@ -289,7 +289,7 @@ func TestOFctrlGroup(t *testing.T) { } // Check if the group could be added. require.Nil(t, group.Add()) - groups, err := ovsCtlClient.DumpGroups() + groups, err := OfCtlDumpGroups(ovsCtlClient) require.Nil(t, err) require.Len(t, groups, 1) dumpedGroup := groups[0] @@ -311,7 +311,7 @@ func TestOFctrlGroup(t *testing.T) { } // Check if the group could be deleted. require.Nil(t, group.Delete()) - groups, err = ovsCtlClient.DumpGroups() + groups, err = OfCtlDumpGroups(ovsCtlClient) require.Nil(t, err) require.Len(t, groups, 0) }) diff --git a/test/integration/ovs/openflow_test_utils.go b/test/integration/ovs/openflow_test_utils.go index 8fcbbcc22f1..4aba42f08ce 100644 --- a/test/integration/ovs/openflow_test_utils.go +++ b/test/integration/ovs/openflow_test_utils.go @@ -71,7 +71,7 @@ func CheckFlowExists(t *testing.T, ovsCtlClient ovsctl.OVSCtlClient, tableID uin func CheckGroupExists(t *testing.T, ovsCtlClient ovsctl.OVSCtlClient, groupID binding.GroupIDType, groupType string, buckets []string, expectExists bool) { // dump groups - groupList, err := ovsCtlClient.DumpGroups() + groupList, err := OfCtlDumpGroups(ovsCtlClient) if err != nil { t.Errorf("Error dumping flows: Err %v", err) } @@ -140,3 +140,17 @@ func OfctlDeleteFlows(ovsCtlClient ovsctl.OVSCtlClient) error { _, err := ovsCtlClient.RunOfctlCmd("del-flows") return err } + +func OfCtlDumpGroups(ovsCtlClient ovsctl.OVSCtlClient) ([][]string, error) { + rawGroupItems, err := ovsCtlClient.DumpGroups() + if err != nil { + return nil, err + } + + var groupList [][]string + for _, item := range rawGroupItems { + elems := strings.Split(item, ",bucket=") + groupList = append(groupList, elems) + } + return groupList, nil +}