Add agent API and antctl command to dump OVS groups

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.

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

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)
 		}

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,
 				},

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.

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
 }

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)
 		})