cleanup #3403: removed --memory, --min-memory. --max-memory, --cpu, -…

…-min-cpu, --max-cpu flags from 'odo create' command (#3475)

* removed --cpu, --max-cpu,--max-memory,--memory,--max-memory,--min-memory flags to simplify odo create command

* add memory and cpu validation for 'config set' and 'push' commands

* Corrected test cases

pkg/component/component.go

@@ -530,9 +530,31 @@ func ValidateComponentCreateRequest(client *occlient.Client, componentSettings c
 		}
 	}
 
+	if err := ensureAndLogProperResourceUsage(componentSettings.MinMemory, componentSettings.MaxMemory, "memory"); err != nil {
+		return err
+	}
+
+	if err := ensureAndLogProperResourceUsage(componentSettings.MinCPU, componentSettings.MaxCPU, "cpu"); err != nil {
+		return err
+	}
+
 	return
 }
 
+func ensureAndLogProperResourceUsage(resourceMin, resourceMax *string, resourceName string) error {
+	klog.V(4).Infof("Validating configured %v values", resourceName)
+
+	err := fmt.Errorf("`min%s` should accompany `max%s` or use `odo config set %s` to use same value for both min and max", resourceName, resourceName, resourceName)
+
+	if (resourceMin == nil) != (resourceMax == nil) {
+		return err
+	}
+	if (resourceMin != nil && *resourceMin == "") != (resourceMax != nil && *resourceMax == "") {
+		return err
+	}
+	return nil
+}
+
 // ApplyConfig applies the component config onto component dc
 // Parameters:
 //	client: occlient instance

pkg/odo/cli/component/create.go

@@ -47,18 +47,9 @@ type CreateOptions struct {
 	componentContext  string
 	componentPorts    []string
 	componentEnvVars  []string
-	memoryMax         string
-
-	memoryMin string
-
-	appName string
-
-	memory      string
-	cpuMax      string
-	cpuMin      string
-	cpu         string
-	interactive bool
-	now         bool
+	appName           string
+	interactive       bool
+	now               bool
 	*CommonPushOptions
 	devfileMetadata DevfileMetadata
 }
@@ -142,8 +133,8 @@ Note: When you use odo with experimental mode enabled and create devfile compone
 # Create new Node.js component with source from remote git repository
 %[1]s nodejs --git https://github.com/openshift/nodejs-ex.git
 
-# Create new Node.js component with custom ports, additional environment variables and memory and cpu limits
-%[1]s nodejs --port 8080,8100/tcp,9100/udp --env key=value,key1=value1 --memory 4Gi --cpu 2
+# Create new Node.js component with custom ports and environment variables
+%[1]s nodejs --port 8080,8100/tcp,9100/udp --env key=value,key1=value1
 
 # Create new Node.js component and download the sample project named nodejs-starter
 %[1]s nodejs --starter=nodejs-starter`)
@@ -304,36 +295,6 @@ func createDefaultComponentName(context *genericclioptions.Context, componentTyp
 	return componentName, nil
 }
 
-func (co *CreateOptions) setResourceLimits() error {
-	ensureAndLogProperResourceUsage(co.memory, co.memoryMin, co.memoryMax, "memory")
-
-	ensureAndLogProperResourceUsage(co.cpu, co.cpuMin, co.cpuMax, "cpu")
-
-	memoryQuantity, err := util.FetchResourceQuantity(corev1.ResourceMemory, co.memoryMin, co.memoryMax, co.memory)
-	if err != nil {
-		return err
-	}
-	if memoryQuantity != nil {
-		minMemory := memoryQuantity.MinQty.String()
-		maxMemory := memoryQuantity.MaxQty.String()
-		co.componentSettings.MinMemory = &minMemory
-		co.componentSettings.MaxMemory = &maxMemory
-	}
-
-	cpuQuantity, err := util.FetchResourceQuantity(corev1.ResourceCPU, co.cpuMin, co.cpuMax, co.cpu)
-	if err != nil {
-		return err
-	}
-	if cpuQuantity != nil {
-		minCPU := cpuQuantity.MinQty.String()
-		maxCPU := cpuQuantity.MaxQty.String()
-		co.componentSettings.MinCPU = &minCPU
-		co.componentSettings.MaxCPU = &maxCPU
-	}
-
-	return nil
-}
-
 // Complete completes create args
 func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) {
 
@@ -761,10 +722,6 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string
 		if err != nil {
 			return err
 		}
-		err = co.setResourceLimits()
-		if err != nil {
-			return err
-		}
 
 		var portList []string
 		if len(co.componentPorts) > 0 {
@@ -1089,33 +1046,6 @@ func (co *CreateOptions) Run() (err error) {
 	return
 }
 
-// The general cpu/memory is used as a fallback when it's set and both min-cpu/memory max-cpu/memory are not set
-// when the only thing specified is the min or max value, we exit the application
-func ensureAndLogProperResourceUsage(resource, resourceMin, resourceMax, resourceName string) {
-	if strings.HasPrefix(resourceMin, "-") {
-		log.Errorf("min-%s cannot be negative", resource)
-		os.Exit(1)
-	}
-	if strings.HasPrefix(resourceMax, "-") {
-		log.Errorf("max-%s cannot be negative", resource)
-		os.Exit(1)
-	}
-	if strings.HasPrefix(resource, "-") {
-		log.Errorf("%s cannot be negative", resource)
-		os.Exit(1)
-	}
-	if resourceMin != "" && resourceMax != "" && resource != "" {
-		log.Infof("`--%s` will be ignored as `--min-%s` and `--max-%s` has been passed\n", resourceName, resourceName, resourceName)
-	}
-	if (resourceMin == "") != (resourceMax == "") && resource != "" {
-		log.Infof("Using `--%s` %s for min and max limits.\n", resourceName, resource)
-	}
-	if (resourceMin == "") != (resourceMax == "") && resource == "" {
-		log.Errorf("`--min-%s` should accompany `--max-%s` or pass `--%s` to use same value for both min and max or try not passing any of them\n", resourceName, resourceName, resourceName)
-		os.Exit(1)
-	}
-}
-
 func checkoutProject(sparseCheckoutDir, zipURL, path string) error {
 
 	if sparseCheckoutDir != "" {
@@ -1151,12 +1081,6 @@ func NewCmdCreate(name, fullName string) *cobra.Command {
 	componentCreateCmd.Flags().StringVarP(&co.componentGit, "git", "g", "", "Create a git component using this repository.")
 	componentCreateCmd.Flags().StringVarP(&co.componentGitRef, "ref", "r", "", "Use a specific ref e.g. commit, branch or tag of the git repository (only valid for --git components)")
 	genericclioptions.AddContextFlag(componentCreateCmd, &co.componentContext)
-	componentCreateCmd.Flags().StringVar(&co.memory, "memory", "", "Amount of memory to be allocated to the component. ex. 100Mi (sets min-memory and max-memory to this value)")
-	componentCreateCmd.Flags().StringVar(&co.memoryMin, "min-memory", "", "Limit minimum amount of memory to be allocated to the component. ex. 100Mi")
-	componentCreateCmd.Flags().StringVar(&co.memoryMax, "max-memory", "", "Limit maximum amount of memory to be allocated to the component. ex. 100Mi")
-	componentCreateCmd.Flags().StringVar(&co.cpu, "cpu", "", "Amount of cpu to be allocated to the component. ex. 100m or 0.1 (sets min-cpu and max-cpu to this value)")
-	componentCreateCmd.Flags().StringVar(&co.cpuMin, "min-cpu", "", "Limit minimum amount of cpu to be allocated to the component. ex. 100m")
-	componentCreateCmd.Flags().StringVar(&co.cpuMax, "max-cpu", "", "Limit maximum amount of cpu to be allocated to the component. ex. 1")
 	componentCreateCmd.Flags().StringSliceVarP(&co.componentPorts, "port", "p", []string{}, "Ports to be used when the component is created (ex. 8080,8100/tcp,9100/udp)")
 	componentCreateCmd.Flags().StringSliceVar(&co.componentEnvVars, "env", []string{}, "Environmental variables for the component. For example --env VariableName=Value")
 

pkg/odo/cli/config/set.go

@@ -170,6 +170,11 @@ func isValidArgumentList(args []string) error {
 	}
 
 	switch param {
+	case "memory", "minmemory", "maxmemory", "cpu", "mincpu", "maxcpu":
+		err = validation.NonNegativeValidator(args[1])
+		if err != nil {
+			err = errors.Errorf("%s is invalid %v", param, err)
+		}
 	case "ports", "debugport":
 		err = validation.PortsValidator(args[1])
 	}

pkg/odo/util/validation/validators.go

@@ -2,10 +2,11 @@ package validation
 
 import (
 	"fmt"
-	"github.com/openshift/odo/pkg/util"
-	"gopkg.in/AlecAivazis/survey.v1"
 	"strconv"
 	"strings"
+
+	"github.com/openshift/odo/pkg/util"
+	"gopkg.in/AlecAivazis/survey.v1"
 )
 
 // NameValidator provides a Validator view of the ValidateName function.
@@ -40,6 +41,16 @@ func IntegerValidator(ans interface{}) error {
 	return fmt.Errorf("don't know how to convert %v into an integer", ans)
 }
 
+// NonNegativeValidator validates whether the given value is not negative or not
+func NonNegativeValidator(arg interface{}) error {
+	if s, ok := arg.(string); ok {
+		if strings.HasPrefix(s, "-") {
+			return fmt.Errorf("negative value is not acceptable")
+		}
+	}
+	return nil
+}
+
 // PathValidator validates whether the given path exists on the file system
 func PathValidator(path interface{}) error {
 	if s, ok := path.(string); ok {

tests/e2escenarios/e2e_images_test.go

@@ -41,7 +41,10 @@ var _ = Describe("odo supported images e2e tests", func() {
 
 		// create the component
 		helper.CopyExample(filepath.Join("source", srcType), context)
-		helper.CmdShouldPass("odo", "create", cmpType, srcType+"-app", "--project", project, "--context", context, "--app", appName, "--min-memory", "400Mi", "--max-memory", "700Mi")
+		helper.CmdShouldPass("odo", "create", cmpType, srcType+"-app", "--project", project, "--context", context, "--app", appName)
+
+		helper.CmdShouldPass("odo", "config", "set", "minmemory", "400Mi", "--context", context)
+		helper.CmdShouldPass("odo", "config", "set", "maxmemory", "700Mi", "--context", context)
 
 		// push component and validate
 		helper.CmdShouldPass("odo", "push", "--context", context)

tests/integration/cmd_push_test.go

@@ -53,6 +53,72 @@ var _ = Describe("odo push command tests", func() {
 
 	})
 
+	Context("Check memory and cpu config before odo push", func() {
+		It("Should work when memory is set..", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "Memory", "300Mi", "--context", context)
+			helper.CmdShouldPass("odo", "push", "--context", context)
+		})
+
+		It("Should fail if minMemory is set but maxmemory is not set..", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "minmemory", "100Mi", "--context", context)
+			output := helper.CmdShouldFail("odo", "push", "--context", context)
+			Expect(output).To(ContainSubstring("`minmemory` should accompany `maxmemory` or use `odo config set memory` to use same value for both min and max"))
+		})
+
+		It("should fail if maxmemory is set but minmemory is not set..", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "maxmemory", "400Mi", "--context", context)
+			output := helper.CmdShouldFail("odo", "push", "--context", context)
+			Expect(output).To(ContainSubstring("`minmemory` should accompany `maxmemory` or use `odo config set memory` to use same value for both min and max"))
+		})
+
+		It("Should work when cpu is set", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "cpu", "0.4", "--context", context)
+			helper.CmdShouldPass("odo", "push", "--context", context)
+		})
+
+		It("Should fail if mincpu is set but maxcpu is not set..", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "mincpu", "0.4", "--context", context)
+			output := helper.CmdShouldFail("odo", "push", "--context", context)
+			Expect(output).To(ContainSubstring("`mincpu` should accompany `maxcpu` or use `odo config set cpu` to use same value for both min and max"))
+		})
+
+		It("should fail if maxcpu is set but mincpu is not set..", func() {
+
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+
+			helper.CmdShouldPass("odo", "component", "create", "nodejs", cmpName, "--project", project, "--context", context, "--app", appName)
+
+			helper.CmdShouldPass("odo", "config", "set", "maxcpu", "0.5", "--context", context)
+			output := helper.CmdShouldFail("odo", "push", "--context", context)
+			Expect(output).To(ContainSubstring("`mincpu` should accompany `maxcpu` or use `odo config set cpu` to use same value for both min and max"))
+		})
+	})
+
 	Context("Check for label propagation after pushing", func() {
 
 		It("Check for labels", func() {

tests/integration/component.go

@@ -192,8 +192,8 @@ func componentTests(args ...string) {
 		})
 
 		It("should list the component", func() {
-			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--min-memory", "100Mi", "--max-memory", "300Mi", "--min-cpu", "0.1", "--max-cpu", "2", "--context", context, "--app", "testing")...)
-			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing", "MaxMemory,300Mi")
+			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing")...)
+			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing")
 			helper.CmdShouldPass("odo", append(args, "push", "--context", context)...)
 
 			cmpList := helper.CmdShouldPass("odo", append(args, "list", "--project", project)...)
@@ -207,15 +207,15 @@ func componentTests(args ...string) {
 		})
 
 		It("should list the component when it is not pushed", func() {
-			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--min-memory", "100Mi", "--max-memory", "300Mi", "--min-cpu", "0.1", "--max-cpu", "2", "--context", context, "--app", "testing")...)
-			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing", "MinCPU,100m")
+			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing")...)
+			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing")
 			cmpList := helper.CmdShouldPass("odo", append(args, "list", "--context", context)...)
 			helper.MatchAllInOutput(cmpList, []string{"cmp-git", "Not Pushed"})
 			helper.CmdShouldPass("odo", append(args, "delete", "-f", "--all", "--context", context)...)
 		})
 		It("should list the state as unknown for disconnected cluster", func() {
-			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--min-memory", "100Mi", "--max-memory", "300Mi", "--min-cpu", "0.1", "--max-cpu", "2", "--context", context, "--app", "testing")...)
-			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing", "MinCPU,100m")
+			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing")...)
+			helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,cmp-git", "Application,testing")
 			kubeconfigOrig := os.Getenv("KUBECONFIG")
 			os.Setenv("KUBECONFIG", "/no/such/path")
 			cmpList := helper.CmdShouldPass("odo", append(args, "list", "--context", context, "--v", "9")...)
@@ -602,12 +602,8 @@ func componentTests(args ...string) {
 
 		It("should be able to create a git component and update it from local to git", func() {
 			helper.CopyExample(filepath.Join("source", "nodejs"), context)
-			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--min-cpu", "0.1", "--max-cpu", "2", "--context", context, "--app", "testing")...)
+			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--context", context, "--app", "testing")...)
 			helper.CmdShouldPass("odo", append(args, "push", "--context", context)...)
-			getCPULimit := oc.MaxCPU("cmp-git", "testing", project)
-			Expect(getCPULimit).To(ContainSubstring("2"))
-			getCPURequest := oc.MinCPU("cmp-git", "testing", project)
-			Expect(getCPURequest).To(ContainSubstring("100m"))
 
 			helper.CmdShouldPass("odo", "update", "--git", "https://github.com/openshift/nodejs-ex.git", "--context", context)
 			// check the source location and type in the deployment config
@@ -618,23 +614,14 @@ func componentTests(args ...string) {
 		})
 
 		It("should be able to update a component from git to local", func() {
-			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--min-memory", "100Mi", "--max-memory", "300Mi", "--context", context, "--app", "testing")...)
+			helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--project", project, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing")...)
 			helper.CmdShouldPass("odo", append(args, "push", "--context", context)...)
-			getMemoryLimit := oc.MaxMemory("cmp-git", "testing", project)
-			Expect(getMemoryLimit).To(ContainSubstring("300Mi"))
-			getMemoryRequest := oc.MinMemory("cmp-git", "testing", project)
-			Expect(getMemoryRequest).To(ContainSubstring("100Mi"))
 
 			// update the component config according to the git component
 			helper.CopyExample(filepath.Join("source", "nodejs"), context)
 
 			helper.CmdShouldPass("odo", "update", "--local", "./", "--context", context)
 
-			getMemoryLimit = oc.MaxMemory("cmp-git", "testing", project)
-			Expect(getMemoryLimit).To(ContainSubstring("300Mi"))
-			getMemoryRequest = oc.MinMemory("cmp-git", "testing", project)
-			Expect(getMemoryRequest).To(ContainSubstring("100Mi"))
-
 			// check the source location and type in the deployment config
 			getSourceLocation := oc.SourceLocationDC("cmp-git", "testing", project)
 			Expect(getSourceLocation).To(ContainSubstring(""))
@@ -764,20 +751,6 @@ func componentTests(args ...string) {
 		})
 	})
 
-	Context("when creating component with improper memory quantities", func() {
-		JustBeforeEach(func() {
-			project = helper.CreateRandProject()
-		})
-		JustAfterEach(func() {
-			helper.DeleteProject(project)
-		})
-		It("should fail gracefully with proper error message", func() {
-			stdError := helper.CmdShouldFail("odo", append(args, "create", "java:8", "backend", "--memory", "1GB", "--project", project, "--context", context)...)
-			Expect(stdError).ToNot(ContainSubstring("panic: cannot parse"))
-			Expect(stdError).To(ContainSubstring("quantities must match the regular expression"))
-		})
-	})
-
 	Context("Creating component using symlink", func() {
 		var symLinkPath string