fix(backend): On Pipeline delete, clean all associated Minio records #7542
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
difince
changed the title
… with the Pipeline and its Versions Fixes: kubeflow#7368 Signed-off-by: Diana Atanasova <dianaa@vmware.com>
| glog.Errorf("%v", errors.Wrapf(err, "Failed to delete pipeline file for pipeline %v", pipelineId)) | ||
| errChan := r.objectStore.DeleteFiles(fileNames) | ||
| for err1 := range errChan { | ||
| glog.Errorf("%v", errors.Wrapf(err1, "Failed to delete pipeline file for pipeline %v.", pipelineId)) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Note that I keep the current implementation - if Minios requests fail I print the errors from MInio down and return from the method without any error (even though the Pipeline and its Versions are not deleted. )
// Not fail the request if this step failed. A background run will do the cleanup.
// #388
I cannot find this background run that should do the cleanup.
Tell me, if you think this behavior needs to change
|
/assign @zijianjoy |
difince
changed the title
|
cc: @annajung, @juliusvonkohout |
|
Actually i just had a discussion with @zijianjoy from google and maybe we will get rid of pipeline template/definitions on Minio altogether. He needs to discuss it with his team. I do not see a point in storing the pipeline on Minio AND MySQL at the same time and hope that his team agrees. THis would also make the remaining namespace isolation easier. Maybe we should wait until @zijianjoy gets a descision from his team. Furthermore i have something orthogonal to this concerning the cache server. While this here would fix pipeline versions, i am trying to fix the cache server when artifacts have been deleted from S3 due to lifecyclepolicies. My collegue will prepare a PR that make the cache server check whether artifacts still exist on minio before assuming that they are still cached. |
|
Thank you @juliusvonkohout for your clarifications on both Minos vs Mysql and the cached issue. |
|
/assign @difince |
|
It depends on the use case. I would like the pipeline definition AND pipeline artifacts be deleted from minio if they are deleted from the data base. Related is also #7938 |
|
Closing the PR due to inactivity for too longtime |
On Pipeline delete, clean all Object Store (Minio) records associated with the Pipeline and its Versions
Fixes: #7368
Signed-off-by: Diana Atanasova dianaa@vmware.com
Description of your changes:
Checklist: