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

feat: enable csi plugin for backups using nutanix csi and velero #112

Merged
merged 13 commits into from
May 23, 2024

Conversation

justicorn
Copy link
Contributor

@justicorn justicorn commented May 20, 2024

feat: #64 #57
Enabling the CSI plugin (should be moved upstream) to the init containers, as well as the default plugin for AWS.
This includes a patch version downgrade from upstream as the change to the Dockerfile did not create a required directory.
Upstream issues are being tracked on Iron Bank's plugin for csi and on the upstream Iron Bank Velero package. Once this is fixed, these changes can be removed.

@anthonywendt
Copy link
Collaborator

anthonywendt commented May 20, 2024

I would update the PR title to something like
feat: enable csi plugin for backups
This will be the title of the squashed PR and will be picked up by release please when merged to main.
There are workflows we can add to check for this when we improve our CI situation for this repo.

@justicorn justicorn changed the title Feat/64 csi backups feat: enable csi plugin for backups using nutanix csi and velero May 21, 2024
Copy link
Contributor

@blancharda blancharda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current changes look good!
Let's add a section (or expand the existing velero section) to include the steps used for testing.

I also think it makes sense to push this back up into core itself.. There's probably some discussion to have over enabling the CSI initContainer by default, but I think it's worth including the images at the very least and maybe exposing it as a configurable option.

I expect that discussion/PR/release to take a bit more time, so I say we merge this (after the docs) and then roll it back when we have an appropriate upstream resolution.

blancharda
blancharda previously approved these changes May 22, 2024
Copy link
Contributor

@blancharda blancharda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I added a note regarding RWX limitations.

Copy link
Contributor

@blancharda blancharda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flava

@blancharda blancharda merged commit 8e2326d into main May 23, 2024
@blancharda blancharda deleted the feat/64-csi-backups branch May 23, 2024 13:32
This was referenced Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants