Thin pool creation #131

sp98 · 2022-03-21T03:47:15Z

Create thin pool via VG Manager.

Testing:

CR:

apiVersion: lvm.topolvm.io/v1alpha1
kind: LVMCluster
metadata:
  name: lvmcluster-sample
spec:
  storage:
    deviceClasses:
    - name: vg1
      thinPoolConfig:
        name: mytp1
        sizePercent: 50
        overprovisionRatio: 50

sh-4.4# vgs
  VG  #PV #LV #SN Attr   VSize  VFree
  vg1   3   1   0 wz--n- <4.37t 2.18t
sh-4.4# lvs
  LV    VG  Attr       LSize Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert
  mytp1 vg1 twi-a-tz-- 2.18t             0.00   6.13
sh-4.4#

sh-4.4# cat lvmd.yaml
device-classes:
- default: true
  lvcreate-options: null
  name: vg2
  spare-gb: null
  stripe: null
  stripe-size: ""
  thin-pool:
    name: mytp2
    overprovision-ratio: 30
  type: thin
  volume-group: vg2
socket-name: /run/lvmd/lvmd.sock

Apps using thin volumes:

  VG  #PV #LV #SN Attr   VSize  VFree
  vg2   3   3   0 wz--n- <4.37t 2.18t
sh-4.4# lvs
  LV                                   VG  Attr       LSize  Pool  Origin Data%  Meta%  Move Log Cpy%Sync Convert
  76e79637-5cc8-4795-8644-4298d8db3d1b vg2 Vwi-aotz--  5.00g mytp2        0.29
  c293d9ab-04dc-4b6d-8d9c-b3bf368a091c vg2 Vwi-aotz-- 40.00g mytp2        0.07
  mytp2                                vg2 twi-aotz--  2.18t              0.01   6.13
sh-4.4#

api/v1alpha1/lvmcluster_types.go

api/v1alpha1/lvmvolumegroup_types.go

pkg/vgmanager/vgmanager_controller.go

api/v1alpha1/lvmcluster_types.go

leelavg

in general, we don't have any validation of CR yet, better we start adding validation code with this CR or mark mandatory fields as required in CRD via kubebuilder?

sp98 · 2022-04-07T07:26:08Z

in general, we don't have any validation of CR yet, better we start adding validation code with this CR or mark mandatory fields as required in CRD via kubebuilder?

added CRD validation for the required fields for the new ThinPoolConfig.

leelavg

/lgtm

nbalacha

Does anything extra need to be done for the cleanup?

nbalacha · 2022-04-13T09:19:50Z

api/v1alpha1/lvmcluster_types.go

+	// OverProvisionRatio is the factor by which additional storage can be provisioned compared to
+	// the available storage in the thin pool.
+	// +kubebuilder:validation:Minimum=2
+	OverprovisionRatio int `json:"overprovisionRatio,omitempty"`


can this be empty?

No. Marked it as required.

nbalacha

The thin pool needs to be deleted before the VG is deleted:
{"level":"error","ts":1650198338.4370134,"logger":"controller.lvmcluster","msg":"Reconciler error","reconciler group":"lvm.topolvm.io","reconciler kind":"LVMCluster","name":"lvmcluster-sample","namespace":"lvm-operator-system","error":"failed cleaning up: lvmvg-manager waiting for all VGs to be deleted","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227"}

- Also update LVMVolumeGroup CR to use thinPoolConfig - Update suite test to include the required thinPoolConfig Signed-off-by: Santosh Pillai <[email protected]>

sp98 · 2022-04-18T06:46:10Z

pkg/vgmanager/vgmanager_controller.go

+		args := []string{"-f", fmt.Sprintf("%s/%s", volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig.Name)}
+		_, err = r.executor.ExecuteCommandWithOutputAsHost(lvRemoveCmd, args...)
+		if err != nil {
+			return fmt.Errorf("failed to delete thin pool %q. %v", volumeGroup.Spec.ThinPoolConfig.Name, err)


It does not cover the case when the thin pool is already deleted by the user.

updated. Checking if the thin pool exists before deleting it.

Signed-off-by: Santosh Pillai <[email protected]>

nbalacha · 2022-04-18T12:25:06Z

pkg/vgmanager/vgmanager_controller.go

@@ -249,6 +294,25 @@ func (r *VGReconciler) processDelete(ctx context.Context, volumeGroup *lvmv1alph
 		return nil
 	}

+	// Delete thin pool


This should be done after checking if the volumegroup exists.

That's right. Fixed.

Signed-off-by: Santosh Pillai <[email protected]>

openshift-ci · 2022-04-18T14:43:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nbalacha, sp98

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nbalacha]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 21, 2022

nbalacha reviewed Mar 22, 2022

View reviewed changes

sp98 force-pushed the thin-pool branch 4 times, most recently from 67ebf50 to be11c69 Compare March 23, 2022 06:41

leelavg reviewed Mar 23, 2022

View reviewed changes

api/v1alpha1/lvmcluster_types.go Outdated Show resolved Hide resolved

sp98 changed the title ~~[WIP] Thin pool creation~~ Thin pool creation Mar 23, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2022

sp98 force-pushed the thin-pool branch 2 times, most recently from 8ac50ec to 3051678 Compare March 24, 2022 04:52

sp98 requested review from nbalacha and leelavg March 24, 2022 04:54

sp98 force-pushed the thin-pool branch 4 times, most recently from a75ed2c to 063de1c Compare March 30, 2022 09:19

sp98 force-pushed the thin-pool branch 2 times, most recently from 18790fc to 530a9c0 Compare March 31, 2022 09:36

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2022

sp98 force-pushed the thin-pool branch from 530a9c0 to 8b6f1dd Compare April 5, 2022 04:40

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 5, 2022

sp98 force-pushed the thin-pool branch from 8b6f1dd to d0370a0 Compare April 5, 2022 04:41

leelavg reviewed Apr 5, 2022

View reviewed changes

sp98 force-pushed the thin-pool branch 3 times, most recently from 053d951 to 4634436 Compare April 7, 2022 06:55

sp98 requested a review from leelavg April 8, 2022 07:26

leelavg approved these changes Apr 8, 2022

View reviewed changes

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 12, 2022

sp98 force-pushed the thin-pool branch from 7c1cda6 to f076efc Compare April 13, 2022 04:51

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2022

sp98 force-pushed the thin-pool branch from f076efc to 0282153 Compare April 13, 2022 04:59

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2022

sp98 requested a review from nbalacha April 13, 2022 05:48

nbalacha reviewed Apr 13, 2022

View reviewed changes

sp98 force-pushed the thin-pool branch from 0282153 to d9a44f1 Compare April 14, 2022 05:29

nbalacha suggested changes Apr 17, 2022

View reviewed changes

openshift-ci bot assigned nbalacha Apr 17, 2022

feat: update API to support required thin-pool config

2b78f8a

- Also update LVMVolumeGroup CR to use thinPoolConfig - Update suite test to include the required thinPoolConfig Signed-off-by: Santosh Pillai <[email protected]>

sp98 force-pushed the thin-pool branch from d9a44f1 to f1ba754 Compare April 18, 2022 06:45

sp98 commented Apr 18, 2022

View reviewed changes

sp98 force-pushed the thin-pool branch 2 times, most recently from 4e778e6 to a710e84 Compare April 18, 2022 10:08

sp98 added 2 commits April 18, 2022 15:40

feat: update topolvm to v0.11.0

b7961c6

Signed-off-by: Santosh Pillai <[email protected]>

feat: create thin pool

aaa080a

Signed-off-by: Santosh Pillai <[email protected]>

sp98 force-pushed the thin-pool branch 2 times, most recently from c053dfa to 03afe07 Compare April 18, 2022 10:14

sp98 requested a review from nbalacha April 18, 2022 10:21

nbalacha reviewed Apr 18, 2022

View reviewed changes

feat: delete thin pool on cleanup

a809e90

Signed-off-by: Santosh Pillai <[email protected]>

sp98 force-pushed the thin-pool branch from 03afe07 to a809e90 Compare April 18, 2022 12:29

sp98 requested a review from nbalacha April 18, 2022 12:30

nbalacha approved these changes Apr 18, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 18, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 18, 2022

openshift-merge-robot merged commit d69fbde into openshift:main Apr 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thin pool creation #131

Thin pool creation #131

sp98 commented Mar 21, 2022 •

edited

Loading

leelavg left a comment

sp98 commented Apr 7, 2022

leelavg left a comment

nbalacha left a comment

nbalacha Apr 13, 2022

sp98 Apr 14, 2022

nbalacha left a comment

sp98 Apr 18, 2022

sp98 Apr 18, 2022

nbalacha Apr 18, 2022

sp98 Apr 18, 2022

openshift-ci bot commented Apr 18, 2022

Thin pool creation #131

Thin pool creation #131

Conversation

sp98 commented Mar 21, 2022 • edited Loading

leelavg left a comment

Choose a reason for hiding this comment

sp98 commented Apr 7, 2022

leelavg left a comment

Choose a reason for hiding this comment

nbalacha left a comment

Choose a reason for hiding this comment

nbalacha Apr 13, 2022

Choose a reason for hiding this comment

sp98 Apr 14, 2022

Choose a reason for hiding this comment

nbalacha left a comment

Choose a reason for hiding this comment

sp98 Apr 18, 2022

Choose a reason for hiding this comment

sp98 Apr 18, 2022

Choose a reason for hiding this comment

nbalacha Apr 18, 2022

Choose a reason for hiding this comment

sp98 Apr 18, 2022

Choose a reason for hiding this comment

openshift-ci bot commented Apr 18, 2022

sp98 commented Mar 21, 2022 •

edited

Loading