Ability to use to CSI plugin for Velero. Supports AWS EBS CSI driver. - #1285
Imran Pochi (ipochi) wants to merge 2 commits into
Conversation
|
Imran Pochi (@ipochi) component name is |
Mateusz Gozdek (invidian)
left a comment
Mateusz Gozdek (invidian)
left a comment
There was a problem hiding this comment.
Just some nits, overall LGTM
| func (c *Configuration) Validate() hcl.Diagnostics { | ||
| return c.validate() | ||
| } | ||
|
|
||
| // validate validates component configuration | ||
| func (c *Configuration) validate() hcl.Diagnostics { |
There was a problem hiding this comment.
Any reason to have those 2 function separate?
| } | ||
|
|
||
| // validate validates BackupStorageLocation struct fields. | ||
| func (b *BackupStorageLocation) validate() hcl.Diagnostics { // nolint:dupl |
There was a problem hiding this comment.
Maybe we could deduplicate the code?
There was a problem hiding this comment.
what do you mean ?
There was a problem hiding this comment.
I mean:
$ git grep 'BackupStorageLocation) validate'
pkg/components/velero/csi/drivers/aws/aws.go:func (b *BackupStorageLocation) validate() hcl.Diagnostics {
pkg/components/velero/openebs/openebs.go:func (b *BackupStorageLocation) validate(defaultProvider string) hcl.Diagnostics { // nolint:dupl
This code is very similar, we can perhaps put it in separate package and import to avoid duplication.
|
|
||
| // Values returns the plugin specific values for Velero Helm chart. | ||
| func (c *Configuration) Values() (string, error) { | ||
| t := template.Must(template.New("values").Parse(chartValuesTmpl)) |
There was a problem hiding this comment.
If we use template.Must we need to have unit tests to validate the template.
There was a problem hiding this comment.
pkg/components/velero/component_test.go has the tests for each provider.
There was a problem hiding this comment.
Oh.. Can we have a tests locally to the package? Otherwise changing code in one package breaks tests in other, this is not nice.
There was a problem hiding this comment.
added unit tests.
4528838 to
7e996aa
Compare
7e996aa to
7c61da3
Compare
This adds the ability to create backup and restore using CSI plugin when Lokomotive is installed on AWS and `aws_ebs_csi_driver` componnent is used to dynamically provision volumes. Signed-off-by: Imran Pochi <imran@kinvolk.io>
Update configuration reference documentation to reflect the new CSI plugin. Signed-off-by: Imran Pochi <imran@kinvolk.io>
0145eea to
db02ca3
Compare
|
Imran Pochi (@ipochi) what's the progress on this? Is this being worked on actively? |
|
Suraj Deshmukh (@surajssd) yes. This would not be part of the 0.6.0 release. I didn't get much time to address the review comments as I am focussing on baremetal platform missing features. |
3 participants
This adds the ability to create backup and restore with Velero using CSI plugin when Lokomotive is installed on AWS and
aws-ebs-csi-drivercomponent is used to dynamically provision volumes