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

Implement BackingStore type pv-pool in Operator CR & CLI #91

Closed
ron1 opened this issue Oct 16, 2019 · 8 comments
Closed

Implement BackingStore type pv-pool in Operator CR & CLI #91

ron1 opened this issue Oct 16, 2019 · 8 comments
Labels
low hanging fruit Pick me up for instant satisfaction
Milestone

Comments

@ron1
Copy link

ron1 commented Oct 16, 2019

According to the noobaa-operator reconciliation code here:

return nil, util.NewPersistentError("NotYetImplemented",
, the noobaa-operator does not yet support BackingStore type "pv-pool". This is a blocker for many folks that want a performant NooBaa deployment on Kubernetes using their own storage.

At a minimum, the BackingStore documentation available here: https://github.com/noobaa/noobaa-operator/blob/master/doc/backing-store-crd.md#pvc-type should be updated to reflect that this feature has not yet been implemented. In addition, consider elevating the priority of this issue in the NooBaa backlog.

@tamireran
Copy link
Contributor

@ron1 You are correct. We will add it to the CLI soon. In the meantime, you can use the web management console from your browser. The access details can be found with the command
noobaa status or kubectl describe noobaa
Once you are in, you will see a button "Add Storage Resources". Click on it, and click on "Deploy Kubernetes Pool" in the new window. You will see a little wizard that will get you a local storage on local PV.
Please let us know if you have any issues.

@ron1
Copy link
Author

ron1 commented Oct 16, 2019

@tamireran Thanks for the clarification. I did not realize NooBaa Core supported this feature and that the limitation is just in the Operator and CLI. As you can imagine, I was taken back a bit when, during my evaluation of the product, I attempted to create a BackingStore CR with type pv-pool and was greeted with "NotYetImplemented".

Am I correct that pv-pools work with all Kubernetes/OpenShift dynamically-provisioned volumes and not just Kubernetes 1.14 GA Local Persistent Volumes? Based on the reference to "local storage on local PV" in your comment, this is not quite clear.

@ron1 ron1 changed the title Implement BackingStore type pv-pool Implement BackingStore type pv-pool in Operator Oct 16, 2019
@ron1 ron1 changed the title Implement BackingStore type pv-pool in Operator Implement BackingStore type pv-pool in Operator and CLI Oct 16, 2019
@ron1 ron1 changed the title Implement BackingStore type pv-pool in Operator and CLI Implement BackingStore type pv-pool in Operator CR & CLI Oct 16, 2019
@tamireran
Copy link
Contributor

tamireran commented Oct 17, 2019 via email

@guymguym guymguym added the low hanging fruit Pick me up for instant satisfaction label Oct 28, 2019
@ron1
Copy link
Author

ron1 commented Nov 13, 2019

@tamireran @guymguym Unfortunately, pvpool/pvstore type storage resources created via the "Advanced pvstore create" CLI or through the Web UI cannot be referenced from BucketClass CRs, the "Manage bucketclass" CLI, and therefore from ObjectBucketClaim CRs and the "Manage obc" CLI. This essentially makes them unusable unless I am missing something. If this is the case, please elevate the priority of this issue. A patch should be fairly easy to implement since the code needed to create a BackingStore of type pvpool has already been implemented for the "Advanced pvstore create" CLI.

@guymguym
Copy link
Member

@ron1 I know unfortunately. That's a hard gap. I'll try to push it.

@ron1
Copy link
Author

ron1 commented Nov 14, 2019

@guymguym Note that I am currently working on a patch for this issue here: https://github.com/ron1/noobaa-operator/tree/issue-91-pv-pool-backing-store.

I think my patch will make the CLI "Advanced pvstore management" function obsolete. Do you agree? If so, should this patch also remove that functionality or should the removal of that functionality be accomplished via a separate issue/PR?

@guymguym
Copy link
Member

@ron1 That's awesome!

Yes of course the CLI actions of pvstore where just a temporary solution until this was handled properly using pv-pool BackingStore.

I think that it makes most sense to remove both the commands of noobaa pvstore list/create and noobaa bucket list/create/delete once we implement the generic noobaa api * command as suggested in #138 - what do you think?

ron1 pushed a commit to ron1/noobaa-operator that referenced this issue Nov 16, 2019
ron1 pushed a commit to ron1/noobaa-operator that referenced this issue Nov 26, 2019
ron1 pushed a commit to ron1/noobaa-operator that referenced this issue Nov 26, 2019
guymguym added a commit that referenced this issue Nov 26, 2019
Feature #91 Enable backingstore type pv-pool
@guymguym guymguym added this to the v2.0.9 milestone Nov 26, 2019
@guymguym
Copy link
Member

Fixed in #146

guymguym pushed a commit to guymguym/noobaa-operator that referenced this issue Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low hanging fruit Pick me up for instant satisfaction
Projects
None yet
Development

No branches or pull requests

3 participants