odo create namespace #5724

valaparthvi · 2022-05-09T13:43:04Z

What type of PR is this:
/kind feature

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes part of #5525

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:

odo create namespace my-namespace
odo create project my-project
odo create
odo create namespace --help

Note: The main logic of this PR comes from https://github.com/valaparthvi/odo/blob/main/pkg/odo/cli/project/create.go.

netlify · 2022-05-09T13:43:11Z

✅ Deploy Preview for odo-docusaurus-preview ready!

Name	Link
🔨 Latest commit	`abd914b`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/627d089cbe68e600088fb47c
😎 Deploy Preview	https://deploy-preview-5724--odo-docusaurus-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

odo-robot · 2022-05-09T13:47:50Z

OpenShift Tests on commit e61ce4d finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-09T13:50:38Z

Unit Tests on commit e61ce4d finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-09T13:52:22Z

Windows Tests (OCP) on commit e61ce4d finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-09T13:56:09Z

Kubernetes Tests on commit e61ce4d finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-09T13:59:08Z

Validate Tests on commit e61ce4d finished successfully.
View logs: TXT HTML

feloy · 2022-05-09T14:07:51Z

docs/website/versioned_docs/version-3.0.0/command-reference/odo-create-namespace.md

+sidebar_position: 3
+---
+
+`odo create namespace` lets you create a namespace/project on your cluster.


Please explain that it will create a Project resource when the resource type exists in the cluster (generally in OpenShift clusters), or a Namespace resource otherwise

feloy · 2022-05-09T14:09:41Z

docs/website/versioned_docs/version-3.0.0/command-reference/odo-create-namespace.md

+ ✓  New namespace created and now using namespace: mynamespace
+```
+
+Optionally, you can also use `project` as an alias to `namespace`.


Please explain that using Project or Namespace won't change the resource created in the cluster: only the existence of the Project resource type in the cluster will.

feloy · 2022-05-09T14:10:59Z

tests/integration/cmd_namespace_test.go

+	})
+	It("should fail when an existent namespace is created again", func() {
+		helper.Cmd("odo", "create", "project", commonVar.Project).ShouldFail()
+	})


Could you add specific tests for OpenShift/Kubernetes to check the created resource? A project should have been created on OpenShift, but not on Kubernetes.

feloy · 2022-05-09T14:12:01Z

tests/integration/cmd_namespace_test.go

+		helper.CommonAfterEach(commonVar)
+	})
+	It("should successfully create the namespace", func() {
+		helper.Cmd("odo", "create", "namespace", "my-namespace").ShouldPass()


You will need to delete these namespaces after the tests, or they will stay in the cluster and make future tests fail

feloy · 2022-05-10T09:57:33Z

tests/integration/cmd_namespace_test.go

+		AfterEach(func() {
+			commonVar.CliRunner.DeleteNamespaceProject(project)
+		})
+		It("should successfully create the project", func() {


I can see this test failing on Windows, but I'm not sure why

Was this test passing before? I don't remember.

I think the tests are failing because you are not using the wait flag. I don't know why (*), the Project resource is not created in an atomic way, and you have to wait for it to be created.

I can see that the specs don't talk about the wait flag, probably you could force to wait everytime we create a project

(*) after some code reading, the creation of a project is done by first creating a projectrequest, and some operator will create a project later, asynchronously

feloy · 2022-05-10T10:50:01Z

tests/integration/cmd_namespace_test.go

+		AfterEach(func() {
+			commonVar.CliRunner.DeleteNamespaceProject(project)
+		})
+		It("should successfully create the project", func() {


I think the tests are failing because you are not using the wait flag. I don't know why (*), the Project resource is not created in an atomic way, and you have to wait for it to be created.

I can see that the specs don't talk about the wait flag, probably you could force to wait everytime we create a project

(*) after some code reading, the creation of a project is done by first creating a projectrequest, and some operator will create a project later, asynchronously

valaparthvi · 2022-05-10T12:44:53Z

/retest-required

feloy · 2022-05-10T12:57:08Z

tests/integration/cmd_namespace_test.go

+		})
+		It("should successfully create the project", func() {
+			helper.Cmd("odo", "create", "project", project, "--wait").ShouldPass()
+			Expect(commonVar.CliRunner.GetNamespaceProject()).To(ContainSubstring(project))


You could try to run the command kubectl get project/namespace protjetname instead of getting the full list of projects/namespaces

valaparthvi · 2022-05-10T14:11:11Z

/retest-required

rm3l

Another question after playing with the command: what is the expected behavior of the --kubeconfig flag with this command ? I can see it mentioned in the help:

❯ odo help create namespace
Create a new namespace. This command directly performs actions on the cluster and doesn't require a push.

Usage:
  odo create namespace [flags]

Aliases:
  namespace, project

Examples:
  # Create a new namespace
  odo create namespace my-namespace

Flags:
  -h, --help   Help for namespace
  -w, --wait   Wait until the namespace is ready

Additional Flags:
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

But when running the command with this flag, it looks like it didn't read/update the specified kubeconfig:

❯ odo create namespace test1 --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt --wait
 ✓  Waiting for namespace to come up [7ms]
 ✓  Namespace "test1" is ready for use
 ✓  New namespace created and now using namespace: test1

❯ kubectl --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt config get-contexts
CURRENT   NAME    CLUSTER               AUTHINFO   NAMESPACE
*         admin   ci-ln-yqzqj1t-72292   admin

It looks like it ignored the specified kubeconfig and used the default one:

❯ kubectl config get-contexts            
CURRENT   NAME                                     CLUSTER                  AUTHINFO                         NAMESPACE
          admin@talos-default                      talos-default            admin@talos-default              default
          asoro/api-crc-testing:6443/kubeadmin     api-crc-testing:6443     kubeadmin/api-crc-testing:6443   asoro
          crc-admin                                api-crc-testing:6443     kubeadmin                        default
          crc-developer                            api-crc-testing:6443     developer                        default
          default/api-crc-testing:6443/kubeadmin   api-crc-testing:6443     kubeadmin/api-crc-testing:6443   default
*         kind-local-k8s-cluster                   kind-local-k8s-cluster   kind-local-k8s-cluster           test1

pkg/odo/cli/create/namespace/namespace.go

rm3l · 2022-05-11T08:53:40Z

Another point I just stumbled upon: maybe we should also validate the input namespace name, using the same ValidateK8sResourceName function we use from the github.com/devfile/library/pkg/util package ? Just to be consistent with the current validation rules.

Otherwise, we may be affected by #5652: I successfully created an all-numeric Kubernetes namespace using odo create namespace. And now that it has been set as my current context namespace, odo dev won't start:

☸ kind-local-k8s-cluster (default) in ~/w/t/5620-devfile-with-container-component-command on  main [$!?] via ☕ v17.0.2 on ☁️   
❯ odo create namespace 5648
 ✓  Namespace "5648" is ready for use
 ✓  New namespace created and now using namespace: 5648

☸ kind-local-k8s-cluster (5648) in ~/w/t/5620-devfile-with-container-component-command on  main [$!?] via ☕ v17.0.2 on ☁️   
❯ odo dev                  
  __
 /  \__     Developing using the my-component-with-command Devfile
 \__/  \    Namespace: 5648
 /  \__/    odo version: v3.0.0-alpha1
 \__/

↪ Deploying to the cluster in developer mode
 ✗  Waiting for Kubernetes resources 
Cleaning resources, please wait
 ✗  failed to create the component: component namespace "5648" is not valid, component namespace should conform the following requirements: 
- Contain at most 63 characters
- Contain only lowercase alphanumeric characters or ‘-’
- Start with an alphanumeric character
- End with an alphanumeric character
- Must not contain all numeric values

docs/website/versioned_docs/version-3.0.0/command-reference/create-namespace.md

rm3l · 2022-05-11T12:46:14Z

tests/integration/cmd_namespace_test.go

+			helper.Cmd("odo", "create", "namespace", commonVar.Project).ShouldFail()
+		})
+		By("using an invalid namespace name", func() {
+			helper.Cmd("odo", "create", "namespace", "12345").ShouldFail()


Even if the command did not pass, would it be possible to also check that:

no namespace/project with this name has been created in the cluster

the active namespace we had prior to running this command has not changed

I think the first check is unnecessary here, it's being extra cautious. The second check makes sense.

I think the first check is unnecessary here, it's being extra cautious.

To me, the call to ShouldFail just guarantees that an error occurred with the command, but at this point, you don't necessarily know what kind of error occurred.
If tomorrow, someone removes the code that validates the namespace name, and the command fails for any reason when setting the current active namespace, then this test would still pass, I think; and in this case, I think it would be nice to detect this.
But okay, that should be fine for now.

rm3l · 2022-05-11T14:55:35Z

Another question after playing with the command: what is the expected behavior of the --kubeconfig flag with this command ? I can see it mentioned in the help:

❯ odo help create namespace
Create a new namespace. This command directly performs actions on the cluster and doesn't require a push.

Usage:
  odo create namespace [flags]

Aliases:
  namespace, project

Examples:
  # Create a new namespace
  odo create namespace my-namespace

Flags:
  -h, --help   Help for namespace
  -w, --wait   Wait until the namespace is ready

Additional Flags:
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

But when running the command with this flag, it looks like it didn't read/update the specified kubeconfig:

❯ odo create namespace test1 --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt --wait
 ✓  Waiting for namespace to come up [7ms]
 ✓  Namespace "test1" is ready for use
 ✓  New namespace created and now using namespace: test1

❯ kubectl --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt config get-contexts
CURRENT   NAME    CLUSTER               AUTHINFO   NAMESPACE
*         admin   ci-ln-yqzqj1t-72292   admin

It looks like it ignored the specified kubeconfig and used the default one:

❯ kubectl config get-contexts            
CURRENT   NAME                                     CLUSTER                  AUTHINFO                         NAMESPACE
          admin@talos-default                      talos-default            admin@talos-default              default
          asoro/api-crc-testing:6443/kubeadmin     api-crc-testing:6443     kubeadmin/api-crc-testing:6443   asoro
          crc-admin                                api-crc-testing:6443     kubeadmin                        default
          crc-developer                            api-crc-testing:6443     developer                        default
          default/api-crc-testing:6443/kubeadmin   api-crc-testing:6443     kubeadmin/api-crc-testing:6443   default
*         kind-local-k8s-cluster                   kind-local-k8s-cluster   kind-local-k8s-cluster           test1

The fact that the --kubeconfig flag is not honored seems to affect almost all commands that expose it. As such, it is not necessarily related to this PR. So I created a separate issue: #5732

valaparthvi · 2022-05-11T15:19:26Z

Another question after playing with the command: what is the expected behavior of the --kubeconfig flag with this command ? I can see it mentioned in the help:

❯ odo help create namespace
Create a new namespace. This command directly performs actions on the cluster and doesn't require a push.

Usage:
  odo create namespace [flags]

Aliases:
  namespace, project

Examples:
  # Create a new namespace
  odo create namespace my-namespace

Flags:
  -h, --help   Help for namespace
  -w, --wait   Wait until the namespace is ready

Additional Flags:
      --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
  -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
      --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging

But when running the command with this flag, it looks like it didn't read/update the specified kubeconfig:

❯ odo create namespace test1 --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt --wait
 ✓  Waiting for namespace to come up [7ms]
 ✓  Namespace "test1" is ready for use
 ✓  New namespace created and now using namespace: test1

❯ kubectl --kubeconfig /home/asoro/work/tmp/cluster-bot/cluster-bot.kubeconfig.txt config get-contexts
CURRENT   NAME    CLUSTER               AUTHINFO   NAMESPACE
*         admin   ci-ln-yqzqj1t-72292   admin

It looks like it ignored the specified kubeconfig and used the default one:

❯ kubectl config get-contexts            
CURRENT   NAME                                     CLUSTER                  AUTHINFO                         NAMESPACE
          admin@talos-default                      talos-default            admin@talos-default              default
          asoro/api-crc-testing:6443/kubeadmin     api-crc-testing:6443     kubeadmin/api-crc-testing:6443   asoro
          crc-admin                                api-crc-testing:6443     kubeadmin                        default
          crc-developer                            api-crc-testing:6443     developer                        default
          default/api-crc-testing:6443/kubeadmin   api-crc-testing:6443     kubeadmin/api-crc-testing:6443   default
*         kind-local-k8s-cluster                   kind-local-k8s-cluster   kind-local-k8s-cluster           test1

The fact that the --kubeconfig flag is not honored seems to affect almost all commands that expose it. As such, it is not necessarily related to this PR. So I created a separate issue: #5732

Ohhhh. I didn't see this was happening. The logic of this code is simply copy pasted from v2, I didn't take a look at it 🙄. Thanks for creating the issue!

valaparthvi · 2022-05-11T16:01:23Z

/retest-required
Infra errors.

Signed-off-by: Parthvi Vala <[email protected]>

sonarqubecloud · 2022-05-12T13:17:27Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.6% Duplication

feloy · 2022-05-12T13:35:55Z

/lgtm

valaparthvi · 2022-05-12T13:45:57Z

/retest-required

rm3l

/approve

openshift-ci · 2022-05-12T14:31:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

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~~ [rm3l]

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

* odo create namespace * Add documentation Signed-off-by: Parthvi Vala <[email protected]> * Philippe's review Signed-off-by: Parthvi Vala <[email protected]> * Rename doc file * Attempt at fixing integration tests Signed-off-by: Parthvi Vala <[email protected]> * Second Attempt at fixing integration tests Signed-off-by: Parthvi Vala <[email protected]> * Arm3l review * Arm3l review part 2

openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label May 9, 2022

openshift-ci bot requested review from feloy and rnapoles-rh May 9, 2022 13:43

valaparthvi force-pushed the create-namespace branch 2 times, most recently from dfabc11 to b77c98c Compare May 9, 2022 13:57

feloy suggested changes May 9, 2022

View reviewed changes

openshift-ci bot assigned feloy May 9, 2022

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label May 10, 2022

valaparthvi force-pushed the create-namespace branch from b77c98c to e71ed18 Compare May 10, 2022 09:34

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label May 10, 2022

feloy suggested changes May 10, 2022

View reviewed changes

valaparthvi force-pushed the create-namespace branch from 85000e9 to 8efedf3 Compare May 10, 2022 14:15

feloy approved these changes May 10, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 10, 2022

rm3l reviewed May 11, 2022

View reviewed changes

pkg/odo/cli/create/namespace/namespace.go Show resolved Hide resolved

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 11, 2022

valaparthvi requested review from rm3l and feloy May 11, 2022 11:01

rm3l requested changes May 11, 2022

View reviewed changes

openshift-ci bot assigned rm3l May 11, 2022

valaparthvi added 8 commits May 12, 2022 18:38

odo create namespace

7500339

Add documentation

e69d8c2

Signed-off-by: Parthvi Vala <[email protected]>

Philippe's review

97d3786

Signed-off-by: Parthvi Vala <[email protected]>

Rename doc file

c9e3e8d

Attempt at fixing integration tests

097b3c2

Signed-off-by: Parthvi Vala <[email protected]>

Second Attempt at fixing integration tests

006708e

Signed-off-by: Parthvi Vala <[email protected]>

Arm3l review

4514e6a

Arm3l review part 2

abd914b

valaparthvi force-pushed the create-namespace branch from 4e92967 to abd914b Compare May 12, 2022 13:16

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 12, 2022

valaparthvi requested a review from rm3l May 12, 2022 14:21

rm3l approved these changes May 12, 2022

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label May 12, 2022

valaparthvi mentioned this pull request May 12, 2022

odo create/delete/list namespace #5525

Closed

8 tasks

openshift-ci bot merged commit e9c0579 into redhat-developer:main May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

odo create namespace #5724

odo create namespace #5724

valaparthvi commented May 9, 2022 •

edited

Loading

netlify bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

feloy May 9, 2022

feloy May 9, 2022

feloy May 9, 2022

feloy May 9, 2022

feloy May 10, 2022

valaparthvi May 10, 2022

feloy May 10, 2022 •

edited

Loading

feloy May 10, 2022 •

edited

Loading

valaparthvi commented May 10, 2022

feloy May 10, 2022

valaparthvi commented May 10, 2022

rm3l left a comment •

edited

Loading

rm3l commented May 11, 2022 •

edited

Loading

rm3l May 11, 2022

valaparthvi May 11, 2022

rm3l May 11, 2022 •

edited

Loading

rm3l commented May 11, 2022

valaparthvi commented May 11, 2022

valaparthvi commented May 11, 2022

sonarqubecloud bot commented May 12, 2022

feloy commented May 12, 2022

valaparthvi commented May 12, 2022

rm3l left a comment

openshift-ci bot commented May 12, 2022

odo create namespace #5724

odo create namespace #5724

Conversation

valaparthvi commented May 9, 2022 • edited Loading

netlify bot commented May 9, 2022 • edited Loading

✅ Deploy Preview for odo-docusaurus-preview ready!

odo-robot bot commented May 9, 2022 • edited Loading

odo-robot bot commented May 9, 2022 • edited Loading

odo-robot bot commented May 9, 2022 • edited Loading

odo-robot bot commented May 9, 2022 • edited Loading

odo-robot bot commented May 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feloy May 10, 2022 • edited Loading

Choose a reason for hiding this comment

feloy May 10, 2022 • edited Loading

Choose a reason for hiding this comment

valaparthvi commented May 10, 2022

Choose a reason for hiding this comment

valaparthvi commented May 10, 2022

rm3l left a comment • edited Loading

Choose a reason for hiding this comment

rm3l commented May 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rm3l May 11, 2022 • edited Loading

Choose a reason for hiding this comment

rm3l commented May 11, 2022

valaparthvi commented May 11, 2022

valaparthvi commented May 11, 2022

sonarqubecloud bot commented May 12, 2022

feloy commented May 12, 2022

valaparthvi commented May 12, 2022

rm3l left a comment

Choose a reason for hiding this comment

openshift-ci bot commented May 12, 2022

valaparthvi commented May 9, 2022 •

edited

Loading

netlify bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

odo-robot bot commented May 9, 2022 •

edited

Loading

feloy May 10, 2022 •

edited

Loading

feloy May 10, 2022 •

edited

Loading

rm3l left a comment •

edited

Loading

rm3l commented May 11, 2022 •

edited

Loading

rm3l May 11, 2022 •

edited

Loading