openebs / lvm-localpv

Dynamically provision Stateful Persistent Node-Local Volumes & Filesystems for Kubernetes that is integrated with a backend LVM2 data storage stack.
Apache License 2.0
235 stars 92 forks source link

Update README.md #293

Closed ianb-mp closed 2 months ago

ianb-mp commented 2 months ago

Fix helm install instructions

Pull Request template

Please, go through these steps before you submit a PR.

Why is this PR required? What issue does it fix?:

What this PR does?:

Does this PR require any upgrade changes?:

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? : Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

orville-wright commented 2 months ago

This readme update looks good.

avishnu commented 2 months ago

@ianb-mp Thanks for your concern and efforts on this PR. However, the 'issue' you are trying to address in this PR, is not exactly an issue. It is the intended behavior going forward, as per the new structure for OpenEBS. Kindly refer to the 3709 for more details.

To provide a quick summary, Starting v4.0.0, OpenEBS intends to move towards a structure which will have two variants - local and replicated. The local will be a unified solution for hostpath, lvm, zfs and the replicated will be mayastor. The new combined helm will be hosted from helm repo - (https://openebs.github.io/openebs)

Please stay tuned on the releases page.

ianb-mp commented 2 months ago

Thanks for the explanation. The fact remains that the latest installation instructions don't work. For example, running kubectl get pods -n openebs -l role=openebs-lvm will never return results (because lvm-localpv was not enabled).

avishnu commented 2 months ago

Can you paste the output of kubectl get pods -n openebs?

ianb-mp commented 2 months ago

After running helm install openebs --namespace openebs openebs/openebs --create-namespace, then I run kubectl get pods -n openebs:

NAME                                           READY   STATUS    RESTARTS   AGE
openebs-localpv-provisioner-56d6489bbc-5497s   1/1     Running   0          17s
openebs-ndm-operator-5d7944c94d-msnv4          1/1     Running   0          17s
openebs-ndm-zrbt6                              1/1     Running   0          17s

However, if I run install with --set lvm-localpv.enabled=true then list pods, I get this:

NAME                                           READY   STATUS    RESTARTS   AGE
openebs-localpv-provisioner-56d6489bbc-jrbqg   1/1     Running   0          51s
openebs-lvm-localpv-controller-0               5/5     Running   0          51s
openebs-lvm-localpv-node-4fhtb                 2/2     Running   0          51s
openebs-ndm-896ct                              1/1     Running   0          51s
openebs-ndm-operator-5d7944c94d-xj2ft          1/1     Running   0          51s
niladrih commented 2 months ago

Hi @ianb-mp, this is intended for use with the new changes going into https://github.com/openebs/openebs/pull/3704. The openebs/openebs chart is intended to be the default installation mode for all openebs 'storage' options including the LocalPV LVM driver.

This README.md is a part of a larger change that is in-flight at the moment.

The README.md that's relevant to the published helm charts for LocalPV LVM, and/or the OpenEBS, is the one packaged with the chart.

The instructions on the README.md will eventually make it into a released chart. I understand that we've published the documentation prematurely, but that is just a result of development efforts on the openebs/openebs chart and the docs surrounding it being asynchronous.

The older instructions apply to the 3.10 openebs/openebs chart. These apply to the new one (and it's coming :)).

ianb-mp commented 2 months ago

The older instructions apply to the 3.10 openebs/openebs chart. These apply to the new one (and it's coming :)).

Ok, I understand. I'm happy for this PR to be closed.

I see in PR https://github.com/openebs/openebs/pull/3704 that zfs-localpv and lvm-localpv are enabled by default, which explains why the instructions don't include a helm flag to enable them explicitly. It doesn't seem like a good idea to enable them both...

avishnu commented 2 months ago

The older instructions apply to the 3.10 openebs/openebs chart. These apply to the new one (and it's coming :)).

Ok, I understand. I'm happy for this PR to be closed.

I see in PR openebs/openebs#3704 that zfs-localpv and lvm-localpv are enabled by default, which explains why the instructions don't include a helm flag to enable them explicitly. It doesn't seem like a good idea to enable them both...

The product intent in subsequent versions is to have a single CSI provisioner that can provision localpv volumes of hostpath/lvm/zfs. The current installer is a step in that direction.