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

Do not necessarily error out if odo dev is stopped via Ctrl+C #6917

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Jun 21, 2023

What type of PR is this:
/kind bug
/area dev
/area refactoring

What does this PR do / why we need it:
This PR makes sure hitting Ctrl-C is not reported as an error. Well, the exit code is mapped onto any error returned while cleaning up the Dev resources.
But in telemetry, no error should be reported.

To do so, it first starts with refactoring error handling, making sure errors are propagated correctly up to the CLI layer, so that non-nil errors would propagate as non-zero exit code and nil errors will be propagated as zero exit code.
See [1] and [2] for the rationale.

[1] https://github.com/redhat-developer/odo/wiki/Dev:-Coding-Conventions#exit-in-main
[2] https://github.com/redhat-developer/odo/wiki/Dev:-Coding-Conventions#exit-once

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

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:
The exit code of odo dev is now mapped to the error returned when cleaning up the Dev resources. However, no error is reported in telemetry regardless of the status of the cleanup logic.

@netlify
Copy link

netlify bot commented Jun 21, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 9b891a7
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/649404827bf511000888bcc3

@openshift-ci openshift-ci bot added kind/bug Categorizes issue or PR as related to a bug. area/dev Issues or PRs related to `odo dev` area/refactoring Issues or PRs related to code refactoring labels Jun 21, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and kadel June 21, 2023 10:10
@rm3l rm3l requested review from feloy and valaparthvi and removed request for kadel and anandrkskd June 21, 2023 10:10
@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

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

@rm3l
Copy link
Member Author

rm3l commented Jun 21, 2023

Looking into the tests that are not passing...

@rm3l rm3l marked this pull request as draft June 21, 2023 11:49
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jun 21, 2023
@rm3l rm3l force-pushed the 6514-ctrl-c-odo-dev-reported-as-an-error-in-telemetry branch 2 times, most recently from 21ec4e7 to ceb4872 Compare June 21, 2023 15:47
@odo-robot
Copy link

odo-robot bot commented Jun 21, 2023

Kubernetes Docs Tests on commit 28ed064 finished successfully.
View logs: TXT HTML

@rm3l rm3l marked this pull request as ready for review June 21, 2023 16:10
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jun 21, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 22, 2023
@rm3l rm3l force-pushed the 6514-ctrl-c-odo-dev-reported-as-an-error-in-telemetry branch from ceb4872 to 9b891a7 Compare June 22, 2023 08:21
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 22, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rm3l
Copy link
Member Author

rm3l commented Jun 22, 2023

Had to rebase onto main and force-push to fix a conflict in cmd/odo/common_test.go.

@rm3l rm3l requested a review from feloy June 22, 2023 08:23
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 22, 2023
@openshift-merge-robot openshift-merge-robot merged commit 147542d into redhat-developer:main Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev Issues or PRs related to `odo dev` area/refactoring Issues or PRs related to code refactoring 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.

ctrl-c odo dev reported as an error in telemetry
3 participants