feat(hostpath):enforce xfs quota on the hostpath

[almas33]

feat(hostpath):enforce xfs quota on the hostpath

Issue no. - #13

To test:-

1. Follow documentation of task 1 - https://docs.google.com/document/d/1Vs-8Xv4fm_B5-qp4I7dYEVYfO4hEVY7KMV9uhAgdZfk/edit# to setup xfs fs in the basepath.
2. Use my image almas33/openebs-pv in place of openebs/provisioner-localpv and almas33/xfs-quota in place of openebs/linux-utils
3. Add type:enforceXfsQuota , bsoft as soft limit and bhard as hard limit in the parameters in storage class.yaml
4. Run pod as done normally.

[codecov-commenter]

Codecov Report

Merging #78 (1b00e61) into develop (46ac0ac) will decrease coverage by 0.49%.
The diff coverage is 20.58%.

@@             Coverage Diff             @@
##           develop      #78      +/-   ##
===========================================
- Coverage    33.99%   33.49%   -0.50%     
===========================================
  Files           29       29              
  Lines         2518     2618     +100     
===========================================
+ Hits           856      877      +21     
- Misses        1618     1697      +79     
  Partials        44       44              

Impacted Files	Coverage Δ
...md/provisioner-localpv/app/provisioner_hostpath.go	0.00% <0.00%> (ø)
cmd/provisioner-localpv/app/helper_hostpath.go	13.54% <30.43%> (+13.54%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9884bd9...1b00e61. Read the comment docs.

[Ab-hishek]

@almas33 Given some comments for changes.

[Ab-hishek]

I am not sure whether this will work on only system as while testing myself I was not able to run these commands without the root access. So, I feel better to give this pod root access. cc: @kmova @akhilerm .

[Ab-hishek]

Also in this way, if one commands fails then what's the point of running the subsequent commands. I feel rather than passing these commands like this better to create a shell script for all these commands(with proper error or successful logs) but still the logs in the pod won't be of much help as the pod will be deleted after few seconds and pass them as configmap volumes to the helper pod and execute the script. cc: @kmova @akhilerm .

[Ab-hishek]

All these can be performed outside of this if condition. If the fields are not filled by the user then they will remain empty. And for if condition you can check for:

Suggested change

	bsoft := opts.StorageClass.Parameters[Bsoft]
	bhard := opts.StorageClass.Parameters[Bhard]
	podOpts := &HelperPodOptions{
	name: name,
	path: path,
	nodeAffinityLabelKey: nodeAffinityKey,
	nodeAffinityLabelValue: nodeAffinityValue,
	serviceAccountName: saName,
	selectedNodeTaints: taints,
	imagePullSecrets: imagePullSecrets,
	bsoft: bsoft,
	bhard: bhard,
	}
	if scType == "enforceXfsQuota" && (len(Bsoft) != 0 || len(Bhard) != 0)

And then have proper error handling checks too.

[almas33]

I don't think we should check like this if scType == "enforceXfsQuota" && (len(Bsoft) != 0 || len(Bhard) != 0) because if user has given type enforceXfsQuota that means he wants to set quota , then we should try to create quota pod , but while pod creation the error both limits can't be 0 will be displayed and it will fail but doing like the way you suggested there will be no error displayed

[almas33]

If we want to remove redeclaration of podOpts we can declare value of bsoft and bhard while sending it to helper pod , it will be empty anyways ( values are not provided in parameters )as variable is declared in podOpts.

[kmova]

@almas33 - it is quite possible to misconfigure the parameters or provide incorrect parameters by users - while they are learning how to use the solution. The code should handle this via:

• Sane defaults
• Catch errors

[almas33]

@kmova - does that mean there will be some default limits applied if there is error in limits entered in parameter . Error handling of incorrect parameters are already taken care in convertToK()

[kmova]

@almas33, let us get back to this comment after the changes suggested in the design doc for storage class parameters are updated. The validation will change.

[Ab-hishek]

The PR looks good to me. @kmova can you review it once.

[Ab-hishek]

I'll try to add integration tests in the same PR. Or else, we can take that up in a separate PR.

[niladrih]

@almas33 Could you please rebase your branch with 'develop' once? Thanks.