WIP: cross-namespace snapshot populator for csi-hostpath driver

[mkimuram]

What type of PR is this?
/kind feature

What this PR does / why we need it:
This is an example implementation for "provisioning volumes from cross-namespace snapshots" KEP by using populator approach. lib-volume-populator needs more adaption for GA and this use case will be a good candidate.

Which issue(s) this PR fixes:

Fixes #30

Special notes for your reviewer:
@bswartz @xing-yang @Elbehery

Does this PR introduce a user-facing change?:

Example implementation of cross-namespace snapshot populator for csi-hostpath driver is added.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mkimuram
To complete the pull request process, please assign saad-ali after the PR has been reviewed.
You can assign the PR to them by writing /assign @saad-ali in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[k8s-ci-robot]

Hi @mkimuram. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mkimuram]

@bswartz @xing-yang @Elbehery As I implemented this PR, I noticed below three points:

1. As a driver specific implementation, cross-namespace snapshot populator can be implemented (at least for CSI hostpath driver).
   Please also see README for how it can be used and tested.

2. Current lib-volume-populator doesn't allow below. So, it would be better adding such feature (This PR directly modified the library codes, which won't be the right way)

   • Adding/Mounting extra volumes
   • Modifying ServiceAccount

3. It is better to provide common implementation, not driver specific implementation like this. To achieve it by utilizing the existing populator approach, we may need to add a new CSI call to populate the already provisioned volume, instead of only allowing creating pre-populated volume in CreateVolume.

I've create another issues for (2) in this repo, and let's continue discussion related to (1) and (3) on cross-namespace snapshot KEP (but any comments on this PR are still welcome).

[bswartz]

@mkimuram I've read through most of this patch, and it's difficult to understand everything its doing.

My first complaint is that this repo shouldn't be used for any actual populators. I understand this is a WIP and it's easier to try out something new inside the repo, but the understanding should be that we'll eventually put the actual code somwhere else.

I'm still trying to understand why we need to patch the pod. It could be that the current library interface isn't flexible enough (in fact I strongly suspect that) but I need specific use cases to understand how we can improve it. What were you trying to do that you couldn't achieve with the existing interface?

[mkimuram]

@bswartz Thank you for checking the codes and giving feedback.

My first complaint is that this repo shouldn't be used for any actual populators. I understand this is a WIP and it's easier to try out something new inside the repo, but the understanding should be that we'll eventually put the actual code somwhere else.

This PR is just to share the idea, so the codes won't be necessary in this repo. I agree that we need another new repo if we implement this way.

I'm still trying to understand why we need to patch the pod. It could be that the current library interface isn't flexible enough (in fact I strongly suspect that) but I need specific use cases to understand how we can improve it. What were you trying to do that you couldn't achieve with the existing interface?

For this particular use case, this populator is trying to:

1. Copy the data from the hostpath that needs to be mounted on the populator Pod,
2. Access to other CRDs like VolumeSnapshot, VolumeSnapshotContents, and ReferencePolicies to find from where the data should be copied and whether it is allowed

So, it needs to mount volumes and to add extra RBAC, therefore it needs to modify the populator Pod's spec.

These requirements won't be special, because if we try to implement a simple populator that copies data from Configmap or Secret, for example, we would at least need either of the above.

[k8s-ci-robot]

@mkimuram: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.