diff --git a/docs/dev/experimental-mode.adoc b/docs/dev/experimental-mode.adoc index 6fda622ffcb..7fd563b90f9 100644 --- a/docs/dev/experimental-mode.adoc +++ b/docs/dev/experimental-mode.adoc @@ -123,3 +123,30 @@ $ odo delete myspring The devfile registry can be accessed link:https://github.com/elsony/devfile-registry[here]. For more information on how to develop and write a devfile, please read the link:https://docs.google.com/document/d/1piBG2Zu2PqPZSl0WySj25UouK3X5MKcFKqPrfC9ZAsc[Odo stack creation] document. + +#### Forcing s2i type component creation over devfile type components: + +If there is an s2i type component with the same name as a devfile type component, you can use `--s2i` flag to force the creation of s2i type component over devfile type. +If there is a devfile type component with a given name but no s2i component, `odo create --s2i` will fail. +Also, using flags specific to s2i component creation without using --s2i would also fail the `odo create` command. + +In the above example output of `odo catalog list components` command, you can observe that `nodejs` component type is available in both s2i and devfile categories. + +The following command should create an s2i type component. +``` +$ odo create nodejs mynode --s2i +Experimental mode is enabled, use at your own risk + +Validation + ✓ Validating component [3s] + +Please use `odo push` command to create the component with source deployed +``` + +The following command would fail for using `--s2i` flag as there is no s2i component type available with name "java-spring-boot". +``` +$ odo create java-spring-boot myspring --s2i +Experimental mode is enabled, use at your own risk + + ✗ Cannot select this component with --s2i flag +``` \ No newline at end of file diff --git a/pkg/component/types.go b/pkg/component/types.go index 0c0ad274243..116d6528523 100644 --- a/pkg/component/types.go +++ b/pkg/component/types.go @@ -47,7 +47,7 @@ const ( // StateTypePushed means that Storage is present both locally and on cluster StateTypePushed State = "Pushed" // StateTypeNotPushed means that Storage is only in local config, but not on the cluster - StateTypeNotPushed = "Not Pushed" + StateTypeNotPushed State = "Not Pushed" // StateTypeUnknown means that odo cannot tell its state - StateTypeUnknown = "Unknown" + StateTypeUnknown State = "Unknown" ) diff --git a/pkg/odo/cli/component/create.go b/pkg/odo/cli/component/create.go index 7178e4ffba9..c5379eb22c2 100644 --- a/pkg/odo/cli/component/create.go +++ b/pkg/odo/cli/component/create.go @@ -189,7 +189,7 @@ func (co *CreateOptions) setComponentSourceAttributes() (err error) { // Error out by default if no type of sources were passed.. default: - return fmt.Errorf("The source can be either --binary or --local or --git") + return fmt.Errorf("the source can be either --binary or --local or --git") } @@ -200,7 +200,7 @@ func (co *CreateOptions) setComponentSourceAttributes() (err error) { // Error out if reference is passed but no --git flag passed if len(co.componentGit) == 0 && len(co.componentGitRef) != 0 { - return fmt.Errorf("The --ref flag is only valid for --git flag") + return fmt.Errorf("the --ref flag is only valid for --git flag") } return @@ -284,8 +284,68 @@ func createDefaultComponentName(context *genericclioptions.Context, componentTyp return componentName, nil } +func (co *CreateOptions) checkConflictingFlags() (err error) { + if err = co.checkConflictingS2IFlags(); err != nil { + return + } + + if err = co.checkConflictingDevfileFlags(); err != nil { + return + } + + return nil +} + +func (co *CreateOptions) checkConflictingS2IFlags() error { + if !co.forceS2i { + errorString := "flag --%s, requires --s2i flag to be set, when in experimental mode." + + var flagName string + if co.now { + flagName = "now" + } else if len(co.componentBinary) != 0 { + flagName = "binary" + } else if len(co.componentGit) != 0 { + flagName = "git" + } else if len(co.componentEnvVars) != 0 { + flagName = "env" + } else if len(co.componentPorts) != 0 { + flagName = "port" + } + + if len(flagName) != 0 { + return errors.New(fmt.Sprintf(errorString, flagName)) + } + } + return nil +} + +func (co *CreateOptions) checkConflictingDevfileFlags() error { + if co.forceS2i { + errorString := "you can't set --s2i flag as true if you want to use the %s via --%s flag" + + var flagName string + if len(co.devfileMetadata.devfilePath.value) != 0 { + flagName = "devfile" + } else if len(co.devfileMetadata.devfileRegistry.Name) != 0 { + flagName = "registry" + } else if len(co.devfileMetadata.token) != 0 { + flagName = "token" + } else if len(co.devfileMetadata.starter) != 0 { + flagName = "starter" + } + + if len(flagName) != 0 { + return errors.New(fmt.Sprintf(errorString, flagName, flagName)) + } + } + return nil +} + // Complete completes create args func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string) (err error) { + // this populates the LocalConfigInfo as well + co.Context = genericclioptions.NewContextCreatingAppIfNeeded(cmd) // this populates the LocalConfigInfo as well co.Context = genericclioptions.NewContextCreatingAppIfNeeded(cmd) @@ -297,6 +357,11 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string return } + err = co.checkConflictingFlags() + if err != nil { + return + } + // Configure the context if co.componentContext != "" { DevfilePath = filepath.Join(co.componentContext, devFile) @@ -306,21 +371,30 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string } if util.CheckPathExists(ConfigFilePath) || util.CheckPathExists(EnvFilePath) { - return errors.New("This directory already contains a component") + return errors.New("this directory already contains a component") } if util.CheckPathExists(DevfilePath) && co.devfileMetadata.devfilePath.value != "" && !util.PathEqual(DevfilePath, co.devfileMetadata.devfilePath.value) { - return errors.New("This directory already contains a devfile, you can't specify devfile via --devfile") + return errors.New("this directory already contains a devfile, you can't specify devfile via --devfile") } co.appName = genericclioptions.ResolveAppFlag(cmd) + var catalogList catalog.ComponentTypeList + if co.forceS2i { + client := co.Client + catalogList, err = catalog.ListComponents(client) + if err != nil { + return err + } + } + // Validate user specify devfile path if co.devfileMetadata.devfilePath.value != "" { fileErr := util.ValidateFile(co.devfileMetadata.devfilePath.value) urlErr := util.ValidateURL(co.devfileMetadata.devfilePath.value) if fileErr != nil && urlErr != nil { - return errors.Errorf("The devfile path you specify is invalid with either file error \"%v\" or url error \"%v\"", fileErr, urlErr) + return errors.Errorf("the devfile path you specify is invalid with either file error \"%v\" or url error \"%v\"", fileErr, urlErr) } else if fileErr == nil { co.devfileMetadata.devfilePath.protocol = "file" } else if urlErr == nil { @@ -330,16 +404,17 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string // Validate user specify registry if co.devfileMetadata.devfileRegistry.Name != "" { + if co.devfileMetadata.devfilePath.value != "" { - return errors.New("You can't specify registry via --registry if you want to use the devfile that is specified via --devfile") + return errors.New("you can't specify registry via --registry if you want to use the devfile that is specified via --devfile") } registryList, err := catalog.GetDevfileRegistries(co.devfileMetadata.devfileRegistry.Name) if err != nil { - return errors.Wrap(err, "Failed to get registry") + return errors.Wrap(err, "failed to get registry") } if len(registryList) == 0 { - return errors.Errorf("Registry %s doesn't exist, please specify a valid registry via --registry", co.devfileMetadata.devfileRegistry.Name) + return errors.Errorf("registry %s doesn't exist, please specify a valid registry via --registry", co.devfileMetadata.devfileRegistry.Name) } } @@ -366,7 +441,7 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string var catalogDevfileList catalog.DevfileComponentTypeList isDevfileRegistryPresent := true // defaulted to true since odo ships with a default registry set - if co.interactive { + if co.interactive && !co.forceS2i { // Interactive mode // Get available devfile components for checking devfile compatibility @@ -404,8 +479,12 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string if util.CheckPathExists(DevfilePath) || co.devfileMetadata.devfilePath.value != "" { // Use existing devfile directly + if co.forceS2i { + return errors.Errorf("existing devfile component detected. Please remove the devfile component before creating an s2i component") + } + if len(args) > 1 { - return errors.Errorf("Accepts between 0 and 1 arg when using existing devfile, received %d", len(args)) + return errors.Errorf("accepts between 0 and 1 arg when using existing devfile, received %d", len(args)) } // If user can use existing devfile directly, the first arg is component name instead of component type @@ -421,7 +500,7 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string } co.devfileMetadata.devfileSupport = true - } else { + } else if len(args) > 0 { // Download devfile from registry // Component type: Get from full command's first argument (mandatory in direct mode) @@ -440,7 +519,7 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string return err } if co.devfileMetadata.devfileRegistry.Name != "" && catalogDevfileList.Items == nil { - return errors.Errorf("Can't create devfile component from registry %s", co.devfileMetadata.devfileRegistry.Name) + return errors.Errorf("can't create devfile component from registry %s", co.devfileMetadata.devfileRegistry.Name) } if len(catalogDevfileList.DevfileRegistries) == 0 { @@ -499,7 +578,7 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string return nil } - if isDevfileRegistryPresent { + if isDevfileRegistryPresent && !co.forceS2i { // Categorize the sections log.Info("Validation") @@ -519,6 +598,19 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string } } + if co.forceS2i && hasComponent { + s2iOverride := false + for _, item := range catalogList.Items { + if item.Name == co.devfileMetadata.componentType { + s2iOverride = true + break + } + } + if !s2iOverride { + return errors.New("cannot select this devfile component type with --s2i flag") + } + } + existSpinner := log.Spinner("Checking devfile existence") if hasComponent { existSpinner.End(true) @@ -546,9 +638,9 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string // we should error out instead of running s2i componet code and throw warning message if co.devfileMetadata.devfileRegistry.Name != "" { return errors.Errorf("Devfile component type %s is not supported, please run `odo catalog list components` for a list of supported devfile component types", co.devfileMetadata.componentType) - } else { - log.Warningf("Devfile component type %s is not supported, please run `odo catalog list components` for a list of supported devfile component types", co.devfileMetadata.componentType) } + + log.Warningf("Devfile component type %s is not supported, please run `odo catalog list components` for a list of supported devfile component types", co.devfileMetadata.componentType) } } @@ -556,12 +648,6 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string co.interactive = true } - // this populates the LocalConfigInfo as well - co.Context = genericclioptions.NewContextCreatingAppIfNeeded(cmd) - if err != nil { - return errors.Wrap(err, "failed initiating local config") - } - // Do not execute S2I specific code on Kubernetes Cluster or Docker // return from here, if it is not an openshift cluster. var openshiftCluster bool @@ -582,8 +668,6 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string co.componentSettings = co.LocalConfigInfo.GetComponentSettings() - co.Context = genericclioptions.NewContextCreatingAppIfNeeded(cmd) - // Below code is for INTERACTIVE mode if co.interactive { client := co.Client diff --git a/pkg/odo/cli/component/push.go b/pkg/odo/cli/component/push.go index 5470779188d..864689de7a4 100644 --- a/pkg/odo/cli/component/push.go +++ b/pkg/odo/cli/component/push.go @@ -210,10 +210,10 @@ func (po *PushOptions) Run() (err error) { if util.CheckPathExists(po.DevfilePath) { // Return Devfile push return po.DevfilePush() - } else { - // Legacy odo push - return po.Push() } + + // Legacy odo push + return po.Push() } // NewCmdPush implements the push odo command diff --git a/tests/integration/component.go b/tests/integration/component.go index 2e295a851c3..1f60a14a9ee 100644 --- a/tests/integration/component.go +++ b/tests/integration/component.go @@ -116,7 +116,7 @@ func componentTests(args ...string) { It("should show an error when ref flag is provided with sources except git", func() { outputErr := helper.CmdShouldFail("odo", append(args, "create", "nodejs", "--project", project, "cmp-git", "--ref", "test")...) - Expect(outputErr).To(ContainSubstring("The --ref flag is only valid for --git flag")) + Expect(outputErr).To(ContainSubstring("the --ref flag is only valid for --git flag")) }) It("create component twice fails from same directory", func() { diff --git a/tests/integration/devfile/cmd_devfile_catalog_test.go b/tests/integration/devfile/cmd_devfile_catalog_test.go index f4c81defa94..f3a466d05c8 100644 --- a/tests/integration/devfile/cmd_devfile_catalog_test.go +++ b/tests/integration/devfile/cmd_devfile_catalog_test.go @@ -1,6 +1,7 @@ package devfile import ( + "encoding/json" "os" "path/filepath" "time" @@ -106,7 +107,7 @@ var _ = Describe("odo devfile catalog command tests", func() { output := helper.CmdShouldPass("odo", "registry", "list") helper.MatchAllInOutput(output, []string{registryName, addRegistryURL}) output = helper.CmdShouldPass("odo", "catalog", "describe", "component", "nodejs") - helper.MatchAllInOutput(output, []string{"name: nodejs-starter", "Registry: DefaultDevfileRegistry", "Registry: " + registryName}) + helper.MatchAllInOutput(output, []string{"name: nodejs-starter", "Registry: " + registryName}) }) }) Context("When executing catalog describe component with a component name that does not have a devfile component", func() { @@ -127,4 +128,38 @@ var _ = Describe("odo devfile catalog command tests", func() { helper.MatchAllInOutput(output, []string{"accepts 1 arg(s), received 0"}) }) }) + + Context("When executing catalog list components with experimental mode set to true", func() { + + componentName := "nodejs" + + It("should prove that nodejs is present in both S2I Component list and Devfile Component list", func() { + + output := helper.CmdShouldPass("odo", "catalog", "list", "components", "-o", "json") + + wantOutput := []string{componentName} + + var data map[string]interface{} + + err := json.Unmarshal([]byte(output), &data) + + if err != nil { + Expect(err).Should(BeNil()) + } + outputBytes, err := json.Marshal(data["s2iItems"]) + if err == nil { + output = string(outputBytes) + } + + helper.MatchAllInOutput(output, wantOutput) + + outputBytes, err = json.Marshal(data["devfileItems"]) + if err == nil { + output = string(outputBytes) + } + + helper.MatchAllInOutput(output, wantOutput) + }) + }) + }) diff --git a/tests/integration/devfile/cmd_devfile_create_test.go b/tests/integration/devfile/cmd_devfile_create_test.go index eb7032f32ca..07281bbfc9d 100644 --- a/tests/integration/devfile/cmd_devfile_create_test.go +++ b/tests/integration/devfile/cmd_devfile_create_test.go @@ -1,6 +1,7 @@ package devfile import ( + "encoding/json" "os" "path" "path/filepath" @@ -71,6 +72,22 @@ var _ = Describe("odo devfile create command tests", func() { }) }) + Context("Disabling experimental preference should error out on providing --s2i flag", func() { + JustBeforeEach(func() { + if os.Getenv("KUBERNETES") == "true" { + Skip("Skipping test because s2i image is not supported on Kubernetes cluster") + } + }) + It("checks that the --s2i flag is unrecognised when Experimental is set to false for create", func() { + helper.CmdShouldPass("odo", "preference", "set", "Experimental", "false", "-f") + helper.CopyExample(filepath.Join("source", "nodejs"), context) + + // Check that it will contain the experimental mode output + s2iFlagUnknownMsg := "Error: unknown flag: --s2i" + Expect(helper.CmdShouldFail("odo", "create", "nodejs", "--s2i")).To(ContainSubstring(s2iFlagUnknownMsg)) + }) + }) + Context("When executing odo create with devfile component type argument", func() { It("should successfully create the devfile component", func() { helper.CmdShouldPass("odo", "create", "java-openliberty") @@ -113,6 +130,91 @@ var _ = Describe("odo devfile create command tests", func() { }) }) + Context("When executing odo create with component type argument and --s2i flag", func() { + + JustBeforeEach(func() { + if os.Getenv("KUBERNETES") == "true" { + Skip("Skipping test because s2i image is not supported on Kubernetes cluster") + } + }) + + componentType := "nodejs" + + It("should successfully create the localconfig component", func() { + componentName := helper.RandString(6) + helper.CopyExample(filepath.Join("source", componentType), context) + helper.CmdShouldPass("odo", "create", componentType, componentName, "--s2i") + helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,"+componentName, "Application,app") + helper.CmdShouldPass("odo", "push", "--context", context, "-v4") + + // clean up + helper.CmdShouldPass("odo", "app", "delete", "app", "-f") + helper.CmdShouldFail("odo", "app", "delete", "app", "-f") + helper.CmdShouldFail("odo", "delete", componentName, "-f") + + }) + + It("should successfully create the localconfig component with --git flag", func() { + componentName := "cmp-git" + helper.CmdShouldPass("odo", "create", componentType, "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--s2i", "true", "--app", "testing") + helper.CmdShouldPass("odo", "push", "--context", context, "-v4") + + // clean up + helper.CmdShouldPass("odo", "app", "delete", "testing", "-f") + helper.CmdShouldFail("odo", "app", "delete", "testing", "-f") + helper.CmdShouldFail("odo", "delete", componentName, "-f") + }) + + It("should fail to create the devfile component which doesn't have an s2i component of same name", func() { + componentName := helper.RandString(6) + + output := helper.CmdShouldPass("odo", "catalog", "list", "components", "-o", "json") + + wantOutput := []string{"java-openliberty"} + + var data map[string]interface{} + + err := json.Unmarshal([]byte(output), &data) + + if err != nil { + Expect(err).Should(BeNil()) + } + outputBytes, err := json.Marshal(data["s2iItems"]) + if err == nil { + output = string(outputBytes) + } + + helper.DontMatchAllInOutput(output, wantOutput) + + outputBytes, err = json.Marshal(data["devfileItems"]) + if err == nil { + output = string(outputBytes) + } + + helper.MatchAllInOutput(output, wantOutput) + + helper.CmdShouldFail("odo", "create", "java-openliberty", componentName, "--s2i") + }) + + It("should fail the create command as --git flag, which is specific to s2i component creation, is used without --s2i flag", func() { + output := helper.CmdShouldFail("odo", "create", "nodejs", "cmp-git", "--git", "https://github.com/openshift/nodejs-ex", "--context", context, "--app", "testing") + Expect(output).Should(ContainSubstring("flag --git, requires --s2i flag to be set, when in experimental mode.")) + }) + + It("should fail the create command as --binary flag, which is specific to s2i component creation, is used without --s2i flag", func() { + helper.CopyExample(filepath.Join("binary", "java", "openjdk"), context) + + output := helper.CmdShouldFail("odo", "create", "java:8", "sb-jar-test", "--binary", filepath.Join(context, "sb.jar"), "--context", context) + Expect(output).Should(ContainSubstring("flag --binary, requires --s2i flag to be set, when in experimental mode.")) + }) + + It("should fail the create command as --now flag, which is specific to s2i component creation, is used without --s2i flag", func() { + componentName := helper.RandString(6) + output := helper.CmdShouldFail("odo", "create", "nodejs", componentName, "--now") + Expect(output).Should(ContainSubstring("flag --now, requires --s2i flag to be set, when in experimental mode.")) + }) + }) + Context("When executing odo create with devfile component type argument and --project flag", func() { It("should successfully create the devfile component", func() { componentNamespace := helper.RandString(6) @@ -129,7 +231,7 @@ var _ = Describe("odo devfile create command tests", func() { It("should fail to create the devfile component if specified registry is invalid", func() { componentRegistry := "fake" output := helper.CmdShouldFail("odo", "create", "java-openliberty", "--registry", componentRegistry) - helper.MatchAllInOutput(output, []string{"Registry fake doesn't exist, please specify a valid registry via --registry"}) + helper.MatchAllInOutput(output, []string{"registry fake doesn't exist, please specify a valid registry via --registry"}) }) })