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 delete is not deleting user created storage #2710

Closed
adisky opened this issue Mar 12, 2020 · 9 comments · Fixed by #2750
Closed

odo delete is not deleting user created storage #2710

adisky opened this issue Mar 12, 2020 · 9 comments · Fixed by #2750
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@adisky
Copy link
Contributor

adisky commented Mar 12, 2020

/kind bug

What versions of software are you using?

Operating System:

Output of odo version:

How did you run odo exactly?

odo component has following storage.

$ odo describe 
Component Name: nodejs-mytest1-mhaz
Type: nodejs
Source: file://./
Environment Variables:
 · DEBUG_PORT=5858
Storage:
 · nodejs-mytest1-mhaz-csgq of size 1Gi mounted to /tmp1

When odo delete command is run, it gives message of deleting storage but it is not actually deleting it.

$ odo delete 
This component has following storages which will be deleted with the component
Storage nodejs-mytest1-mhaz-csgq of size 1Gi
? Are you sure you want to delete nodejs-mytest1-mhaz from app? Yes
 ✓  Deleting component nodejs-mytest1-mhaz [213ms]
 ✓  Component nodejs-mytest1-mhaz from application app has been deleted

$ oc get pvc nodejs-mytest1-mhaz-csgq-app-pvc
NAME                               STATUS   VOLUME   CAPACITY   ACCESS MODES   STORAGECLASS   AGE
nodejs-mytest1-mhaz-csgq-app-pvc   Bound    pv0013   100Gi      RWO,ROX,RWX                   3m35s

Actual behavior

What should be the behavior of odo when odo delete command is run. It should delete user created pvc along with or not?
I think Ideally user created persistent volume to have persisted data. so deleting along with component deletion is not right.

option1: Delete user created storage with odo delete

  1. This needs to be fixed as currently we are only showing message, not actually deleting the pvc.

option2: Do not Delete user created storage with odo delete

  1. The message in odo delete needs to be updated.
  2. behavior of odo push after odo delete? as it will give error storage already exist . Proceed for component creation and show storage exist error as message?
$ odo push
Validation
 ✓  Checking component [25ms]

Configuration changes
 ✗  Failed to create component with name nodejs-mytest1-mhaz. Please use `odo config view` to view settings used to create component. Error: unable to create PVC: unable to create PVC: persistentvolumeclaims "nodejs-mytest1-mhaz-csgq-app-pvc" already exists

Expected behavior

Any logs, error output, etc?

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 12, 2020
@adisky
Copy link
Contributor Author

adisky commented Mar 12, 2020

From the Integration tests[1] it is evident that we want to delete user pvc with odo delete
[1 ]https://github.com/openshift/odo/blob/master/tests/integration/component.go#L834.

The above tests is also missed to catch that pvc is not deleted
cc @girishramnani as discussed on slack we should go with option2, I am not sure how to proceed for odo push after that.

@adisky
Copy link
Contributor Author

adisky commented Mar 17, 2020

as discussed with @kadel we need to delete user created pvc with odo delete , as it was an input from odo users and to keep it simple..lets currently delete pvc with odo delete.

@girishramnani
Copy link
Contributor

You can add owner reference to the pvc and that should get deleted if dc is deleted

@dharmit
Copy link
Member

dharmit commented Mar 17, 2020

You can add owner reference to the pvc and that should get deleted if dc is deleted

It's already there in PVC since we merged #2125. Unless there's a bug in the code added via that PR. 😉

@adisky
Copy link
Contributor Author

adisky commented Mar 17, 2020

You can add owner reference to the pvc and that should get deleted if dc is deleted

It's already there in PVC since we merged #2125. Unless there's a bug in the code added via that PR.

No ownerReference being passed here :)
https://github.com/openshift/odo/blob/master/pkg/occlient/volumes.go#L16

@dharmit
Copy link
Member

dharmit commented Mar 17, 2020

Umm, are you looking at the right place? I see two calls to CreatePVC function in occlient.go file and they both have ownerReference.

  1. https://github.com/openshift/odo/blob/e170ddee351452f1b208d66364c62900a6571d0c/pkg/occlient/occlient.go#L1199

  2. https://github.com/openshift/odo/blob/e170ddee351452f1b208d66364c62900a6571d0c/pkg/occlient/occlient.go#L1576

@adisky
Copy link
Contributor Author

adisky commented Mar 17, 2020

While odo push storage (user created not S2I PVC) is created from here, and it is not passing ownerRef
https://github.com/openshift/odo/blob/2faf28420a64a238f1661560a01335c401cc32b6/pkg/storage/storage.go#L48

@mik-dass
Copy link
Contributor

mik-dass commented Mar 24, 2020

I recently raised a PR which was supposed to update owner references for storage #2533. The problem is in the function updateStorageOwnerReference where I updated the owner references to the outdated PVC instead of the latest one https://github.com/openshift/odo/pull/2533/files#diff-edf1fd5b42bb3074e31c464e921dbcaaR196-R199. I will send a fix asap.

/assign

@adisky
Copy link
Contributor Author

adisky commented Mar 24, 2020

@mik-dass Thanks!!

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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants