cmd/snap/quota: refactor quota CLI as per new design #10333
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ package main | |
| import ( | ||
| "fmt" | ||
| "sort" | ||
| "strings" | ||
|
|
||
| "github.com/jessevdk/go-flags" | ||
|
|
||
|
|
@@ -30,13 +31,11 @@ import ( | |
| "github.com/snapcore/snapd/strutil" | ||
| ) | ||
|
|
||
| var shortQuotaHelp = i18n.G("Show quota group for a set of snaps") | ||
| var longQuotaHelp = i18n.G(` | ||
| The quota command shows information about a quota group, including the set of | ||
| snaps and sub-groups that are in a group, as well as the resource constraints | ||
| and current usage of the resource constraints. | ||
| `) | ||
|
|
||
| var shortQuotasHelp = i18n.G("Show quota groups") | ||
|
|
@@ -45,24 +44,47 @@ The quotas command shows all quota groups. | |
| `) | ||
|
|
||
| var shortRemoveQuotaHelp = i18n.G("Remove quota group") | ||
| var longRemoveQuotaHelp = i18n.G(` | ||
| The remove-quota command removes the given quota group. | ||
|
|
||
| Currently, only quota groups with no sub-groups can be removed. In order to | ||
| remove a quota group with sub-groups, the sub-groups must first be removed until | ||
| there are no sub-groups for the group, then the group itself can be removed. | ||
| `) | ||
|
|
||
| var shortSetQuotaHelp = i18n.G(`Create or update a quota group.`) | ||
| var longSetQuotaHelp = i18n.G(` | ||
| The set-quota command updates or creates a quota group with the specified set of | ||
| snaps. | ||
|
|
||
| A quota group sets resource limits (currently maximum memory only) on the set of | ||
| snaps that belong to it. Snaps can be at most in one quota group. Quota groups | ||
| can be nested. | ||
|
|
||
| All snaps provided are appended to the group; to remove a snap from a | ||
| quota group the entire group must be removed with remove-quota and recreated | ||
| without the quota group. To remove a sub-group from the quota group, the | ||
|
There was a problem hiding this comment. "without the quota group" -> "without the undesired snap" Unless, of course, I misunderstood something :-) There was a problem hiding this comment. Yes good catch this was a typo. I will adjust the wording but not in this PR since it is finally green and I would love to land it :-) Also this reminds me when @degville is back it would be great to have him look at the help texts here as well. |
||
| sub-group must be removed directly with the remove-quota command. | ||
|
|
||
| The memory limit for a quota group can be increased, but cannot be decreased. To | ||
| decrease the memory limit for a quota group, the entire group must be removed | ||
| with the remove-quota command and recreated with the lower limit. Increasing the | ||
| memory limit for a quota group does not restart any services associated with | ||
| snaps in the quota group. | ||
|
|
||
| Adding new snaps to a quota group will result in all non-disabled services in | ||
| that snap being restarted. | ||
|
|
||
| One cannot modify the parent of an existing sub-quota group, nor can an existing | ||
| sub-quota group be moved from one parent to another. | ||
| `) | ||
|
|
||
| func init() { | ||
| // TODO: unhide the commands when non-experimental | ||
| cmd := addCommand("set-quota", shortSetQuotaHelp, longSetQuotaHelp, func() flags.Commander { return &cmdSetQuota{} }, nil, nil) | ||
| cmd.hidden = true | ||
|
|
||
| cmd = addCommand("quota", shortQuotaHelp, longQuotaHelp, func() flags.Commander { return &cmdQuota{} }, nil, nil) | ||
| cmd.hidden = true | ||
|
|
||
| cmd = addCommand("quotas", shortQuotasHelp, longQuotasHelp, func() flags.Commander { return &cmdQuotas{} }, nil, nil) | ||
|
|
@@ -72,57 +94,127 @@ func init() { | |
| cmd.hidden = true | ||
| } | ||
|
|
||
| type cmdSetQuota struct { | ||
| clientMixin | ||
|
|
||
| MemoryMax string `long:"memory" optional:"true"` | ||
| Parent string `long:"parent" optional:"true"` | ||
| Positional struct { | ||
| GroupName string `positional-arg-name:"<group-name>" required:"true"` | ||
| Snaps []installedSnapName `positional-arg-name:"<snap>" optional:"true"` | ||
| } `positional-args:"yes"` | ||
| } | ||
|
|
||
| func (x *cmdSetQuota) Execute(args []string) (err error) { | ||
| var maxMemory string | ||
| switch { | ||
| case x.MemoryMax != "": | ||
| maxMemory = x.MemoryMax | ||
| } | ||
|
|
||
| names := installedSnapNames(x.Positional.Snaps) | ||
|
|
||
| // figure out if the group exists or not to make error messages more useful | ||
| groupExists := false | ||
| if _, err = x.client.GetQuotaGroup(x.Positional.GroupName); err == nil { | ||
| groupExists = true | ||
| } | ||
|
|
||
| switch { | ||
| case maxMemory == "" && x.Parent == "" && len(x.Positional.Snaps) == 0: | ||
| // no snaps were specified, no memory limit was specified, and no parent | ||
| // was specified, so just the group name was provided - this is not | ||
| // supported since there is nothing to change/create | ||
|
|
||
| if groupExists { | ||
| return fmt.Errorf("no options set to change quota group") | ||
| } | ||
| return fmt.Errorf("cannot create quota group without memory limit") | ||
|
|
||
| case maxMemory == "" && x.Parent != "" && len(x.Positional.Snaps) == 0: | ||
| // this is either trying to create a new group with a parent and forgot | ||
| // to specify the memory limit for the new group, or the user is trying | ||
| // to re-parent a group, i.e. move it from the current parent to a | ||
| // different one, which is currently unsupported | ||
|
|
||
| if groupExists { | ||
| // TODO: or this could be setting the parent to the existing parent, | ||
| // which is effectively no change or update but maybe we allow since | ||
| // it's a noop? | ||
| return fmt.Errorf("cannot move a quota group to a new parent") | ||
| } | ||
| return fmt.Errorf("cannot create quota group without memory limit") | ||
|
|
||
| case maxMemory != "": | ||
| // we have a memory limit to set for this group, so specify that along | ||
| // with whatever snaps may have been provided and whatever parent may | ||
| // have been specified | ||
|
|
||
| mem, err := strutil.ParseByteSize(maxMemory) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // note that the group could currently exist with a parent, and we could | ||
| // be specifying x.Parent as "" here - in the future that may mean to | ||
| // 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 | ||
| return x.client.EnsureQuota(x.Positional.GroupName, x.Parent, names, uint64(mem)) | ||
|
|
||
| case len(x.Positional.Snaps) != 0: | ||
| // there are snaps specified for this group but no memory limit, so the | ||
| // group must already exist and we must be adding the specified snaps to | ||
| // the group | ||
|
|
||
| // TODO: this case may someday also imply overwriting the current set of | ||
| // snaps with whatever was specified with some option, but we don't | ||
| // currently support that, so currently all snaps specified here are | ||
| // just added to the group | ||
|
|
||
| return x.client.EnsureQuota(x.Positional.GroupName, x.Parent, names, 0) | ||
|
|
||
| default: | ||
| // should be logically impossible to reach here | ||
| panic("impossible set of options") | ||
| } | ||
| } | ||
|
|
||
| type cmdQuota struct { | ||
| clientMixin | ||
|
|
||
| Positional struct { | ||
| GroupName string `positional-arg-name:"<group-name>" required:"true"` | ||
| } `positional-args:"yes"` | ||
| } | ||
|
|
||
| func (x *cmdQuota) Execute(args []string) (err error) { | ||
| if len(args) != 0 { | ||
| return fmt.Errorf("too many arguments provided") | ||
| } | ||
|
|
||
| group, err := x.client.GetQuotaGroup(x.Positional.GroupName) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| w := tabWriter() | ||
| defer w.Flush() | ||
|
|
||
| fmt.Fprintf(w, "name:\t%s\n", group.GroupName) | ||
| 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)))) | ||
| fmt.Fprintf(w, "current:\n") | ||
| fmt.Fprintf(w, " memory:\t%s\n", strings.TrimSpace(fmtSize(int64(group.CurrentMemory)))) | ||
| if len(group.Subgroups) > 0 { | ||
| fmt.Fprint(w, "subgroups:\n") | ||
| for _, name := range group.Subgroups { | ||
| fmt.Fprintf(w, " - %s\n", name) | ||
| } | ||
| } | ||
| if len(group.Snaps) > 0 { | ||
| fmt.Fprint(w, "snaps:\n") | ||
| for _, snapName := range group.Snaps { | ||
|
|
@@ -159,9 +251,19 @@ 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))) | ||
| } | ||
|
|
||
| currentMem := "" | ||
| if q.CurrentMemory != 0 { | ||
| currentMem = "memory=" + strings.TrimSpace(fmtSize(int64(q.CurrentMemory))) | ||
| } | ||
|
|
||
| fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", q.GroupName, q.Parent, constraintMem, currentMem) | ||
| }) | ||
| if err != nil { | ||
| return err | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this very detailed description!