Fix --namespace flag bug for odo create #2797

GeekArthur · 2020-04-01T15:49:37Z

Signed-off-by: jingfu wang jingfu.j.wang@ibm.com

What type of PR is this?
/kind bug

What does does this PR do / why we need it:
odo create with --namespace doesn't work properly, this PR fix the issue.

Which issue(s) this PR fixes:
Fixes #2796

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-04-01T15:49:55Z

/assign

GeekArthur · 2020-04-01T15:54:19Z

@johnmcollier @kadel Can you please review this PR? Thanks

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

codecov · 2020-04-01T18:06:16Z

Codecov Report

Merging #2797 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2797   +/-   ##
=======================================
  Coverage   43.51%   43.51%           
=======================================
  Files          91       91           
  Lines        8312     8312           
=======================================
  Hits         3617     3617           
  Misses       4346     4346           
  Partials      349      349

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 918c903...d9ec8a8. Read the comment docs.

GeekArthur · 2020-04-01T19:39:58Z

/retest

kadel · 2020-04-02T12:18:29Z

/approve

amitkrout

@GeekArthur Can i expect an integration test scenario

GeekArthur · 2020-04-02T16:05:59Z

@amitkrout Yup, I have the same thoughts before, but I hesitate as it may create merge conflict with this PR #2771, that being said, let me push one more commit to add the integration test

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-04-02T16:21:25Z

@amitkrout I added the integration tests, please review again, thanks!

amitkrout

@GeekArthur Thanks for adding integration test.

However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.

other observation like

$ odo create openLiberty --namespace test123
 ✓  Checking if the specified component type is supported devfile component type [180272ns]
 ✓  Validating component [102178ns]
Please use `odo push` command to create the component with source deployed

$ tree -a .
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
2 directories, 2 files

The same above structure is being observed with our regular odo command odo create nodejs --project test123. Seems it has conflict with regular odo component config.yaml as well as the component dir structure.

Ping @kadel @girishramnani

amitkrout · 2020-04-03T11:00:14Z

@GeekArthur Please rebase.
tests/integration/cmd_devfile_create_test.go has been moved to tests/integration/devfile/cmd_devfile_create_test.go

kadel · 2020-04-03T14:53:29Z

However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.

@amitkrout what do you mean? Both --project and --namespace are there

▶ ODO_EXPERIMENTAL=false odo create --help

Create a configuration describing a component.

 If a component name is not provided, it'll be auto-generated.

 A full list of component types that can be deployed is available using: 'odo catalog list'

 By default, builder images (component type) will be used from the current namespace. You can explicitly supply a namespace by using: odo create namespace/name:version If version is not specified by default, latest will be chosen as the version.

Usage:
  odo create <component_type> [component_name] [flags]

Examples:
  # Create new Node.js component with the source in current directory.
  odo create nodejs

  # Create new Node.js component named 'frontend' with the source in './frontend' directory
  odo create nodejs frontend --context ./frontend

  # Create new Java component with binary named sample.jar in './target' directory
  odo create java:8  --binary target/sample.jar

  # Create new Node.js component with source from remote git repository
  odo create 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
  odo create nodejs --port 8080,8100/tcp,9100/udp --env key=value,key1=value1 --memory 4Gi --cpu 2

Flags:
      --app string          Application, defaults to active application
  -b, --binary string       Create a binary file component component using given artifact. Works only with Java components. File needs to be in the context directory.
      --context string      Use given context directory as a source for component settings
      --cpu string          Amount of cpu to be allocated to the component. ex. 100m or 0.1 (sets min-cpu and max-cpu to this value)
      --env stringSlice     Environmental variables for the component. For example --env VariableName=Value
  -g, --git string          Create a git component using this repository.
  -h, --help                Help for create
      --max-cpu string      Limit maximum amount of cpu to be allocated to the component. ex. 1
      --max-memory string   Limit maximum amount of memory to be allocated to the component. ex. 100Mi
      --memory string       Amount of memory to be allocated to the component. ex. 100Mi (sets min-memory and max-memory to this value)
      --min-cpu string      Limit minimum amount of cpu to be allocated to the component. ex. 100m
      --min-memory string   Limit minimum amount of memory to be allocated to the component. ex. 100Mi
  -n, --namespace string    Create devfile component under specific namespace
      --now                 Push changes to the cluster immediately
  -p, --port stringSlice    Ports to be used when the component is created (ex. 8080,8100/tcp,9100/udp)
      --project string      Project, defaults to active project
  -r, --ref string          Use a specific ref e.g. commit, branch or tag of the git repository

Additional Flags:
  -v, --v Level              Log level for V logs. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

…ateFlag

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-04-03T15:40:17Z

@GeekArthur Thanks for adding integration test.

However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.

other observation like
$ odo create openLiberty --namespace test123
 ✓  Checking if the specified component type is supported devfile component type [180272ns]
 ✓  Validating component [102178ns]
Please use `odo push` command to create the component with source deployed

$ tree -a .
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
2 directories, 2 files
The same above structure is being observed with our regular odo command odo create nodejs --project test123. Seems it has conflict with regular odo component config.yaml as well as the component dir structure.

Ping @kadel @girishramnani

@amitkrout We have both --project and --namespace flags in odo create -h as @kadel shows above, can you explain more about the concern?

Actually this file system structure

├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml

is there for a long time, it shares the .odo folder with s2i component but I think they are mutually exclusive since we have the validation code to ensure they can't be created at the same

@amitkrout Also integration tests are moved, please review again

amitkrout · 2020-04-04T09:15:11Z

However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.

@amitkrout what do you mean? Both --project and --namespace are there

▶ ODO_EXPERIMENTAL=false odo create --help

Create a configuration describing a component.

 If a component name is not provided, it'll be auto-generated.

 A full list of component types that can be deployed is available using: 'odo catalog list'

 By default, builder images (component type) will be used from the current namespace. You can explicitly supply a namespace by using: odo create namespace/name:version If version is not specified by default, latest will be chosen as the version.

Usage:
  odo create <component_type> [component_name] [flags]

Examples:
  # Create new Node.js component with the source in current directory.
  odo create nodejs

  # Create new Node.js component named 'frontend' with the source in './frontend' directory
  odo create nodejs frontend --context ./frontend

  # Create new Java component with binary named sample.jar in './target' directory
  odo create java:8  --binary target/sample.jar

  # Create new Node.js component with source from remote git repository
  odo create 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
  odo create nodejs --port 8080,8100/tcp,9100/udp --env key=value,key1=value1 --memory 4Gi --cpu 2

Flags:
      --app string          Application, defaults to active application
  -b, --binary string       Create a binary file component component using given artifact. Works only with Java components. File needs to be in the context directory.
      --context string      Use given context directory as a source for component settings
      --cpu string          Amount of cpu to be allocated to the component. ex. 100m or 0.1 (sets min-cpu and max-cpu to this value)
      --env stringSlice     Environmental variables for the component. For example --env VariableName=Value
  -g, --git string          Create a git component using this repository.
  -h, --help                Help for create
      --max-cpu string      Limit maximum amount of cpu to be allocated to the component. ex. 1
      --max-memory string   Limit maximum amount of memory to be allocated to the component. ex. 100Mi
      --memory string       Amount of memory to be allocated to the component. ex. 100Mi (sets min-memory and max-memory to this value)
      --min-cpu string      Limit minimum amount of cpu to be allocated to the component. ex. 100m
      --min-memory string   Limit minimum amount of memory to be allocated to the component. ex. 100Mi
  -n, --namespace string    Create devfile component under specific namespace
      --now                 Push changes to the cluster immediately
  -p, --port stringSlice    Ports to be used when the component is created (ex. 8080,8100/tcp,9100/udp)
      --project string      Project, defaults to active project
  -r, --ref string          Use a specific ref e.g. commit, branch or tag of the git repository

Additional Flags:
  -v, --v Level              Log level for V logs. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

Ahh, my bad. Actually i was searching in the example section. As this is a key flag for creation of odo devfile component, so thought there should be an example for better user experience. I have created a separate issue #2812 to track devfile UX issues.

@kadel As per the command you executed ODO_EXPERIMENTAL=false odo create --help if you are seeing --namespace flag then there is problem because namespace flag should not be displayed when experimental is disable. Can you please cross verify the command again.

Ideally ODO_EXPERIMENTAL=true odo create --help should be the command to see --namespace flag in UX.

amitkrout · 2020-04-04T09:16:49Z

prow issue
/test v4.1-integration-e2e-benchmark

amitkrout · 2020-04-04T12:49:10Z

@GeekArthur Thanks for adding integration test.
However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.
other observation like
$ odo create openLiberty --namespace test123
 ✓  Checking if the specified component type is supported devfile component type [180272ns]
 ✓  Validating component [102178ns]
Please use `odo push` command to create the component with source deployed

$ tree -a .
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
2 directories, 2 files
The same above structure is being observed with our regular odo command odo create nodejs --project test123. Seems it has conflict with regular odo component config.yaml as well as the component dir structure.
Ping @kadel @girishramnani
@amitkrout We have both --project and --namespace flags in odo create -h as @kadel shows above, can you explain more about the concern?

Actually this file system structure
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
is there for a long time, it shares the .odo folder with s2i component but I think they are mutually exclusive since we have the validation code to ensure they can't be created at the same

@GeekArthur Let me put in this way

I just enable experimental along with regular nodejs odo create command

$ ODO_EXPERIMENTAL=true odo create nodejs --context ../tests/examples/source/nodejs/
 ✓  Checking if the specified component type is supported devfile component type [164155ns]
 ✓  Validating component [67410ns]

Please use `odo push` command to create the component with source deployed

Expectation:
Nodejs component should be created along with component dir in context dir

$ ls -a ../tests/examples/source/nodejs/
.               ..              package.json    server.js

No component dir i mean .odo not found in context dir

Actual:

All the component configuration found the the same dir from where i launches the command

$ tree -a 
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml

Why a this configuration files are created for regular odo component create command ?

Seems regular java component works but not properly

$ ODO_EXPERIMENTAL=true odo create java --context ../tests/examples/source/openjdk/
 ✗  Checking if the specified component type is supported devfile component type [60904ns]   <-- Why its checking for dev file support ?

Please run 'odo catalog list components' for a list of supported devfile component types
 ✓  Validating component [655ms]

Please use `odo push` command to create the component with source deployed

$ ls -a ../tests/examples/source/openjdk/
.               ..          src.   .odo      pom.xml

May be you need to handle devfile component create just by passing devfile subcommand. For example

odo create devfile <component> --namespace test

So that it will be easy to distinguish between devfile and regular command.

amitkrout · 2020-04-04T13:34:21Z

@GeekArthur Thanks for adding integration test.
However i can see missing thing like the command is not updated in UX, i mean odo create -h has nothing to show for this command.
other observation like
$ odo create openLiberty --namespace test123
 ✓  Checking if the specified component type is supported devfile component type [180272ns]
 ✓  Validating component [102178ns]
Please use `odo push` command to create the component with source deployed

$ tree -a .
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
2 directories, 2 files
The same above structure is being observed with our regular odo command odo create nodejs --project test123. Seems it has conflict with regular odo component config.yaml as well as the component dir structure.
Ping @kadel @girishramnani
@amitkrout We have both --project and --namespace flags in odo create -h as @kadel shows above, can you explain more about the concern?
Actually this file system structure
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
is there for a long time, it shares the .odo folder with s2i component but I think they are mutually exclusive since we have the validation code to ensure they can't be created at the same
@GeekArthur Let me put in this way

I just enable experimental along with regular nodejs odo create command
$ ODO_EXPERIMENTAL=true odo create nodejs --context ../tests/examples/source/nodejs/
 ✓  Checking if the specified component type is supported devfile component type [164155ns]
 ✓  Validating component [67410ns]

Please use `odo push` command to create the component with source deployed
Expectation:
Nodejs component should be created along with component dir in context dir
$ ls -a ../tests/examples/source/nodejs/
.               ..              package.json    server.js
No component dir i mean .odo not found in context dir

Actual:

All the component configuration found the the same dir from where i launches the command
$ tree -a 
.
├── .odo
│   └── env
│       └── env.yaml
└── devfile.yaml
Why a this configuration files are created for regular odo component create command ?

Seems regular java component works but not properly
$ ODO_EXPERIMENTAL=true odo create java --context ../tests/examples/source/openjdk/
 ✗  Checking if the specified component type is supported devfile component type [60904ns]   <-- Why its checking for dev file support ?

Please run 'odo catalog list components' for a list of supported devfile component types
 ✓  Validating component [655ms]

Please use `odo push` command to create the component with source deployed

$ ls -a ../tests/examples/source/openjdk/
.               ..          src.   .odo      pom.xml
May be you need to handle devfile component create just by passing devfile subcommand. For example
odo create devfile <component> --namespace test
So that it will be easy to distinguish between devfile and regular command.

Found the same issue on master aswell. Issue created - #2813

amitkrout · 2020-04-05T02:42:19Z

prow issue
/test v4.1-integration-e2e-benchmark

kadel · 2020-04-06T13:22:02Z

/retest

GeekArthur · 2020-04-06T15:44:16Z

@amitkrout I see what you mean, thanks for your explanations and the two issues you open. I can open separate PRs to resolve the two issues as they are separate issues (this PR only fixes the --namespace issue)

To summarize the expected implementations of two issues here:

Devfile UX improvements with experimental mode enabled (#2812)

Add examples to the help page
Remove --namespace when experimental mode is disabled

Directory structure issue with experimental mode enabled (#2813)

I can describe the current design and answer your question under the issue and we can discuss there directly

@amitkrout Given this issue is only fixes the --namespace and the fix isn't directly related with the two issues above, can you review/approve this PR only again? Thanks

GeekArthur · 2020-04-06T17:02:52Z

@amitkrout @kadel
Just confirm the --namespace won't be displayed with experimental mode disabled because of this line: https://github.com/openshift/odo/blob/master/pkg/odo/cli/component/create.go#L721, so just need to add examples to the help page.

amitkrout · 2020-04-07T09:07:17Z

/lgtm

kadel · 2020-04-07T09:36:01Z

/approve

openshift-ci-robot · 2020-04-07T09:36:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kadel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

amitkrout · 2020-04-07T09:59:12Z

/refresh

Fix --namespace flag bug for odo create

e13a31f

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 1, 2020

openshift-ci-robot assigned GeekArthur Apr 1, 2020

openshift-ci-robot requested review from mik-dass and prietyc123 April 1, 2020 15:50

Validate namespace

0443584

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Apr 2, 2020

amitkrout suggested changes Apr 2, 2020

View reviewed changes

openshift-ci-robot assigned amitkrout Apr 2, 2020

Add integration test

833bacb

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

amitkrout reviewed Apr 3, 2020

View reviewed changes

GeekArthur added 3 commits April 3, 2020 11:20

Merge branch 'master' of github.com:openshift/odo into issue-2796-cre…

44583a9

…ateFlag

Rebase

12806e8

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

Move integration tests

d9ec8a8

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 7, 2020

openshift-merge-robot merged commit 23e9df5 into redhat-developer:master Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix --namespace flag bug for odo create #2797

Fix --namespace flag bug for odo create #2797

GeekArthur commented Apr 1, 2020 •

edited

GeekArthur commented Apr 1, 2020

GeekArthur commented Apr 1, 2020

codecov bot commented Apr 1, 2020 •

edited

GeekArthur commented Apr 1, 2020

kadel commented Apr 2, 2020

amitkrout left a comment •

edited

GeekArthur commented Apr 2, 2020

GeekArthur commented Apr 2, 2020

amitkrout left a comment •

edited

amitkrout commented Apr 3, 2020

kadel commented Apr 3, 2020

GeekArthur commented Apr 3, 2020

amitkrout commented Apr 4, 2020

amitkrout commented Apr 4, 2020

amitkrout commented Apr 4, 2020 •

edited

amitkrout commented Apr 4, 2020

amitkrout commented Apr 5, 2020

kadel commented Apr 6, 2020

GeekArthur commented Apr 6, 2020

GeekArthur commented Apr 6, 2020 •

edited

amitkrout commented Apr 7, 2020

kadel commented Apr 7, 2020

openshift-ci-robot commented Apr 7, 2020

amitkrout commented Apr 7, 2020

Fix --namespace flag bug for odo create #2797

Fix --namespace flag bug for odo create #2797

Conversation

GeekArthur commented Apr 1, 2020 • edited

GeekArthur commented Apr 1, 2020

GeekArthur commented Apr 1, 2020

codecov bot commented Apr 1, 2020 • edited

Codecov Report

GeekArthur commented Apr 1, 2020

kadel commented Apr 2, 2020

amitkrout left a comment • edited

Choose a reason for hiding this comment

GeekArthur commented Apr 2, 2020

GeekArthur commented Apr 2, 2020

amitkrout left a comment • edited

Choose a reason for hiding this comment

amitkrout commented Apr 3, 2020

kadel commented Apr 3, 2020

GeekArthur commented Apr 3, 2020

amitkrout commented Apr 4, 2020

amitkrout commented Apr 4, 2020

amitkrout commented Apr 4, 2020 • edited

amitkrout commented Apr 4, 2020

amitkrout commented Apr 5, 2020

kadel commented Apr 6, 2020

GeekArthur commented Apr 6, 2020

GeekArthur commented Apr 6, 2020 • edited

amitkrout commented Apr 7, 2020

kadel commented Apr 7, 2020

openshift-ci-robot commented Apr 7, 2020

amitkrout commented Apr 7, 2020

GeekArthur commented Apr 1, 2020 •

edited

codecov bot commented Apr 1, 2020 •

edited

amitkrout left a comment •

edited

amitkrout left a comment •

edited

amitkrout commented Apr 4, 2020 •

edited

GeekArthur commented Apr 6, 2020 •

edited