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

deploy_che.sh deletes project by default #13436

Closed
wenzowski opened this issue May 29, 2019 · 15 comments
Closed

deploy_che.sh deletes project by default #13436

wenzowski opened this issue May 29, 2019 · 15 comments
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@wenzowski
Copy link

Description

Why on earth is CHE_REMOVE_PROJECT=true the default in createNewProject() @eivantsov? No harm done here, but that's really going to bite someone.

Pretty shocking first experience following the docs.

Reproduction Steps

> ./deploy_che.sh --multiuser --project=project-that-exists
[INFO]: Local templates directory not found. Downloading templates...
[INFO]: Checking if you are currently logged in...
[INFO]: Active session found. Your current context is: project-that-exists/redacted:8443/username
[WARNING]: Namespace "project-that-exists" exists.
[INFO]: Removing namespace project-that-exists ^C

😿 api call already sent before a ^C could stop the script

OS and version:

oc version
oc v3.11.0+0cbc58b
kubernetes v1.11.0+d4cacc0
features: Basic-Auth

Server https://redacted:8443
openshift v3.11.59
kubernetes v1.11.0+d4cacc0

Diagnostics:
Whoops...all gone.

wenzowski added a commit to wenzowski/che that referenced this issue May 29, 2019
why on earth would we blindly delete the project without any confirmation?
This is a high priority temporary fix to eclipse-che#13436 until a better solution is found.
@dneary
Copy link
Contributor

dneary commented Jun 7, 2019

@l0rd @davidfestal Can either of you respond to this, please?

@wenzowski I see you are using deploy_che.sh - is this also an issue when deploying Che with the Operator? https://operatorhub.io/operator/cheoperator

@kuanfandevops
Copy link

When specify --project=my-project, if my-projects is an existing project, it will delete it without giving a chance to quit.
$./deploy_che.sh --multiuser -s --project=my-project
[INFO]: Templates directory found at /tmp/templates. Applying templates from this directory. Delete it to get the latest templates if necessary
[INFO]: Checking if you are currently logged in...
[INFO]: Active session found. Your current context is: my-project/console-myhost:8443/myHitHubUser
[WARNING]: Namespace "my-project" exists.
[INFO]: Removing namespace my-project . Done!
[INFO]: Creating namespace "my-project" (attempt 1/50)
[INFO]: Creating namespace "my-project" (attempt 2/50)
...

@skabashnyuk
Copy link
Contributor

This is expected behavior I think. I believe CHE_REMOVE_PROJECT means should we delete or not before installation. @wenzowski @kuanfandevops What part of documentation made confusion?

@skabashnyuk
Copy link
Contributor

you should see $(printInfo "Removing namespace ${CHE_OPENSHIFT_PROJECT}") in the console before project removal.

@kuanfandevops
Copy link

I didn't set any value to CHE_REMOVE_PROJECT. What I add in my comment is all I can see from console. I recommend to set the env variable to be false by default and ask user's approval before removing the project.

@metlos
Copy link
Contributor

metlos commented Jun 10, 2019

What are we trying to achieve here? Me as a developer I'm quite happy with deploy_che.sh deleting the prior deployment by default.

@wenzowski
Copy link
Author

you should see $(printInfo "Removing namespace ${CHE_OPENSHIFT_PROJECT}") in the console before project removal.

@skabashnyuk Per the bug filed, that's exactly what I saw. This is terrible behaviour. DO NOT DELETE WITHOUT CONFIRMATION. The point of this bug is that "expected" behaviour is deeply and seriously flawed. I'm not trying to be an ass here; you are setting people up for irrecoverable data loss and you must change this default.

@wenzowski
Copy link
Author

What are we trying to achieve here? Me as a developer I'm quite happy with deploy_che.sh deleting the prior deployment by default.

Then at least document the default? Imagine, for a second, that a developer actually had something running in the project they're deploying to? This bug is open because delete-by-default is an unacceptable first experience.

I would like to at least see a read thrown in there so the user has a chance to recover before something they care about is deleted. But more fundamentally, why is a deletion necessary?

@wenzowski
Copy link
Author

@wenzowski I see you are using deploy_che.sh - is this also an issue when deploying Che with the Operator? https://operatorhub.io/operator/cheoperator

I haven't had a chance to try out the operator yet. This issue is specific to the deploy_che.sh script unless that's being used under the hood by the operator.

@wenzowski
Copy link
Author

What part of documentation made confusion?

The part where the project named tasty-lasagna gets deleted without any warning by doing ./deploy_che.sh --multiuser --project=tasty-lasagna. Someone's going to try and follow your docs and wreck something important by running this script.

@metlos
Copy link
Contributor

metlos commented Jun 10, 2019

I agree that this behavior is bad for an official user-facing functionality. I actually wasn't aware deploy_che.sh is what we recommend to customers 😱

👍 for only removing the project if explicitly asked for by a command line parameter.

@skabashnyuk
Copy link
Contributor

Let's imagine we have parameter --remove-che-project. What behavior do you expect?

@l0rd
Copy link
Contributor

l0rd commented Jun 11, 2019

@wenzowski your concern makes a lot of sense. I am not sure if we are going to fix the script ./deploy_che.sh though because it will be deprecated by mid-july. It will be replaced by chectl and in particular by the command chectl server:start.

Currently chectl supports local minikube and minishift only so it may not be useful for your use case but by july we are going to support remote kubernetes clusters as well. And chectl server:start behavior if che is already deployed is to re-start it (or update it if the flag --if-already-exists=update is provided c.f. che-incubator/chectl#32).

@mborodin
Copy link

mborodin commented Jul 2, 2019

Can you at least put a big warning in documentation? I just lost like a week of work...

@che-bot
Copy link
Contributor

che-bot commented Dec 30, 2019

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 30, 2019
@che-bot che-bot closed this as completed Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

8 participants