From c9bbddcfcbbcad64dd9453afec77ba614598fc67 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Mon, 18 Mar 2019 16:01:19 -0400 Subject: [PATCH] refactor: drop global resource limits Global limits make it harder to reason about the inheritance in the system. Rather than support global limits, Dokku should provide primitives to automatically set them on an app at provision time, or explicitly set them for an app. Both of these are likely best suited for a higher-level provisioner, or a community plugin. --- docs/advanced-usage/resource-management.md | 10 +-- plugins/resource/src/commands/commands.go | 7 +- .../subcommands/limit-clear/limit-clear.go | 3 +- .../resource/src/subcommands/limit/limit.go | 3 +- .../reserve-clear/reserve-clear.go | 3 +- .../src/subcommands/reserve/reserve.go | 3 +- plugins/resource/subcommands.go | 80 +++++++++---------- 7 files changed, 52 insertions(+), 57 deletions(-) diff --git a/docs/advanced-usage/resource-management.md b/docs/advanced-usage/resource-management.md index 7725cefec..3ac2ef5f7 100644 --- a/docs/advanced-usage/resource-management.md +++ b/docs/advanced-usage/resource-management.md @@ -3,11 +3,11 @@ > New as of 0.15.0 ``` -dokku resource:limit [--global] [--process-type ] [RESOURCE_OPTS...] -dokku resource:reserve [--global] [--process-type ] [RESOURCE_OPTS...] -dokku resource:limit-clear [--process-type ] [] -dokku resource:reserve-clear [--process-type ] [] -dokku resource:report [] +resource:limit [--process-type ] [RESOURCE_OPTS...] # Limit resources for a given app/process-type combination +resource:limit-clear [--process-type ] # Limit resources for a given app/process-type combination +resource:report [] [] # Displays a resource report for one or more apps +resource:reserve [--process-type ] [RESOURCE_OPTS...] # Reserve resources for a given app/process-type combination +resource:reserve-clear [--process-type ] # Reserve resources for a given app/process-type combination ``` The resource plugin is meant to allow users to limit or reserve resources for a given app/process-type combination. diff --git a/plugins/resource/src/commands/commands.go b/plugins/resource/src/commands/commands.go index 5e13ed013..a998d631d 100644 --- a/plugins/resource/src/commands/commands.go +++ b/plugins/resource/src/commands/commands.go @@ -19,8 +19,11 @@ Manages resource settings for an app Additional commands:` helpContent = ` - resource:limit [--global] [--process-type process-type] [RESOURCE_OPTS...] , Limit resources for a given app/process-type combination - resource:reserve [--global] [--process-type process-type] [RESOURCE_OPTS...] , Reserve resources for a given app/process-type combination + resource:limit [--process-type ] [RESOURCE_OPTS...] , Limit resources for a given app/process-type combination + resource:limit-clear [--process-type ] , Limit resources for a given app/process-type combination + resource:report [] [], Displays a resource report for one or more apps + resource:reserve [--process-type ] [RESOURCE_OPTS...] , Reserve resources for a given app/process-type combination + resource:reserve-clear [--process-type ] , Reserve resources for a given app/process-type combination ` ) diff --git a/plugins/resource/src/subcommands/limit-clear/limit-clear.go b/plugins/resource/src/subcommands/limit-clear/limit-clear.go index 32000d6d6..b794dfc30 100644 --- a/plugins/resource/src/subcommands/limit-clear/limit-clear.go +++ b/plugins/resource/src/subcommands/limit-clear/limit-clear.go @@ -11,10 +11,9 @@ import ( func main() { args := flag.NewFlagSet("resource:limit-clear", flag.ExitOnError) processType := args.String("process-type", "", "process-type: A process type to clear") - global := args.Bool("global", false, "global: Whether to clear global settings or not") args.Parse(os.Args[2:]) - err := resource.CommandLimitClear(args.Args(), *processType, *global) + err := resource.CommandLimitClear(args.Args(), *processType) if err != nil { common.LogFail(err.Error()) } diff --git a/plugins/resource/src/subcommands/limit/limit.go b/plugins/resource/src/subcommands/limit/limit.go index 024835acd..d45438e3a 100644 --- a/plugins/resource/src/subcommands/limit/limit.go +++ b/plugins/resource/src/subcommands/limit/limit.go @@ -11,7 +11,6 @@ import ( func main() { args := flag.NewFlagSet("resource:limit", flag.ExitOnError) processType := args.String("process-type", "", "process-type: A process type to manage") - global := args.Bool("global", false, "global: Whether to clear global settings or not") cpu := args.String("cpu", "", "cpu: The amount of cpu to limit processes by") memory := args.String("memory", "", "memory: The amount of memory to limit processes by") memorySwap := args.String("memory-swap", "", "memory-swap: The amount of swap memory to limit processes by") @@ -29,7 +28,7 @@ func main() { NetworkEgress: *networkEgress, } - err := resource.CommandLimit(args.Args(), *processType, resources, *global) + err := resource.CommandLimit(args.Args(), *processType, resources) if err != nil { common.LogFail(err.Error()) } diff --git a/plugins/resource/src/subcommands/reserve-clear/reserve-clear.go b/plugins/resource/src/subcommands/reserve-clear/reserve-clear.go index c27a02bba..4f882388d 100644 --- a/plugins/resource/src/subcommands/reserve-clear/reserve-clear.go +++ b/plugins/resource/src/subcommands/reserve-clear/reserve-clear.go @@ -11,10 +11,9 @@ import ( func main() { args := flag.NewFlagSet("resource:reserve-clear", flag.ExitOnError) processType := args.String("process-type", "", "process-type: A process type to clear") - global := args.Bool("global", false, "global: Whether to clear global settings or not") args.Parse(os.Args[2:]) - err := resource.CommandReserveClear(args.Args(), *processType, *global) + err := resource.CommandReserveClear(args.Args(), *processType) if err != nil { common.LogFail(err.Error()) } diff --git a/plugins/resource/src/subcommands/reserve/reserve.go b/plugins/resource/src/subcommands/reserve/reserve.go index a62a34d55..afeccb551 100644 --- a/plugins/resource/src/subcommands/reserve/reserve.go +++ b/plugins/resource/src/subcommands/reserve/reserve.go @@ -11,7 +11,6 @@ import ( func main() { args := flag.NewFlagSet("resource:reserve", flag.ExitOnError) processType := args.String("process-type", "", "process-type: A process type to manage") - global := args.Bool("global", false, "global: Whether to clear global settings or not") cpu := args.String("cpu", "", "cpu: The amount of cpu to reserve for processes") memory := args.String("memory", "", "memory: The amount of memory to reserve for processes") memorySwap := args.String("memory-swap", "", "memory-swap: The amount of swap memory to reserve for processes") @@ -29,7 +28,7 @@ func main() { NetworkEgress: *networkEgress, } - err := resource.CommandReserve(args.Args(), *processType, resources, *global) + err := resource.CommandReserve(args.Args(), *processType, resources) if err != nil { common.LogFail(err.Error()) } diff --git a/plugins/resource/subcommands.go b/plugins/resource/subcommands.go index 040c9ef44..09a5dab73 100644 --- a/plugins/resource/subcommands.go +++ b/plugins/resource/subcommands.go @@ -7,8 +7,8 @@ import ( ) // CommandLimit implements resource:limit -func CommandLimit(args []string, processType string, r Resource, global bool) error { - appName, err := getAppName(args, global) +func CommandLimit(args []string, processType string, r Resource) error { + appName, err := getAppName(args) if err != nil { return err } @@ -17,8 +17,8 @@ func CommandLimit(args []string, processType string, r Resource, global bool) er } // CommandLimitClear implements resource:limit-clear -func CommandLimitClear(args []string, processType string, global bool) error { - appName, err := getAppName(args, global) +func CommandLimitClear(args []string, processType string) error { + appName, err := getAppName(args) if err != nil { return err } @@ -27,8 +27,8 @@ func CommandLimitClear(args []string, processType string, global bool) error { } // CommandReserve implements resource:reserve -func CommandReserve(args []string, processType string, r Resource, global bool) error { - appName, err := getAppName(args, global) +func CommandReserve(args []string, processType string, r Resource) error { + appName, err := getAppName(args) if err != nil { return err } @@ -37,8 +37,8 @@ func CommandReserve(args []string, processType string, r Resource, global bool) } // CommandReserveClear implements resource:reserve-clear -func CommandReserveClear(args []string, processType string, global bool) error { - appName, err := getAppName(args, global) +func CommandReserveClear(args []string, processType string) error { + appName, err := getAppName(args) if err != nil { return err } @@ -52,31 +52,40 @@ func clearByRequestType(appName string, processType string, requestType string) noun = "reservation" } - humanAppName := appName - if appName == "_all_" { - humanAppName = "default" - } - message := fmt.Sprintf("clearing %v %v", humanAppName, noun) - if processType != "_default_" { + message := fmt.Sprintf("clearing %v %v", appName, noun) + if processType != "_default_" && processType != "" { message = fmt.Sprintf("%v (%v)", message, processType) } common.LogInfo2Quiet(message) - resources := []string{ - "cpu", - "memory", - "memory-swap", - "network", - "network-ingress", - "network-egress", - } - - for _, key := range resources { - property := propertyKey(processType, requestType, key) - err := common.PropertyDelete("resource", appName, property) + if processType == "" { + resources, err := common.PropertyGetAll("resource", appName) if err != nil { return err } + for key, _ := range resources { + err := common.PropertyDelete("resource", appName, key) + if err != nil { + return err + } + } + } else { + resources := []string{ + "cpu", + "memory", + "memory-swap", + "network", + "network-ingress", + "network-egress", + } + + for _, key := range resources { + property := propertyKey(processType, requestType, key) + err := common.PropertyDelete("resource", appName, property) + if err != nil { + return err + } + } } return nil } @@ -112,10 +121,6 @@ func setRequestType(appName string, processType string, r Resource, requestType noun = "reservation" } message := fmt.Sprintf("Setting resource %v for %v", noun, appName) - if appName == "_all_" { - message = fmt.Sprintf("Setting default resource %v", noun) - } - if processType != "_default_" { message = fmt.Sprintf("%v (%v)", message, processType) } @@ -142,11 +147,7 @@ func reportRequestType(appName string, processType string, requestType string) { noun = "reservation" } - humanAppName := appName - if appName == "_all_" { - humanAppName = "default" - } - message := fmt.Sprintf("resource %v %v information", noun, humanAppName) + message := fmt.Sprintf("resource %v %v information", noun, appName) if processType != "_default_" { message = fmt.Sprintf("%v (%v)", message, processType) } @@ -173,17 +174,12 @@ func propertyKey(processType string, requestType string, key string) string { return fmt.Sprintf("%v.%v.%v", processType, requestType, key) } -func getAppName(args []string, global bool) (string, error) { - appName := "_all_" - if global { - return appName, nil - } - +func getAppName(args []string) (string, error) { if len(args) < 1 { return "", errors.New("Please specify an app to run the command on") } - appName = args[0] + appName := args[0] if err := common.VerifyAppName(appName); err != nil { return "", err }