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 dev/deploy should display a warning about default namespace on cluster #6688

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

valaparthvi
Copy link
Contributor

What type of PR is this:
/kind bug

What does this PR do / why we need it:
This PR adds a warning if the user is using a default namespace.
This PR also modifies helper.IsKubernetesCluster to work if KUBERNETES=true,t,1,T,true,True,TRUE.
Which issue(s) this PR fixes:

Fixes #5591

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

  1. odo set namespace default
  2. odo init --devfile go --starter go-starter --devfile-version latest
  3. odo dev should display the warning right away
  4. PODMAN_CMD=echo odo deploy should display the warning right away.
    If you are on an OpenShift cluster, it will use project instead of namespace.

@netlify
Copy link

netlify bot commented Mar 27, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 18892a8
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/64230722f8cba500089262ff

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 27, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and feloy March 27, 2023 07:11
…uster

Signed-off-by: Parthvi Vala <pvala@redhat.com>
@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

NoCluster Tests on commit 8558795 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

Kubernetes Docs Tests on commit 61d53dc finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 27, 2023

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

pkg/odo/cli/deploy/deploy.go Outdated Show resolved Hide resolved
tests/integration/cmd_dev_test.go Outdated Show resolved Hide resolved
valaparthvi and others added 2 commits March 28, 2023 13:49
Signed-off-by: Parthvi Vala <pvala@redhat.com>
…er or podman

Signed-off-by: Parthvi Vala <pvala@redhat.com>
Co-authored-by: Armel Soro <armel@rm3l.org>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@rm3l rm3l linked an issue Mar 28, 2023 that may be closed by this pull request
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

In terms of UX, what do you think about adding an empty line between the header and the warning message? Maybe it's just me, but I currently find the warning to be too close to the logo:

$ odo deploy  
  __
 /  \__     Running the application in Deploy mode using warning-default-ns-test Devfile
 \__/  \    Namespace: default
 /  \__/    odo version: v3.8.0
 \__/
 ⚠  You are using "default" project, odo may not work as expected in the default project.
 ⚠  You may set a new project by running `odo create project <name>`, or set an existing one by running `odo set project <name>`

↪ Executing command:
[...]

pkg/odo/genericclioptions/util.go Outdated Show resolved Hide resolved
…d of default

Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi valaparthvi reopened this Mar 29, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.3% 0.3% Duplication

@valaparthvi valaparthvi requested a review from rm3l March 29, 2023 06:50
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 29, 2023
@openshift-merge-robot openshift-merge-robot merged commit dfb36dd into redhat-developer:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo deploy panics if there is no Kubeconfig odo dev should display a warning on default namespace on cluster
3 participants