Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

odo create support existing devfile.yaml #2798

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 30 additions & 8 deletions pkg/odo/cli/component/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,11 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string
supDevfileCatalogList = append(supDevfileCatalogList, devfileComponent)
}
}
componentType = ui.SelectDevfileComponentType(supDevfileCatalogList)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as [1] is the first check, if a <component-type not present and devfile.yaml is present, it is default going into interactive, IIUC it should create from existing devfile.yaml
[1]

if len(args) == 0 {
  co.interactive = true
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should create the component using the existing devfile.yaml, but user is still able to specify the <component name> via interactive mode, that's why it falls down to interactive mode

// devfile.yaml is not present, user has to specify the component type
if !util.CheckPathExists(DevfilePath) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move this check before listing from registry, as for the case devfile is present it might save one call.

Copy link
Contributor Author

@GeekArthur GeekArthur Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice suggestion! I can do that to improve the performance.

componentType = ui.SelectDevfileComponentType(supDevfileCatalogList)
}

componentName = ui.EnterDevfileComponentName(componentType)

Expand All @@ -338,6 +342,10 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string
// Component type: Get from full command's first argument (mandatory)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the comments at the right place, above componentType = args[0]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I can refactor the comments for both interactive mode and direct mode

// Component name: Get from full command's second argument (optional), by default it is component type from first argument
// Component namespace: Get from --project flag, by default it is the current active namespace
if util.CheckPathExists(DevfilePath) {
return errors.New("This workspace directory already contains a devfile.yaml, please delete it and run the component creation command again")
}

componentType = args[0]

if len(args) == 2 {
Expand All @@ -361,9 +369,21 @@ func (co *CreateOptions) Complete(name string, cmd *cobra.Command, args []string
co.devfileMetadata.componentName = strings.ToLower(componentName)
co.devfileMetadata.componentNamespace = componentNamespace

// If devfile.yaml is present, we don't need to download the devfile.yaml later
if util.CheckPathExists(DevfilePath) {
co.devfileMetadata.devfileSupport = true

err = co.InitEnvInfoFromContext()
if err != nil {
return err
}

return nil
}

// Since we need to support both devfile and s2i, so we have to check if the component type is
// supported by devfile, if it is supported we return, but if it is not supported we still need to
// run all codes related with s2i
// supported by devfile, if it is supported we return and will download the corresponding devfile.yaml later,
// but if it is not supported we still need to run all codes related with s2i
spinner := log.Spinner("Checking if the specified component type is supported devfile component type")

for _, devfileComponent := range catalogDevfileList.Items {
Expand Down Expand Up @@ -585,8 +605,8 @@ func (co *CreateOptions) Validate() (err error) {
spinner := log.Spinner("Validating component")
defer spinner.End(false)

if util.CheckPathExists(DevfilePath) || util.CheckPathExists(EnvFilePath) {
return errors.New("Devfile.yaml or env.yaml already exists")
if util.CheckPathExists(EnvFilePath) {
return errors.New("This workspace directory already contains a devfile component")
}

err = util.ValidateK8sResourceName("component name", co.devfileMetadata.componentName)
Expand Down Expand Up @@ -629,9 +649,11 @@ func (co *CreateOptions) Run() (err error) {
return errors.Wrap(err, "Failed to create env.yaml for devfile component")
}

err = util.DownloadFile(co.devfileMetadata.devfileRegistry+co.devfileMetadata.devfileLink, DevfilePath)
if err != nil {
return errors.Wrap(err, "Failed to download devfile.yaml for devfile component")
if !util.CheckPathExists(DevfilePath) {
err = util.DownloadFile(co.devfileMetadata.devfileRegistry+co.devfileMetadata.devfileLink, DevfilePath)
if err != nil {
return errors.Wrap(err, "Failed to download devfile.yaml for devfile component")
}
}

log.Italic("\nPlease use `odo push` command to create the component with source deployed")
Expand Down