From 3ec299e39df061bd11e0ba779435b40e2e451cba Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 08:47:04 +0100 Subject: [PATCH 1/6] Remove CommonPushOptions and do some clean up --- Makefile | 5 ++ .../adapters/kubernetes/component/adapter.go | 1 + .../parser/data/2.0.0/devfileJsonSchema200.go | 4 +- pkg/devfile/parser/data/common/types.go | 13 +--- pkg/odo/cli/component/deploy.go | 40 ++++++------- pkg/odo/cli/component/deploy_delete.go | 16 ++--- pkg/odo/cli/component/devfile.go | 59 +++---------------- pkg/sync/adapter.go | 2 +- pkg/util/util.go | 20 ++++++- .../source/devfilesV2/nodejs/devfile.yaml | 4 +- .../devfile/cmd_devfile_deploy_test.go | 23 +++++++- 11 files changed, 84 insertions(+), 103 deletions(-) diff --git a/Makefile b/Makefile index f8b8ab26e2b..321818ea715 100644 --- a/Makefile +++ b/Makefile @@ -208,6 +208,11 @@ test-cmd-devfile-create: test-cmd-devfile-push: ginkgo $(GINKGO_FLAGS) -focus="odo devfile push command tests" tests/integration/devfile/ +# Run odo push devfile command tests +.PHONY: test-cmd-devfile-deploy +test-cmd-devfile-deploy: + ginkgo $(GINKGO_FLAGS) -focus="odo devfile deploy command tests" tests/integration/devfile/ + # Run odo push devfile command tests .PHONY: test-cmd-devfile-deploy-delete test-cmd-devfile-deploy-delete: diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go index df6e37d9be6..f9c529d0ee9 100644 --- a/pkg/devfile/adapters/kubernetes/component/adapter.go +++ b/pkg/devfile/adapters/kubernetes/component/adapter.go @@ -85,6 +85,7 @@ func (a Adapter) runBuildConfig(client *occlient.Client, parameters common.Build return err } + log.Successf("Started build using BuildConfig") bc, err := client.RunBuildConfigWithBinaryInput(buildName, reader) if err != nil { return err diff --git a/pkg/devfile/parser/data/2.0.0/devfileJsonSchema200.go b/pkg/devfile/parser/data/2.0.0/devfileJsonSchema200.go index 5e6f75b1422..bf008943969 100644 --- a/pkg/devfile/parser/data/2.0.0/devfileJsonSchema200.go +++ b/pkg/devfile/parser/data/2.0.0/devfileJsonSchema200.go @@ -849,11 +849,11 @@ const JsonSchema200 = `{ "type": "string", "description": "Optional devfile name" }, - "dockerfile": { + "alpha.build-dockerfile": { "type":"string", "description": "Optional URL to remote Dockerfile" }, - "deployment-manifest": { + "alpha.deployment-manifest": { "type":"string", "description": "Optional URL to remote Deployment Manifest" } diff --git a/pkg/devfile/parser/data/common/types.go b/pkg/devfile/parser/data/common/types.go index 0daf1fcd647..0a62d57b2d0 100644 --- a/pkg/devfile/parser/data/common/types.go +++ b/pkg/devfile/parser/data/common/types.go @@ -58,10 +58,10 @@ type DevfileMetadata struct { Version string `json:"version,omitempty"` // Dockerfile optional URL to remote Dockerfile - Dockerfile string `json:"dockerfile,omitempty"` + Dockerfile string `json:"alpha.build-dockerfile,omitempty"` // Manifest optional URL to remote Deployment Manifest - Manifest string `json:"deployment-manifest,omitempty"` + Manifest string `json:"alpha.deployment-manifest,omitempty"` } // DevfileCommand command specified in devfile @@ -100,15 +100,6 @@ type DevfileComponent struct { // Volume component Volume *Volume `json:"volume,omitempty"` - - // Dockerfile component - Dockerfile *Dockerfile `json:"dockerfile,omitempty"` -} - -// Dockerfile cmponent -type Dockerfile struct { - Path string `json:"path"` - Args []string `json:"args,omitempty"` } // DevfileProject project defined in devfile diff --git a/pkg/odo/cli/component/deploy.go b/pkg/odo/cli/component/deploy.go index 5b4380a7c09..3520827cf75 100644 --- a/pkg/odo/cli/component/deploy.go +++ b/pkg/odo/cli/component/deploy.go @@ -11,7 +11,6 @@ import ( projectCmd "github.com/openshift/odo/pkg/odo/cli/project" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util/completion" - "github.com/openshift/odo/pkg/odo/util/experimental" "github.com/openshift/odo/pkg/util" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -23,7 +22,7 @@ import ( // TODO: add CLI Reference doc // TODO: add delete example -var deployCmdExample = ktemplates.Examples(` # Deploys an image and deploys the application +var deployCmdExample = ktemplates.Examples(` # Build and Deploy our application.Deploys an image and deploys the application %[1]s `) @@ -32,9 +31,11 @@ const DeployRecommendedCommandName = "deploy" // DeployOptions encapsulates options that build command uses type DeployOptions struct { - *CommonPushOptions + componentContext string + sourcePath string + ignores []string + EnvSpecificInfo *envinfo.EnvSpecificInfo - // devfile path DevfilePath string DockerfileURL string DockerfileBytes []byte @@ -42,17 +43,17 @@ type DeployOptions struct { tag string ManifestSource []byte deployOnly bool + + *genericclioptions.Context } // NewDeployOptions returns new instance of BuildOptions // with "default" values for certain values, for example, show is "false" func NewDeployOptions() *DeployOptions { - return &DeployOptions{ - CommonPushOptions: NewCommonPushOptions(), - } + return &DeployOptions{} } -// Complete completes push args +// Complete completes deploy args func (do *DeployOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { do.DevfilePath = filepath.Join(do.componentContext, do.DevfilePath) envInfo, err := envinfo.NewEnvSpecificInfo(do.componentContext) @@ -68,6 +69,7 @@ func (do *DeployOptions) Complete(name string, cmd *cobra.Command, args []string // Validate validates the push parameters func (do *DeployOptions) Validate() (err error) { + log.Infof("\nValidation") // Validate the --tag if do.tag == "" { return errors.New("odo deploy requires a tag, in the format /namespace>/") @@ -125,22 +127,21 @@ func (do *DeployOptions) Run() (err error) { } } else if !util.CheckPathExists(filepath.Join(localDir, "Dockerfile")) { - return errors.New("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory") - } - - err = do.DevfileBuild() - if err != nil { - return err + return errors.New("dockerfile required for build. No 'alpha.build-dockerfile' field found in devfile, or Dockerfile found in project directory") } } + // Need to add validation? + // s := log.Spinner("Validating Dockerfile") + // s.End(true) + // s.End(false) manifestURL := metadata.Manifest do.ManifestSource, err = util.DownloadFileInMemory(manifestURL) if err != nil { return errors.Wrap(err, "Unable to download manifest "+manifestURL) } - err = do.DevfileDeploy() + err = do.DevfileDeploy(devObj) if err != nil { return err } @@ -173,11 +174,10 @@ func NewCmdDeploy(name, fullName string) *cobra.Command { genericclioptions.AddContextFlag(deployCmd, &do.componentContext) // enable devfile flag if experimental mode is enabled - if experimental.IsExperimentalModeEnabled() { - deployCmd.Flags().StringVar(&do.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml") - deployCmd.Flags().StringVar(&do.tag, "tag", "", "Tag used to build the image") - deployCmd.Flags().BoolVar(&do.deployOnly, "deployOnly", false, "Do not build the application, only deploy it") - } + deployCmd.Flags().StringVar(&do.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml") + deployCmd.Flags().StringVar(&do.tag, "tag", "", "Tag used to build the image") + deployCmd.Flags().BoolVar(&do.deployOnly, "deployOnly", false, "Do not build the application, only deploy it") + deployCmd.Flags().StringSliceVar(&do.ignores, "ignore", []string{}, "Files or folders to be ignored via glob expressions.") //Adding `--project` flag projectCmd.AddProjectFlag(deployCmd) diff --git a/pkg/odo/cli/component/deploy_delete.go b/pkg/odo/cli/component/deploy_delete.go index a32de44efa5..605bd9c42bd 100644 --- a/pkg/odo/cli/component/deploy_delete.go +++ b/pkg/odo/cli/component/deploy_delete.go @@ -10,7 +10,6 @@ import ( projectCmd "github.com/openshift/odo/pkg/odo/cli/project" "github.com/openshift/odo/pkg/odo/genericclioptions" "github.com/openshift/odo/pkg/odo/util/completion" - "github.com/openshift/odo/pkg/odo/util/experimental" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -31,21 +30,21 @@ const DeployDeleteRecommendedCommandName = "delete" // DeployDeleteOptions encapsulates options that deploy delete command uses type DeployDeleteOptions struct { - *CommonPushOptions + componentContext string + EnvSpecificInfo *envinfo.EnvSpecificInfo - // devfile path DevfilePath string namespace string ManifestPath string ManifestSource []byte + + *genericclioptions.Context } // NewDeployDeleteOptions returns new instance of DeployDeleteOptions // with "default" values for certain values, for example, show is "false" func NewDeployDeleteOptions() *DeployDeleteOptions { - return &DeployDeleteOptions{ - CommonPushOptions: NewCommonPushOptions(), - } + return &DeployDeleteOptions{} } // Complete completes push args @@ -106,11 +105,6 @@ func NewCmdDeployDelete(name, fullName string) *cobra.Command { } genericclioptions.AddContextFlag(deployDeleteCmd, &ddo.componentContext) - // enable devfile flag if experimental mode is enabled - if experimental.IsExperimentalModeEnabled() { - deployDeleteCmd.Flags().StringVar(&ddo.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml") - } - //Adding `--project` flag projectCmd.AddProjectFlag(deployDeleteCmd) diff --git a/pkg/odo/cli/component/devfile.go b/pkg/odo/cli/component/devfile.go index 618d82d0c53..e3d16b86f49 100644 --- a/pkg/odo/cli/component/devfile.go +++ b/pkg/odo/cli/component/devfile.go @@ -101,19 +101,12 @@ func (po *PushOptions) DevfilePush() (err error) { return } -// DevfileBuild build an image of my application in the cluster -func (do *DeployOptions) DevfileBuild() (err error) { - // Parse devfile - devObj, err := devfileParser.Parse(do.DevfilePath) - if err != nil { - return err - } - +//DevfileDeploy +func (do *DeployOptions) DevfileDeploy(devObj devfileParser.DevfileObj) (err error) { componentName, err := getComponentName(do.componentContext) if err != nil { return errors.Wrap(err, "unable to get component name") } - componentName = "build-" + componentName // Set the source path to either the context or current working directory (if context not set) do.sourcePath, err = util.GetAbsPath(do.componentContext) @@ -124,7 +117,7 @@ func (do *DeployOptions) DevfileBuild() (err error) { // Apply ignore information err = genericclioptions.ApplyIgnore(&do.ignores, do.sourcePath) if err != nil { - return errors.Wrap(err, "unable to apply ignore information") + return errors.Wrap(err, "Unable to apply ignore information") } kubeContext := kubernetes.KubernetesContext{ @@ -143,9 +136,7 @@ func (do *DeployOptions) DevfileBuild() (err error) { EnvSpecificInfo: *do.EnvSpecificInfo, } - // TODO: I don't think we need to check this here, we could check this on the deploy if we want to expose a URL (odo url create) - warnIfURLSInvalid(do.EnvSpecificInfo.GetURL()) - + log.Infof("\nBuilding devfile component %s", componentName) // Build image for the component err = devfileHandler.Build(buildParams) if err != nil { @@ -156,45 +147,7 @@ func (do *DeployOptions) DevfileBuild() (err error) { ) os.Exit(1) } - - log.Infof("\nBuilding devfile component %s", componentName) - log.Success("Changes successfully built image for component") - - return nil -} - -func (do *DeployOptions) DevfileDeploy() (err error) { - // Parse devfile - devObj, err := devfileParser.Parse(do.DevfilePath) - if err != nil { - return err - } - - componentName, err := getComponentName(do.componentContext) - if err != nil { - return errors.Wrap(err, "unable to get component name") - } - - // Set the source path to either the context or current working directory (if context not set) - do.sourcePath, err = util.GetAbsPath(do.componentContext) - if err != nil { - return errors.Wrap(err, "unable to get source path") - } - - // Apply ignore information - err = genericclioptions.ApplyIgnore(&do.ignores, do.sourcePath) - if err != nil { - return errors.Wrap(err, "Unable to apply ignore information") - } - - kubeContext := kubernetes.KubernetesContext{ - Namespace: do.namespace, - } - - devfileHandler, err := adapters.NewPlatformAdapter(componentName, do.componentContext, devObj, kubeContext) - if err != nil { - return err - } + log.Success("Successfully built image") deployParams := common.DeployParameters{ EnvSpecificInfo: *do.EnvSpecificInfo, @@ -202,6 +155,8 @@ func (do *DeployOptions) DevfileDeploy() (err error) { ManifestSource: do.ManifestSource, } + warnIfURLSInvalid(do.EnvSpecificInfo.GetURL()) + // Deploy the application err = devfileHandler.Deploy(deployParams) if err != nil { diff --git a/pkg/sync/adapter.go b/pkg/sync/adapter.go index 9c20892c1a9..27eca821b99 100644 --- a/pkg/sync/adapter.go +++ b/pkg/sync/adapter.go @@ -44,7 +44,7 @@ func (a Adapter) SyncFilesBuild(buildParameters common.BuildParameters, dockerfi var s *log.Status syncFolder := "/" - s = log.Spinner("Checking files for deploy") + s = log.Spinner("Checking files for building") // run the indexer and find the project source files files, err := util.DeployRunIndexer(buildParameters.Path, absIgnoreRules) if len(files) > 0 { diff --git a/pkg/util/util.go b/pkg/util/util.go index afcf729a155..6c843d45534 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -25,6 +25,7 @@ import ( "github.com/gobwas/glob" "github.com/google/go-github/github" + "github.com/openshift/odo/pkg/log" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -1061,6 +1062,11 @@ func ValidateURL(sourceURL string) error { // ValidateDockerfile validates the string passed through has a FROM on it's first non-whitespace/commented line // This function could be expanded to be a more viable linter func ValidateDockerfile(contents []byte) error { + s := log.Spinner("Validating Dockerfile") + if len(contents) == 0 { + s.End(false) + return errors.New("dockerfile URL provided in the Devfile does not point to an empty file") + } // Split the file downloaded line-by-line splitContents := strings.Split(string(contents), "\n") // The first line in a Dockerfile must be a 'FROM', whitespace, or a comment ('#') @@ -1075,18 +1081,23 @@ func ValidateDockerfile(contents []byte) error { continue } if strings.HasPrefix(line, "FROM") { + s.End(true) return nil } - return errors.Errorf("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") + s.End(false) + return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") } + s.End(false) // Would only reach this return statement if splitContents is 0 - return errors.Errorf("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") + return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") } // ValidateTag validates the string that has been passed as a tag meets the requirements of a tag func ValidateTag(tag string) error { + s := log.Spinner("Validating Tag") var splitTag = strings.Split(tag, "/") if len(splitTag) != 3 { + s.End(false) return errors.New("invalid tag: odo deploy reguires a tag in the format /namespace>/") } @@ -1094,24 +1105,29 @@ func ValidateTag(tag string) error { characterMatch := regexp.MustCompile(`[a-zA-Z0-9\.\-:_]{4,128}`) for _, element := range splitTag { if len(element) < 4 { + s.End(false) return errors.New("invalid tag: " + element + " in the tag is too short. Each element needs to be at least 4 characters.") } if len(element) > 128 { + s.End(false) return errors.New("invalid tag: " + element + " in the tag is too long. Each element cannot be longer than 128.") } // Check that the whole string matches the regular expression // Match.String was returning a match even when only part of the string is working if characterMatch.FindString(element) != element { + s.End(false) return errors.New("invalid tag: " + element + " in the tag contains an illegal character. It must only contain alphanumerical values, periods, colons, underscores, and dashes.") } // The registry, namespace, and image, cannot end in '.', '-', '_',or ':' if strings.HasSuffix(element, ".") || strings.HasSuffix(element, "-") || strings.HasSuffix(element, ":") || strings.HasSuffix(element, "_") { + s.End(false) return errors.New("invalid tag: " + element + " in the tag has an invalid final character. It must end in an alphanumeric value.") } } + s.End(true) return nil } diff --git a/tests/examples/source/devfilesV2/nodejs/devfile.yaml b/tests/examples/source/devfilesV2/nodejs/devfile.yaml index 1ff1fcba556..69014f025cc 100644 --- a/tests/examples/source/devfilesV2/nodejs/devfile.yaml +++ b/tests/examples/source/devfilesV2/nodejs/devfile.yaml @@ -1,8 +1,8 @@ schemaVersion: "2.0.0" metadata: name: test-devfile - dockerfile: "https://raw.githubusercontent.com/neeraj-laad/nodejs-stack-registry/build-deploy/devfiles/nodejs-basic/build/Dockerfile" - deployment-manifest: "https://raw.githubusercontent.com/groeges/devfile-registry/master/devfiles/nodejs/deploy_deployment.yaml" + alpha.build-dockerfile: "https://raw.githubusercontent.com/neeraj-laad/nodejs-stack-registry/build-deploy/devfiles/nodejs-basic/build/Dockerfile" + alpha.deployment-manifest: "https://raw.githubusercontent.com/groeges/devfile-registry/master/devfiles/nodejs/deploy_deployment.yaml" projects: - name: nodejs-web-app git: diff --git a/tests/integration/devfile/cmd_devfile_deploy_test.go b/tests/integration/devfile/cmd_devfile_deploy_test.go index dcfe383894d..6633ba27834 100644 --- a/tests/integration/devfile/cmd_devfile_deploy_test.go +++ b/tests/integration/devfile/cmd_devfile_deploy_test.go @@ -1,6 +1,7 @@ package devfile import ( + "fmt" "os" "path/filepath" "time" @@ -47,9 +48,12 @@ var _ = Describe("odo devfile deploy command tests", func() { helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, cmpName) helper.CmdShouldPass("odo", "url", "create", "--port", "3000") + imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) helper.CopyExampleDevFile(filepath.Join("source", "devfilesV2", "nodejs", "devfile.yaml"), filepath.Join(context, "devfile.yaml")) - output := helper.CmdShouldPass("odo", "deploy", "--tag", imageTag, "--devfile", "devfile.yaml") + output := helper.CmdShouldPass("odo", "deploy", "--tag", imageTag, "devfile.yaml", "-v", "4") Expect(output).NotTo(ContainSubstring("does not point to a valid Dockerfile")) + Expect(output).To(ContainSubstring("Successfully built image")) + Expect(output).To(ContainSubstring("Successfully deployed application")) }) }) @@ -62,17 +66,32 @@ var _ = Describe("odo devfile deploy command tests", func() { err := helper.ReplaceDevfileField("devfile.yaml", "dockerfile", "https://google.com") Expect(err).To(BeNil()) + imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) cmdOutput := helper.CmdShouldFail("odo", "deploy", "--tag", imageTag, "--devfile", "devfile.yaml") Expect(cmdOutput).To(ContainSubstring("does not point to a valid Dockerfile")) }) }) + // This test depends on the nodejs stack to no have a alpha.build-dockerfile field. + // This may not be the case in the future when the stack gets updated. Context("Verify error when no Dockerfile exists in project and no 'dockerfile' specified in devfile", func() { It("Should error out with 'dockerfile required for build.'", func() { helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, cmpName) helper.CmdShouldPass("odo", "url", "create", "--port", "3000") + imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) cmdOutput := helper.CmdShouldFail("odo", "deploy", "--tag", imageTag, "--devfile", "devfile.yaml") - Expect(cmdOutput).To(ContainSubstring("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory")) + Expect(cmdOutput).To(ContainSubstring("dockerfile required for build. No 'alpha.build-dockerfile' field found in devfile, or Dockerfile found in project directory")) }) }) + + // Context("Verify error when no Dockerfile exists in project and no 'dockerfile' specified in devfile", func() { + // It("Should error out with 'dockerfile required for build.'", func() { + // helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, cmpName) + // helper.CmdShouldPass("odo", "url", "create", "--port", "3000") + // imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) + // cmdOutput := helper.CmdShouldFail("odo", "deploy", "--tag", imageTag, "--devfile", "devfile.yaml") + + // Expect(cmdOutput).To(ContainSubstring("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory")) + // }) + // }) }) From 1d13b983b70d86ab44c53fa1bd4891b5103c9a4b Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 11:35:43 +0100 Subject: [PATCH 2/6] Continue cleanup and enhance test --- pkg/odo/cli/component/deploy.go | 7 ++++--- pkg/odo/cli/component/devfile.go | 4 ++-- tests/integration/devfile/cmd_devfile_deploy_test.go | 12 +----------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/pkg/odo/cli/component/deploy.go b/pkg/odo/cli/component/deploy.go index 3520827cf75..661e5fefec8 100644 --- a/pkg/odo/cli/component/deploy.go +++ b/pkg/odo/cli/component/deploy.go @@ -37,6 +37,7 @@ type DeployOptions struct { EnvSpecificInfo *envinfo.EnvSpecificInfo DevfilePath string + devObj devfileParser.DevfileObj DockerfileURL string DockerfileBytes []byte namespace string @@ -85,12 +86,12 @@ func (do *DeployOptions) Validate() (err error) { // Run has the logic to perform the required actions as part of command func (do *DeployOptions) Run() (err error) { - devObj, err := devfileParser.Parse(do.DevfilePath) + do.devObj, err = devfileParser.Parse(do.DevfilePath) if err != nil { return err } - metadata := devObj.Data.GetMetadata() + metadata := do.devObj.Data.GetMetadata() dockerfileURL := metadata.Dockerfile localDir, err := os.Getwd() if err != nil { @@ -141,7 +142,7 @@ func (do *DeployOptions) Run() (err error) { return errors.Wrap(err, "Unable to download manifest "+manifestURL) } - err = do.DevfileDeploy(devObj) + err = do.DevfileDeploy() if err != nil { return err } diff --git a/pkg/odo/cli/component/devfile.go b/pkg/odo/cli/component/devfile.go index e3d16b86f49..daab5d276c3 100644 --- a/pkg/odo/cli/component/devfile.go +++ b/pkg/odo/cli/component/devfile.go @@ -102,7 +102,7 @@ func (po *PushOptions) DevfilePush() (err error) { } //DevfileDeploy -func (do *DeployOptions) DevfileDeploy(devObj devfileParser.DevfileObj) (err error) { +func (do *DeployOptions) DevfileDeploy() (err error) { componentName, err := getComponentName(do.componentContext) if err != nil { return errors.Wrap(err, "unable to get component name") @@ -124,7 +124,7 @@ func (do *DeployOptions) DevfileDeploy(devObj devfileParser.DevfileObj) (err err Namespace: do.namespace, } - devfileHandler, err := adapters.NewPlatformAdapter(componentName, do.componentContext, devObj, kubeContext) + devfileHandler, err := adapters.NewPlatformAdapter(componentName, do.componentContext, do.devObj, kubeContext) if err != nil { return err } diff --git a/tests/integration/devfile/cmd_devfile_deploy_test.go b/tests/integration/devfile/cmd_devfile_deploy_test.go index 6633ba27834..e91b477ddd8 100644 --- a/tests/integration/devfile/cmd_devfile_deploy_test.go +++ b/tests/integration/devfile/cmd_devfile_deploy_test.go @@ -51,6 +51,7 @@ var _ = Describe("odo devfile deploy command tests", func() { imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) helper.CopyExampleDevFile(filepath.Join("source", "devfilesV2", "nodejs", "devfile.yaml"), filepath.Join(context, "devfile.yaml")) output := helper.CmdShouldPass("odo", "deploy", "--tag", imageTag, "devfile.yaml", "-v", "4") + cliRunner.WaitAndCheckForExistence("BuildConfig", namespace, 1) Expect(output).NotTo(ContainSubstring("does not point to a valid Dockerfile")) Expect(output).To(ContainSubstring("Successfully built image")) Expect(output).To(ContainSubstring("Successfully deployed application")) @@ -83,15 +84,4 @@ var _ = Describe("odo devfile deploy command tests", func() { Expect(cmdOutput).To(ContainSubstring("dockerfile required for build. No 'alpha.build-dockerfile' field found in devfile, or Dockerfile found in project directory")) }) }) - - // Context("Verify error when no Dockerfile exists in project and no 'dockerfile' specified in devfile", func() { - // It("Should error out with 'dockerfile required for build.'", func() { - // helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, cmpName) - // helper.CmdShouldPass("odo", "url", "create", "--port", "3000") - // imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) - // cmdOutput := helper.CmdShouldFail("odo", "deploy", "--tag", imageTag, "--devfile", "devfile.yaml") - - // Expect(cmdOutput).To(ContainSubstring("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory")) - // }) - // }) }) From adf47dd39cabe8e98a3740d587e1fc926c52b25e Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 12:21:00 +0100 Subject: [PATCH 3/6] Move validation, add better user experience, remove devfile.yaml flag --- Makefile | 7 +--- pkg/odo/cli/component/deploy.go | 42 +++++++++++-------- pkg/odo/cli/component/deploy_delete.go | 26 ++++++++---- pkg/odo/cli/component/devfile.go | 2 +- .../devfile/cmd_devfile_deploy_delete_test.go | 6 ++- .../devfile/cmd_devfile_deploy_test.go | 2 +- 6 files changed, 48 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index 1428dbac1e3..279c08871ad 100644 --- a/Makefile +++ b/Makefile @@ -213,12 +213,7 @@ test-cmd-devfile-push: test-cmd-devfile-deploy: ginkgo $(GINKGO_FLAGS) -focus="odo devfile deploy command tests" tests/integration/devfile/ -# Run odo push devfile command tests -.PHONY: test-cmd-devfile-deploy -test-cmd-devfile-deploy: - ginkgo $(GINKGO_FLAGS) -focus="odo devfile deploy command tests" tests/integration/devfile/ - -# Run odo push devfile command tests +# Run odo deploy delete devfile command tests .PHONY: test-cmd-devfile-deploy-delete test-cmd-devfile-deploy-delete: ginkgo $(GINKGO_FLAGS) -focus="odo devfile deploy delete command tests" tests/integration/devfile/ diff --git a/pkg/odo/cli/component/deploy.go b/pkg/odo/cli/component/deploy.go index bf7d90d095c..4812fffc567 100644 --- a/pkg/odo/cli/component/deploy.go +++ b/pkg/odo/cli/component/deploy.go @@ -20,10 +20,11 @@ import ( ktemplates "k8s.io/kubectl/pkg/util/templates" ) -// TODO: add CLI Reference doc -// TODO: add delete example -var deployCmdExample = ktemplates.Examples(` # Build and Deploy our application.Deploys an image and deploys the application +var deployCmdExample = ktemplates.Examples(` # Build and Deploy the current component %[1]s + +# Specify the tag for the image by calling +%[1]s --tag //: `) // DeployRecommendedCommandName is the recommended build command name @@ -53,9 +54,18 @@ func NewDeployOptions() *DeployOptions { return &DeployOptions{} } +// CompleteDevfilePath completes the devfile path from context +func (do *DeployOptions) CompleteDevfilePath() { + if len(do.DevfilePath) > 0 { + do.DevfilePath = filepath.Join(do.componentContext, do.DevfilePath) + } else { + do.DevfilePath = filepath.Join(do.componentContext, "devfile.yaml") + } +} + // Complete completes deploy args func (do *DeployOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { - do.DevfilePath = filepath.Join(do.componentContext, do.DevfilePath) + do.CompleteDevfilePath() envInfo, err := envinfo.NewEnvSpecificInfo(do.componentContext) if err != nil { return errors.Wrap(err, "unable to retrieve configuration information") @@ -80,11 +90,6 @@ func (do *DeployOptions) Validate() (err error) { return err } - return -} - -// Run has the logic to perform the required actions as part of command -func (do *DeployOptions) Run() (err error) { do.devObj, err = devfileParser.Parse(do.DevfilePath) if err != nil { return err @@ -128,30 +133,32 @@ func (do *DeployOptions) Run() (err error) { return errors.New("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory") } - err = do.DevfileBuild() - if err != nil { - return err - } - // Need to add validation? - // s := log.Spinner("Validating Dockerfile") - // s.End(true) - // s.End(false) + s := log.Spinner("Validating Manifest URL") manifestURL := metadata.Manifest if manifestURL == "" { + s.End(false) return errors.New("Unable to deploy as alpha.deployment-manifest is not defined in devfile.yaml") } err = util.ValidateURL(manifestURL) if err != nil { + s.End(false) return errors.New(fmt.Sprintf("Invalid manifest url: %s, %s", manifestURL, err)) } do.ManifestSource, err = util.DownloadFileInMemory(manifestURL) if err != nil { + s.End(false) return errors.New(fmt.Sprintf("Unable to download manifest: %s, %s", manifestURL, err)) } + s.End(true) + return +} + +// Run has the logic to perform the required actions as part of command +func (do *DeployOptions) Run() (err error) { err = do.DevfileDeploy() if err != nil { return err @@ -185,7 +192,6 @@ func NewCmdDeploy(name, fullName string) *cobra.Command { genericclioptions.AddContextFlag(deployCmd, &do.componentContext) // enable devfile flag if experimental mode is enabled - deployCmd.Flags().StringVar(&do.DevfilePath, "devfile", "./devfile.yaml", "Path to a devfile.yaml") deployCmd.Flags().StringVar(&do.tag, "tag", "", "Tag used to build the image") deployCmd.Flags().StringSliceVar(&do.ignores, "ignore", []string{}, "Files or folders to be ignored via glob expressions.") diff --git a/pkg/odo/cli/component/deploy_delete.go b/pkg/odo/cli/component/deploy_delete.go index 605bd9c42bd..7883e2bd6f1 100644 --- a/pkg/odo/cli/component/deploy_delete.go +++ b/pkg/odo/cli/component/deploy_delete.go @@ -22,7 +22,8 @@ import ( const manifestFile = ".odo/manifest.yaml" // TODO: add CLI Reference doc -var deployDeleteCmdExample = ktemplates.Examples(` # Deletes deployment from Kubernetes cluster +var deployDeleteCmdExample = ktemplates.Examples(` # Delete deployed component +%[1]s `) // DeployDeleteRecommendedCommandName is the recommended build command name @@ -47,9 +48,18 @@ func NewDeployDeleteOptions() *DeployDeleteOptions { return &DeployDeleteOptions{} } +// CompleteDevfilePath completes the devfile path from context +func (ddo *DeployDeleteOptions) CompleteDevfilePath() { + if len(ddo.DevfilePath) > 0 { + ddo.DevfilePath = filepath.Join(ddo.componentContext, ddo.DevfilePath) + } else { + ddo.DevfilePath = filepath.Join(ddo.componentContext, "devfile.yaml") + } +} + // Complete completes push args func (ddo *DeployDeleteOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { - ddo.DevfilePath = filepath.Join(ddo.componentContext, ddo.DevfilePath) + ddo.CompleteDevfilePath() envInfo, err := envinfo.NewEnvSpecificInfo(ddo.componentContext) if err != nil { return errors.Wrap(err, "unable to retrieve configuration information") @@ -62,11 +72,6 @@ func (ddo *DeployDeleteOptions) Complete(name string, cmd *cobra.Command, args [ // Validate validates the push parameters func (ddo *DeployDeleteOptions) Validate() (err error) { - return -} - -// Run has the logic to perform the required actions as part of command -func (ddo *DeployDeleteOptions) Run() (err error) { // ddo.componentContext, .odo, manifest.yaml // TODO: Check manifest is actually there!!! // read bytes into deployDeleteOptions @@ -74,13 +79,16 @@ func (ddo *DeployDeleteOptions) Run() (err error) { return errors.Wrap(err, "manifest file at "+manifestFile+" does not exist") } - manifestSource, err := ioutil.ReadFile(manifestFile) + ddo.ManifestSource, err = ioutil.ReadFile(manifestFile) if err != nil { return err } ddo.ManifestPath = manifestFile - ddo.ManifestSource = manifestSource + return +} +// Run has the logic to perform the required actions as part of command +func (ddo *DeployDeleteOptions) Run() (err error) { err = ddo.DevfileDeployDelete() if err != nil { return err diff --git a/pkg/odo/cli/component/devfile.go b/pkg/odo/cli/component/devfile.go index daab5d276c3..f07abe46ccb 100644 --- a/pkg/odo/cli/component/devfile.go +++ b/pkg/odo/cli/component/devfile.go @@ -117,7 +117,7 @@ func (do *DeployOptions) DevfileDeploy() (err error) { // Apply ignore information err = genericclioptions.ApplyIgnore(&do.ignores, do.sourcePath) if err != nil { - return errors.Wrap(err, "Unable to apply ignore information") + return errors.Wrap(err, uUnable to apply ignore information") } kubeContext := kubernetes.KubernetesContext{ diff --git a/tests/integration/devfile/cmd_devfile_deploy_delete_test.go b/tests/integration/devfile/cmd_devfile_deploy_delete_test.go index 88ce06c228b..734c6b0c31f 100644 --- a/tests/integration/devfile/cmd_devfile_deploy_delete_test.go +++ b/tests/integration/devfile/cmd_devfile_deploy_delete_test.go @@ -1,6 +1,7 @@ package devfile import ( + "fmt" "os" "path/filepath" "time" @@ -12,7 +13,7 @@ import ( ) var _ = Describe("odo devfile deploy delete command tests", func() { - var namespace, context, currentWorkingDirectory, componentName, originalKubeconfig string + var namespace, context, currentWorkingDirectory, componentName, originalKubeconfig, imageTag string // Using program commmand according to cliRunner in devfile cliRunner := helper.GetCliRunner() @@ -29,6 +30,7 @@ var _ = Describe("odo devfile deploy delete command tests", func() { originalKubeconfig = os.Getenv("KUBECONFIG") helper.LocalKubeconfigSet(context) namespace = cliRunner.CreateRandNamespaceProject() + imageTag = fmt.Sprintf("image-registry.openshift-image-registry.svc:5000/%s/my-nodejs:1.0", namespace) currentWorkingDirectory = helper.Getwd() componentName = helper.RandString(6) helper.Chdir(context) @@ -84,7 +86,7 @@ var _ = Describe("odo devfile deploy delete command tests", func() { helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), context) helper.CopyExampleDevFile(filepath.Join("source", "devfilesV2", "nodejs", "devfile.yaml"), filepath.Join(context, "devfile.yaml")) helper.CmdShouldPass("odo", "url", "create", "--port", "3000") - helper.CmdShouldPass("odo", "deploy", "--tag", "image-registry.openshift-image-registry.svc:5000/"+namespace+"/my-nodejs:1.0", "--devfile", "devfile.yaml") + helper.CmdShouldPass("odo", "deploy", "--tag", imageTag) helper.CmdShouldPass("odo", "deploy", "delete") cliRunner.WaitAndCheckForExistence("deployments", namespace, 1) diff --git a/tests/integration/devfile/cmd_devfile_deploy_test.go b/tests/integration/devfile/cmd_devfile_deploy_test.go index 1d9f2a48081..32d2874a4da 100644 --- a/tests/integration/devfile/cmd_devfile_deploy_test.go +++ b/tests/integration/devfile/cmd_devfile_deploy_test.go @@ -50,7 +50,7 @@ var _ = Describe("odo devfile deploy command tests", func() { helper.CopyExampleDevFile(filepath.Join("source", "devfilesV2", "nodejs", "devfile.yaml"), filepath.Join(context, "devfile.yaml")) output := helper.CmdShouldPass("odo", "deploy", "--tag", imageTag) - cliRunner.WaitAndCheckForExistence("BuildConfig", namespace, 1) + cliRunner.WaitAndCheckForExistence("buildconfig", namespace, 1) Expect(output).NotTo(ContainSubstring("does not point to a valid Dockerfile")) Expect(output).To(ContainSubstring("Successfully built image")) Expect(output).To(ContainSubstring("Successfully deployed application")) From e632c0fbd7c8c56964a781bfe922782c60f78443 Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 12:38:34 +0100 Subject: [PATCH 4/6] Fix error typo --- pkg/odo/cli/component/devfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/odo/cli/component/devfile.go b/pkg/odo/cli/component/devfile.go index f07abe46ccb..3bf921c7954 100644 --- a/pkg/odo/cli/component/devfile.go +++ b/pkg/odo/cli/component/devfile.go @@ -117,7 +117,7 @@ func (do *DeployOptions) DevfileDeploy() (err error) { // Apply ignore information err = genericclioptions.ApplyIgnore(&do.ignores, do.sourcePath) if err != nil { - return errors.Wrap(err, uUnable to apply ignore information") + return errors.Wrap(err, "unable to apply ignore information") } kubeContext := kubernetes.KubernetesContext{ From b36d803469de2edd784de050677d4364a52ea81e Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 15:47:40 +0100 Subject: [PATCH 5/6] Cleaning log statements --- .../adapters/kubernetes/component/adapter.go | 24 +++++++++--------- pkg/odo/cli/component/deploy.go | 25 +++++++++---------- pkg/odo/cli/component/devfile.go | 7 +++--- pkg/sync/adapter.go | 2 +- pkg/util/util.go | 13 ---------- 5 files changed, 28 insertions(+), 43 deletions(-) diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go index ab98d965b64..f8aaf71dc79 100644 --- a/pkg/devfile/adapters/kubernetes/component/adapter.go +++ b/pkg/devfile/adapters/kubernetes/component/adapter.go @@ -89,14 +89,13 @@ func (a Adapter) runBuildConfig(client *occlient.Client, parameters common.Build return err } - log.Successf("Started build using BuildConfig") bc, err := client.RunBuildConfigWithBinaryInput(buildName, reader) if err != nil { return err } + log.Successf("Started build %s using BuildConfig", bc.Name) reader, writer := io.Pipe() - s := log.Spinner("Waiting for build to finish") var cmdOutput string // This Go routine will automatically pipe the output from WaitForBuildToFinish to @@ -117,6 +116,7 @@ func (a Adapter) runBuildConfig(client *occlient.Client, parameters common.Build } }() + s := log.Spinner("Waiting for build to complete") if err := client.WaitForBuildToFinish(bc.Name, writer); err != nil { s.End(false) return errors.Wrapf(err, "unable to build image using BuildConfig %s, error: %s", buildName, cmdOutput) @@ -236,8 +236,6 @@ func (a Adapter) Deploy(parameters common.DeployParameters) (err error) { applicationName := a.ComponentName + "-deploy" deploymentManifest := &unstructured.Unstructured{} - log.Info("\nDeploying manifest") - // Specify the substitution keys and values yamlSubstitutions := map[string]string{ "CONTAINER_IMAGE": parameters.Tag, @@ -311,22 +309,22 @@ func (a Adapter) Deploy(parameters common.DeployParameters) (err error) { } } - s := log.Spinnerf("Deploying the manifest for %s", gvk.Kind) + actionType := "Creating" + if instanceFound { + actionType = "Updating" // Update deployment + } + s := log.Spinnerf("%s resource of kind %s", strings.Title(actionType), gvk.Kind) result := &unstructured.Unstructured{} - actionType := "create" if !instanceFound { result, err = a.Client.DynamicClient.Resource(gvr).Namespace(namespace).Create(deploymentManifest, metav1.CreateOptions{}) } else { - actionType = "update" // Update deployment result, err = a.Client.DynamicClient.Resource(gvr).Namespace(namespace).Update(deploymentManifest, metav1.UpdateOptions{}) } if err != nil { s.End(false) return errors.Wrapf(err, "Failed to %s manifest %s", actionType, gvk.Kind) - } else { - s.End(true) - log.Successf("%sd manifest for %s (%s)", strings.Title(actionType), applicationName, gvk.Kind) } + s.End(true) // Write the returned manifest to the local manifest file if writtenToManifest { @@ -370,15 +368,17 @@ func (a Adapter) Deploy(parameters common.DeployParameters) (err error) { knGvr := schema.GroupVersionResource{Group: "serving.knative.dev", Version: "v1", Resource: "routes"} route, err := a.waitForManifestDeployCompletion(applicationName, knGvr, "Ready") if err != nil { + s.End(false) return errors.Wrap(err, "error while waiting for deployment completion") } fullURL = route.UnstructuredContent()["status"].(map[string]interface{})["url"].(string) } - s.End(true) if fullURL != "" { - log.Successf("URL for application %s: %s", applicationName, fullURL) + s.End(true) + log.Successf("Successfully deployed component: %s", fullURL) } else { + s.End(false) log.Errorf("URL unable to be determined for application %s", applicationName) } diff --git a/pkg/odo/cli/component/deploy.go b/pkg/odo/cli/component/deploy.go index 4812fffc567..71b2aeafeeb 100644 --- a/pkg/odo/cli/component/deploy.go +++ b/pkg/odo/cli/component/deploy.go @@ -2,7 +2,6 @@ package component import ( "fmt" - "os" "path/filepath" devfileParser "github.com/openshift/odo/pkg/devfile/parser" @@ -85,39 +84,36 @@ func (do *DeployOptions) Validate() (err error) { return errors.New("odo deploy requires a tag, in the format /namespace>/") } + s := log.Spinner("Validating arguments") err = util.ValidateTag(do.tag) if err != nil { + s.End(false) return err } + s.End(true) do.devObj, err = devfileParser.Parse(do.DevfilePath) if err != nil { return err } + s = log.Spinner("Validating build information") metadata := do.devObj.Data.GetMetadata() dockerfileURL := metadata.Dockerfile - localDir, err := os.Getwd() - if err != nil { - return err - } //Download Dockerfile to .odo, build, then delete from .odo dir //If Dockerfile is present in the project already, use that for the build //If Dockerfile is present in the project and field is in devfile, build the one already in the project and warn the user. - if dockerfileURL != "" && util.CheckPathExists(filepath.Join(localDir, "Dockerfile")) { + if dockerfileURL != "" && util.CheckPathExists(filepath.Join(do.componentContext, "Dockerfile")) { // TODO: make clearer more visible output log.Warning("Dockerfile already exists in project directory and one is specified in Devfile.") log.Warningf("Using Dockerfile specified in devfile from '%s'", dockerfileURL) } - if !util.CheckPathExists(filepath.Join(localDir, ".odo")) { - return errors.Wrap(err, ".odo folder not found") - } - if dockerfileURL != "" { dockerfileBytes, err := util.DownloadFileInMemory(dockerfileURL) if err != nil { + s.End(false) return errors.New("unable to download Dockerfile from URL specified in devfile") } // If we successfully downloaded the Dockerfile into memory, store it in the DeployOptions @@ -126,15 +122,18 @@ func (do *DeployOptions) Validate() (err error) { // Validate the file that was downloaded is a Dockerfile err = util.ValidateDockerfile(dockerfileBytes) if err != nil { + s.End(false) return err } - } else if !util.CheckPathExists(filepath.Join(localDir, "Dockerfile")) { + } else if !util.CheckPathExists(filepath.Join(do.componentContext, "Dockerfile")) { + s.End(false) return errors.New("dockerfile required for build. No 'dockerfile' field found in devfile, or Dockerfile found in project directory") } - // Need to add validation? - s := log.Spinner("Validating Manifest URL") + s.End(true) + + s = log.Spinner("Validating deployment information") manifestURL := metadata.Manifest if manifestURL == "" { s.End(false) diff --git a/pkg/odo/cli/component/devfile.go b/pkg/odo/cli/component/devfile.go index 3bf921c7954..68351b27b5a 100644 --- a/pkg/odo/cli/component/devfile.go +++ b/pkg/odo/cli/component/devfile.go @@ -136,7 +136,7 @@ func (do *DeployOptions) DevfileDeploy() (err error) { EnvSpecificInfo: *do.EnvSpecificInfo, } - log.Infof("\nBuilding devfile component %s", componentName) + log.Infof("\nBuilding component %s", componentName) // Build image for the component err = devfileHandler.Build(buildParams) if err != nil { @@ -147,7 +147,7 @@ func (do *DeployOptions) DevfileDeploy() (err error) { ) os.Exit(1) } - log.Success("Successfully built image") + log.Successf("Successfully built container image: %s", do.tag) deployParams := common.DeployParameters{ EnvSpecificInfo: *do.EnvSpecificInfo, @@ -157,6 +157,7 @@ func (do *DeployOptions) DevfileDeploy() (err error) { warnIfURLSInvalid(do.EnvSpecificInfo.GetURL()) + log.Infof("\nDeploying component %s", componentName) // Deploy the application err = devfileHandler.Deploy(deployParams) if err != nil { @@ -168,8 +169,6 @@ func (do *DeployOptions) DevfileDeploy() (err error) { os.Exit(1) } - log.Successf("Successfully deployed application %s", componentName) - return nil } diff --git a/pkg/sync/adapter.go b/pkg/sync/adapter.go index 27eca821b99..12ccd066f9e 100644 --- a/pkg/sync/adapter.go +++ b/pkg/sync/adapter.go @@ -44,7 +44,7 @@ func (a Adapter) SyncFilesBuild(buildParameters common.BuildParameters, dockerfi var s *log.Status syncFolder := "/" - s = log.Spinner("Checking files for building") + s = log.Spinner("Preparing files for building image") // run the indexer and find the project source files files, err := util.DeployRunIndexer(buildParameters.Path, absIgnoreRules) if len(files) > 0 { diff --git a/pkg/util/util.go b/pkg/util/util.go index a97bdfb01fe..51f68989a10 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -25,7 +25,6 @@ import ( "github.com/gobwas/glob" "github.com/google/go-github/github" - "github.com/openshift/odo/pkg/log" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -1067,9 +1066,7 @@ func ValidateURL(sourceURL string) error { // ValidateDockerfile validates the string passed through has a FROM on it's first non-whitespace/commented line // This function could be expanded to be a more viable linter func ValidateDockerfile(contents []byte) error { - s := log.Spinner("Validating Dockerfile") if len(contents) == 0 { - s.End(false) return errors.New("dockerfile URL provided in the Devfile does not point to an empty file") } // Split the file downloaded line-by-line @@ -1086,23 +1083,18 @@ func ValidateDockerfile(contents []byte) error { continue } if strings.HasPrefix(line, "FROM") { - s.End(true) return nil } - s.End(false) return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") } - s.End(false) // Would only reach this return statement if splitContents is 0 return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") } // ValidateTag validates the string that has been passed as a tag meets the requirements of a tag func ValidateTag(tag string) error { - s := log.Spinner("Validating Tag") var splitTag = strings.Split(tag, "/") if len(splitTag) != 3 { - s.End(false) return errors.New("invalid tag: odo deploy reguires a tag in the format /namespace>/") } @@ -1110,29 +1102,24 @@ func ValidateTag(tag string) error { characterMatch := regexp.MustCompile(`[a-zA-Z0-9\.\-:_]{4,128}`) for _, element := range splitTag { if len(element) < 4 { - s.End(false) return errors.New("invalid tag: " + element + " in the tag is too short. Each element needs to be at least 4 characters.") } if len(element) > 128 { - s.End(false) return errors.New("invalid tag: " + element + " in the tag is too long. Each element cannot be longer than 128.") } // Check that the whole string matches the regular expression // Match.String was returning a match even when only part of the string is working if characterMatch.FindString(element) != element { - s.End(false) return errors.New("invalid tag: " + element + " in the tag contains an illegal character. It must only contain alphanumerical values, periods, colons, underscores, and dashes.") } // The registry, namespace, and image, cannot end in '.', '-', '_',or ':' if strings.HasSuffix(element, ".") || strings.HasSuffix(element, "-") || strings.HasSuffix(element, ":") || strings.HasSuffix(element, "_") { - s.End(false) return errors.New("invalid tag: " + element + " in the tag has an invalid final character. It must end in an alphanumeric value.") } } - s.End(true) return nil } From 21b154cdee5affd364f3c5d50081d1663205638f Mon Sep 17 00:00:00 2001 From: Enrique Lacal Bereslawski Date: Wed, 1 Jul 2020 17:03:49 +0100 Subject: [PATCH 6/6] Steve's Requested Changes --- pkg/devfile/adapters/kubernetes/component/adapter.go | 4 ++-- pkg/util/util.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/devfile/adapters/kubernetes/component/adapter.go b/pkg/devfile/adapters/kubernetes/component/adapter.go index f8aaf71dc79..b47fdef4572 100644 --- a/pkg/devfile/adapters/kubernetes/component/adapter.go +++ b/pkg/devfile/adapters/kubernetes/component/adapter.go @@ -322,7 +322,7 @@ func (a Adapter) Deploy(parameters common.DeployParameters) (err error) { } if err != nil { s.End(false) - return errors.Wrapf(err, "Failed to %s manifest %s", actionType, gvk.Kind) + return errors.Wrapf(err, "Failed when %s manifest %s", actionType, gvk.Kind) } s.End(true) @@ -379,7 +379,7 @@ func (a Adapter) Deploy(parameters common.DeployParameters) (err error) { log.Successf("Successfully deployed component: %s", fullURL) } else { s.End(false) - log.Errorf("URL unable to be determined for application %s", applicationName) + log.Errorf("URL unable to be determined for component %s", a.ComponentName) } return nil diff --git a/pkg/util/util.go b/pkg/util/util.go index 51f68989a10..b49629b3add 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1067,7 +1067,7 @@ func ValidateURL(sourceURL string) error { // This function could be expanded to be a more viable linter func ValidateDockerfile(contents []byte) error { if len(contents) == 0 { - return errors.New("dockerfile URL provided in the Devfile does not point to an empty file") + return errors.New("aplha.build-dockerfile URL provided in the Devfile is referencing an empty file") } // Split the file downloaded line-by-line splitContents := strings.Split(string(contents), "\n") @@ -1085,10 +1085,10 @@ func ValidateDockerfile(contents []byte) error { if strings.HasPrefix(line, "FROM") { return nil } - return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") + return errors.New("dockerfile URL provided in the Devfile does not reference a valid Dockerfile") } // Would only reach this return statement if splitContents is 0 - return errors.New("dockerfile URL provided in the Devfile does not point to a valid Dockerfile") + return errors.New("dockerfile URL provided in the Devfile does not reference a valid Dockerfile") } // ValidateTag validates the string that has been passed as a tag meets the requirements of a tag