client, cmd/snap, daemon: refactor REST API for quotas to match CLI org

The CLI has organization where the current and constraint keys are maps of the
unit of measure for the quota group to the value, so update the REST API to
match this.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

Author: anonymouse64

client/quota.go

@@ -24,29 +24,30 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/snapcore/snapd/gadget/quantity"
 	"golang.org/x/xerrors"
 )
 
 type postQuotaData struct {
-	Action    string   `json:"action"`
-	GroupName string   `json:"group-name"`
-	Parent    string   `json:"parent,omitempty"`
-	Snaps     []string `json:"snaps,omitempty"`
-	MaxMemory uint64   `json:"max-memory,omitempty"`
+	Action      string            `json:"action"`
+	GroupName   string            `json:"group-name"`
+	Parent      string            `json:"parent,omitempty"`
+	Snaps       []string          `json:"snaps,omitempty"`
+	Constraints map[string]string `json:"constraints,omitempty"`
 }
 
 type QuotaGroupResult struct {
-	GroupName     string   `json:"group-name"`
-	Parent        string   `json:"parent,omitempty"`
-	Subgroups     []string `json:"subgroups,omitempty"`
-	Snaps         []string `json:"snaps,omitempty"`
-	MaxMemory     uint64   `json:"max-memory"`
-	CurrentMemory uint64   `json:"current-memory"`
+	GroupName   string            `json:"group-name"`
+	Parent      string            `json:"parent,omitempty"`
+	Subgroups   []string          `json:"subgroups,omitempty"`
+	Snaps       []string          `json:"snaps,omitempty"`
+	Constraints map[string]string `json:"constraints,omitempty"`
+	Current     map[string]string `json:"current,omitempty"`
 }
 
 // EnsureQuota creates a quota group or updates an existing group.
 // The list of snaps can be empty.
-func (client *Client) EnsureQuota(groupName string, parent string, snaps []string, maxMemory uint64) (changeID string, err error) {
+func (client *Client) EnsureQuota(groupName string, parent string, snaps []string, maxMemory quantity.Size) (changeID string, err error) {
 	if groupName == "" {
 		return "", xerrors.Errorf("cannot create or update quota group without a name")
 	}
@@ -57,7 +58,9 @@ func (client *Client) EnsureQuota(groupName string, parent string, snaps []strin
 		GroupName: groupName,
 		Parent:    parent,
 		Snaps:     snaps,
-		MaxMemory: maxMemory,
+		Constraints: map[string]string{
+			"memory": maxMemory.String(),
+		},
 	}
 
 	var body bytes.Buffer

client/quota_test.go

@@ -56,7 +56,9 @@ func (cs *clientSuite) TestEnsureQuotaGroup(c *check.C) {
 		"group-name": "foo",
 		"parent":     "bar",
 		"snaps":      []interface{}{"snap-a", "snap-b"},
-		"max-memory": float64(1001),
+		"constraints": map[string]interface{}{
+			"memory": "1001",
+		},
 	})
 }
 
@@ -76,20 +78,27 @@ func (cs *clientSuite) TestGetQuotaGroup(c *check.C) {
 	cs.rsp = `{
 		"type": "sync",
 		"status-code": 200,
-		"result": {"group-name":"foo", "parent":"bar", "subgroups":["foo-subgrp"], "snaps":["snap-a"], "max-memory":999, "current-memory":450}
+		"result": {
+			"group-name":"foo",
+			"parent":"bar",
+			"subgroups":["foo-subgrp"],
+			"snaps":["snap-a"],
+			"constraints": { "memory": "999" },
+			"current": { "memory": "450" }
+		}
 	}`
 
 	grp, err := cs.cli.GetQuotaGroup("foo")
 	c.Assert(err, check.IsNil)
 	c.Check(cs.req.Method, check.Equals, "GET")
 	c.Check(cs.req.URL.Path, check.Equals, "/v2/quotas/foo")
 	c.Check(grp, check.DeepEquals, &client.QuotaGroupResult{
-		GroupName:     "foo",
-		Parent:        "bar",
-		Subgroups:     []string{"foo-subgrp"},
-		MaxMemory:     999,
-		CurrentMemory: 450,
-		Snaps:         []string{"snap-a"},
+		GroupName:   "foo",
+		Parent:      "bar",
+		Subgroups:   []string{"foo-subgrp"},
+		Constraints: map[string]string{"memory": "999"},
+		Current:     map[string]string{"memory": "450"},
+		Snaps:       []string{"snap-a"},
 	})
 }
 

cmd/snap/cmd_quota.go

@@ -22,11 +22,13 @@ package main
 import (
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/jessevdk/go-flags"
 
 	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/gadget/quantity"
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/strutil"
 )
@@ -162,7 +164,7 @@ func (x *cmdSetQuota) Execute(args []string) (err error) {
 		// orphan a sub-group to no longer have a parent, but currently it just
 		// means leave the group with whatever parent it has, or if it doesn't
 		// currently exist, create the group without a parent group
-		chgID, err = x.client.EnsureQuota(x.Positional.GroupName, x.Parent, names, uint64(mem))
+		chgID, err = x.client.EnsureQuota(x.Positional.GroupName, x.Parent, names, quantity.Size(mem))
 		if err != nil {
 			return err
 		}
@@ -220,10 +222,35 @@ func (x *cmdQuota) Execute(args []string) (err error) {
 	if group.Parent != "" {
 		fmt.Fprintf(w, "parent:\t%s\n", group.Parent)
 	}
+
 	fmt.Fprintf(w, "constraints:\n")
-	fmt.Fprintf(w, "  memory:\t%s\n", strings.TrimSpace(fmtSize(int64(group.MaxMemory))))
+	for constraintKey, val := range group.Constraints {
+		if constraintKey == "memory" {
+			// parse the memory value into a quantity.Size and then format it
+			// with appropriate units
+			var err error
+			val, err = quantitySizeStrToUnitFormat(val, "memory constraint")
+			if err != nil {
+				return err
+			}
+		}
+		fmt.Fprintf(w, "  %s:\t%s\n", constraintKey, val)
+	}
+
 	fmt.Fprintf(w, "current:\n")
-	fmt.Fprintf(w, "  memory:\t%s\n", strings.TrimSpace(fmtSize(int64(group.CurrentMemory))))
+	for currentKey, val := range group.Current {
+		if currentKey == "memory" {
+			// parse the memory value into a quantity.Size and then format it
+			// with appropriate units
+			var err error
+			val, err = quantitySizeStrToUnitFormat(val, "memory current usage")
+			if err != nil {
+				return err
+			}
+		}
+		fmt.Fprintf(w, "  %s:\t%s\n", currentKey, val)
+	}
+
 	if len(group.Subgroups) > 0 {
 		fmt.Fprint(w, "subgroups:\n")
 		for _, name := range group.Subgroups {
@@ -240,6 +267,14 @@ func (x *cmdQuota) Execute(args []string) (err error) {
 	return nil
 }
 
+func quantitySizeStrToUnitFormat(s string, errMsg string) (string, error) {
+	valSize, err := strconv.ParseUint(s, 10, 64)
+	if err != nil {
+		return "", fmt.Errorf("unable to parse %s (%q) as uint: %v", errMsg, s, err)
+	}
+	return strings.TrimSpace(fmtSize(int64(valSize))), nil
+}
+
 type cmdRemoveQuota struct {
 	waitMixin
 
@@ -281,17 +316,39 @@ func (x *cmdQuotas) Execute(args []string) (err error) {
 	w := tabWriter()
 	fmt.Fprintf(w, "Quota\tParent\tConstraints\tCurrent\n")
 	err = processQuotaGroupsTree(res, func(q *client.QuotaGroupResult) {
-		constraintMem := ""
-		if q.MaxMemory != 0 {
-			constraintMem = "memory=" + strings.TrimSpace(fmtSize(int64(q.MaxMemory)))
+		constraintVals := []string{}
+		for constraintKey, val := range q.Constraints {
+			if constraintKey == "memory" {
+				out, err := quantitySizeStrToUnitFormat(val, "memory constraint")
+				if err != nil {
+					fmt.Fprintln(Stderr, err)
+				}
+				val = out
+			}
+
+			constraintVals = append(constraintVals, fmt.Sprintf("%s=%s", constraintKey, val))
 		}
+		constraints := strings.Join(constraintVals, ",")
+
+		currentVals := []string{}
+		for currentKey, val := range q.Current {
+			if currentKey == "memory" {
+				// skip reporting memory if it is exactly zero
+				if val == "" || val == "0" {
+					continue
+				}
+				out, err := quantitySizeStrToUnitFormat(val, "current memory usage")
+				if err != nil {
+					fmt.Fprintln(Stderr, err)
+				}
+				val = out
+			}
 
-		currentMem := ""
-		if q.CurrentMemory != 0 {
-			currentMem = "memory=" + strings.TrimSpace(fmtSize(int64(q.CurrentMemory)))
+			currentVals = append(currentVals, fmt.Sprintf("%s=%s", currentKey, val))
 		}
+		currents := strings.Join(currentVals, ",")
 
-		fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", q.GroupName, q.Parent, constraintMem, currentMem)
+		fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", q.GroupName, q.Parent, constraints, currents)
 	})
 	if err != nil {
 		return err

cmd/snap/cmd_quota_test.go

@@ -92,21 +92,20 @@ func dispatchFakeHandlers(c *check.C, routes map[string]http.HandlerFunc) func(w
 }
 
 type fakeQuotaGroupPostHandlerOpts struct {
-	action        string
-	body          string
-	groupName     string
-	parentName    string
-	snaps         []string
-	maxMemory     int64
-	currentMemory int64
+	action     string
+	body       string
+	groupName  string
+	parentName string
+	snaps      []string
+	maxMemory  int64
 }
 
 type quotasEnsureBody struct {
-	Action     string   `json:"action"`
-	GroupName  string   `json:"group-name,omitempty"`
-	ParentName string   `json:"parent,omitempty"`
-	Snaps      []string `json:"snaps,omitempty"`
-	MaxMemory  int64    `json:"max-memory,omitempty"`
+	Action      string            `json:"action"`
+	GroupName   string            `json:"group-name,omitempty"`
+	ParentName  string            `json:"parent,omitempty"`
+	Snaps       []string          `json:"snaps,omitempty"`
+	Constraints map[string]string `json:"constraints,omitempty"`
 }
 
 func makeFakeQuotaPostHandler(c *check.C, opts fakeQuotaGroupPostHandlerOpts) func(w http.ResponseWriter, r *http.Request) {
@@ -127,11 +126,11 @@ func makeFakeQuotaPostHandler(c *check.C, opts fakeQuotaGroupPostHandlerOpts) fu
 			c.Check(string(buf), check.Equals, fmt.Sprintf(`{"action":"remove","group-name":%q}`+"\n", opts.groupName))
 		case "ensure":
 			exp := quotasEnsureBody{
-				Action:     "ensure",
-				GroupName:  opts.groupName,
-				ParentName: opts.parentName,
-				Snaps:      opts.snaps,
-				MaxMemory:  opts.maxMemory,
+				Action:      "ensure",
+				GroupName:   opts.groupName,
+				ParentName:  opts.parentName,
+				Snaps:       opts.snaps,
+				Constraints: map[string]string{"memory": fmt.Sprintf("%d", opts.maxMemory)},
 			}
 
 			postJSON := quotasEnsureBody{}
@@ -197,8 +196,8 @@ func (s *quotaSuite) TestGetQuotaGroup(c *check.C) {
 			"parent":"bar",
 			"subgroups":["subgrp1"],
 			"snaps":["snap-a","snap-b"],
-			"max-memory":1000,
-			"current-memory":900
+			"constraints": { "memory": "1000" },
+			"current": { "memory": "900" }
 		}
 	}`
 
@@ -232,8 +231,8 @@ func (s *quotaSuite) TestGetQuotaGroupSimple(c *check.C) {
 		"status-code": 200,
 		"result": {
 			"group-name": "foo",
-			"max-memory": 1000,
-			"current-memory": %d
+			"constraints": {"memory": "1000"},
+			"current": {"memory": "%d"}
 		}
 	}`
 
@@ -354,8 +353,8 @@ func (s *quotaSuite) TestSetQuotaGroupUpdateExisting(c *check.C) {
 		"status-code": 200,
 		"result": {
 			"group-name":"foo",
-			"max-memory": %d,
-			"current-memory":500
+			"constraints": { "memory": "%d" },
+			"current": { "memory": "500" }
 		}
 	}`
 
@@ -437,13 +436,14 @@ func (s *quotaSuite) TestGetAllQuotaGroups(c *check.C) {
 
 	s.RedirectClientToTestServer(makeFakeGetQuotaGroupsHandler(c,
 		`{"type": "sync", "status-code": 200, "result": [
-			{"group-name":"aaa","subgroups":["ccc","ddd"],"parent":"zzz","max-memory":1000},
-			{"group-name":"ddd","parent":"aaa","max-memory":400},
-			{"group-name":"bbb","parent":"zzz","max-memory":1000,"current-memory":400},
-			{"group-name":"yyyyyyy","max-memory":1000},
-			{"group-name":"zzz","subgroups":["bbb","aaa"],"max-memory":5000},
-			{"group-name":"ccc","parent":"aaa","max-memory":400},
-			{"group-name":"xxx","max-memory":9900,"current-memory":9999}
+			{"group-name":"aaa","subgroups":["ccc","ddd","fff"],"parent":"zzz","constraints":{"memory":"1000"}},
+			{"group-name":"ddd","parent":"aaa","constraints":{"memory":"400"}},
+			{"group-name":"bbb","parent":"zzz","constraints":{"memory":"1000"},"current":{"memory":"400"}},
+			{"group-name":"yyyyyyy","constraints":{"memory":"1000"}},
+			{"group-name":"zzz","subgroups":["bbb","aaa"],"constraints":{"memory":"5000"}},
+			{"group-name":"ccc","parent":"aaa","constraints":{"memory":"400"}},
+			{"group-name":"fff","parent":"aaa","constraints":{"memory":"1000"},"current":{"memory":"0"}},
+			{"group-name":"xxx","constraints":{"memory":"9900"},"current":{"memory":"10000"}}
 			]}`))
 
 	rest, err := main.Parser(main.Client()).ParseArgs([]string{"quotas"})
@@ -458,6 +458,7 @@ zzz              memory=5000B
 aaa      zzz     memory=1000B  
 ccc      aaa     memory=400B   
 ddd      aaa     memory=400B   
+fff      aaa     memory=1000B  
 bbb      zzz     memory=1000B  memory=400B
 `[1:])
 }