Moves hostpath driver from kubernetes-csi/drivers monorepo #1

vladimirvivien · 2018-12-13T19:48:11Z

This PR is to setup and migrate the Hostpath CSI driver from the github.com/kubernetes-csi/drivers mono repo to its own repository.

Copies source from drivers mono repo
Rearrange packages and names
Adjust Makefile and other build files
Introduce deploy/deploy-hostpath.sh to automatically install hostpath in cluster
Adds README documentation

Signed-off-by: Huamin Chen <[email protected]>

add hostpath driver to test create/delete volume

Signed-off-by: Huamin Chen <[email protected]>

hostpath: create and delete volume

Signed-off-by: Huamin Chen <[email protected]>

hostpath: mount volume

@jsafrane

Refactor based on feedback from @jsafrane

CSI based ISCSI driver

…loyment images - Test end to end

- Add csi-nodeplugin-simplenfs-flexdriver dockerfile & kubernetes dep…

Fix annotation

Refactor to app & pkg directories

add csi cinder driver

pohly · 2018-12-21T15:07:25Z

deploy/deploy-hostpath.sh

+# where it is executed.
+
+K8S_RELEASE=${K8S_RELEASE:-"release-1.13"}
+CSI_RELEASE=${CSI_RELEASE:-"release-1.0"}


I think these two variables are a bit misleading. Instead of selecting one particular, well-tested release they reference the latest code in certain release branches.

I understand that this is easier to implement and review, but is it really what we want?

I personally would prefer a solution where for each of the sidecars we install exactly the RBAC rules released together with that sidecar image.

There's also no guarantee that we will always have the same release branch for all apps. For example, we might release an external-provisioner 1.1.0 on a release-1.1 branch while the rest need no major new release and thus stay on the release-1.0 branch.

One way to avoid duplicating the versions of the sidecars would be to grep for each image in the .yaml file and rename it with sed to obtain the corresponding RBAC yaml URL.

@pohly I am taking a simple approach, in the next push, to just create a variable for each component. That way user of the script can set versions independently.

Can you add a comment that explains the simplification and warns about the downside ("pick the latest version on a branch, which might not match what was released together with the current set of containers")?

I went ahead and changed it so that the version for the RBAC files is taken from the version in the .yaml: vladimirvivien@557d126

pohly · 2018-12-21T15:20:59Z

deploy/hostpath/csi-hostpath-rbac.yaml

+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: csi-node-sa


I think this entire file is obsolete.

The hostpath driver should use the RBAC rules required by the csi-node-driver-registrar. At least on master, that file was removed (kubernetes-csi/node-driver-registrar#7 (comment)), but the release-1.0 still has it and therefore it should be used.

https://github.com/kubernetes-csi/node-driver-registrar/blob/release-1.0/deploy/kubernetes/rbac.yaml

@pohly Ok removed the rbac rule for node-driver-registrar

You probably haven't pushed that change yet, have you? I updated the version of the node-driver-registrar and in the process removed the RBAC file and service account because it was removed also from that new release: vladimirvivien@76766f1

pohly · 2018-12-21T15:23:21Z

app/main.go

+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");


Please move to a cmd/hostpathplugin directory.

pohly · 2018-12-21T15:30:41Z

deploy/deploy-hostpath.sh

+
+# apply CSIDriver and CSINodeInfo API objects
+kubectl apply -f https://raw.githubusercontent.com/kubernetes/csi-api/${K8S_RELEASE}/pkg/crd/manifests/csidriver.yaml --validate=false
+kubectl apply -f https://raw.githubusercontent.com/kubernetes/csi-api/${K8S_RELEASE}/pkg/crd/manifests/csinodeinfo.yaml --validate=false


This is not always needed, is it?

Worse, can it fail when the CRDs were already installed, which (if I'm not mistaken) is how clusters now should be brought up?

@pohly I don't have a good fix for this. I will add a flag (or try to detect if they are there) to install only when needed. Thanks for pointing this out.

I'm fine with how this is now handled (installation disabled by default).

k8s-ci-robot · 2019-01-16T14:59:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vladimirvivien
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: childsb

If they are not already assigned, you can assign the PR to them by writing /assign @childsb in a comment when ready.

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

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 · 2019-01-16T15:02:31Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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. I understand the commands that are listed here.

The RBAC rules were already obsolete before and in 1.0.2 they were also removed officially (https://github.com/kubernetes-csi/node-driver-registrar/releases/tag/v1.0.2).

Picking the RBAC rules from a release branch is not quite accurate (we want the rules released together with the tagged version) and is something that must be kept in sync manually. It is better to not hard-code this in the script and instead get the exact versions from the .yaml files. There should be no need for users to pass parameters to the script, so let's just mention that this is possible without going into details in the README.md. The previous instructions were out-dated, too.

pohly · 2019-01-21T11:29:08Z

/lgtm

We can merge this PR as-is right now and then merge vladimirvivien#1 on top of it, or @vladimirvivien can accept it into his branch and then we merge here. I'm fine with both approaches.

deployment tweaks

k8s-ci-robot · 2019-01-23T11:46:21Z

New changes are detected. LGTM label has been removed.

pohly · 2019-01-23T15:03:25Z

/lgtm

@saad-ali you might have to push this PR directly to master because of the CLA check for some of the commits that this is pulling in from the old repo.

@dims ran into a similar issue with moving code into kubernetes/utils, see kubernetes/utils#55 (comment) for the suggested solution (same as above).

saad-ali · 2019-01-23T19:27:35Z

Ack merging

initial content

chakri-nelluri and others added 30 commits November 15, 2017 15:28

first commit

0acaecd

Update README.md

055d1a6

Add support for flex adapter

d4f5324

Fix file names

1576909

Create app layout

5129616

Add CSI NFS driver

87c6f69

add hostpath driver to test create/delete volume

27bd7b8

Signed-off-by: Huamin Chen <[email protected]>

Merge pull request #1 from rootfs/dev

8d71f47

add hostpath driver to test create/delete volume

hostpath: create and delete volume

4ee4699

Signed-off-by: Huamin Chen <[email protected]>

Merge pull request kubernetes-csi#2 from rootfs/dev

faa0d1d

hostpath: create and delete volume

Rename lib to csi-common

81ff93c

Fix gogland package naming with _

7124f88

hostpath: mount volume

ff648cf

Signed-off-by: Huamin Chen <[email protected]>

Merge pull request kubernetes-csi#3 from rootfs/dev

7e30f0d

hostpath: mount volume

Update readme to indicate drivers are examples

87996b6

Refactor based on feedback from @jsafrane

5c655eb

Merge pull request kubernetes-csi#4 from chakri-nelluri/refactor

b39803e

Refactor based on feedback from @jsafrane

CSI based ISCSI driver

4d1267f

Merge pull request kubernetes-csi#5 from shilpamayanna/iscsi

8f9c2bd

CSI based ISCSI driver

- Add csi-nodeplugin-simplenfs-flexdriver dockerfile & kubernetes dep…

391a326

…loyment images - Test end to end

Merge pull request kubernetes-csi#6 from chakri-nelluri/k8s-deploy

903248f

- Add csi-nodeplugin-simplenfs-flexdriver dockerfile & kubernetes dep…

Fix annotation

c5a2663

Merge pull request kubernetes-csi#8 from chakri-nelluri/k8s-deploy

0b11066

Fix annotation

Refactor to app & pkg directories

5bef3a4

Merge pull request kubernetes-csi#11 from chakri-nelluri/refactor1

332baae

Refactor to app & pkg directories

add csi cinder driver

6f9c800

rebuild cinder driver by new architecture

4e41335

add unit test for csi cinder driver

21f6767

add license block in files

c443497

Merge pull request kubernetes-csi#7 from edisonxiang/addcsicinderdriver

e983709

add csi cinder driver

Adds source code

660c357

pohly requested changes Dec 21, 2018

View reviewed changes

pohly reviewed Dec 21, 2018

View reviewed changes

vladimirvivien force-pushed the transfer-hostpath branch from 44f210d to 57bcb23 Compare January 16, 2019 14:59

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 16, 2019

vladimirvivien added 2 commits January 17, 2019 11:21

Update deployment

06be288

Adds README documentation

2bd6115

vladimirvivien force-pushed the transfer-hostpath branch from 57bcb23 to 2bd6115 Compare January 17, 2019 19:22

This was referenced Jan 21, 2019

hostpath: Implement ValidateVolumeCapabilities kubernetes-retired/drivers#142

Closed

hostpath: Add block volume support kubernetes-retired/drivers#143

Closed

pohly added 2 commits January 21, 2019 11:46

deploy: update to node-driver-registrar 1.0.2

76766f1

The RBAC rules were already obsolete before and in 1.0.2 they were also removed officially (https://github.com/kubernetes-csi/node-driver-registrar/releases/tag/v1.0.2).

pohly mentioned this pull request Jan 21, 2019

deployment tweaks vladimirvivien/csi-driver-host-path#1

Merged

k8s-ci-robot assigned pohly Jan 21, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2019

Merge pull request #1 from pohly/pr-1-fixes

23656aa

deployment tweaks

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 23, 2019

dims mentioned this pull request Jan 23, 2019

CLAbot is lacking logs cncf/foundation#26

Closed

saad-ali merged commit 3b9819c into kubernetes-csi:master Jan 23, 2019

pohly pushed a commit to pohly/csi-driver-host-path that referenced this pull request Jan 24, 2019

Merge pull request kubernetes-csi#1 from pohly/master

b32dd01

initial content

pohly mentioned this pull request Jun 5, 2019

Migrate flex adapter from drivers kubernetes-retired/csi-driver-flex#3

Merged

tpoxa mentioned this pull request Dec 8, 2019

system:serviceaccount:cert-manager:csi-snapshotter cannot create resource ... #127

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moves hostpath driver from kubernetes-csi/drivers monorepo #1

Moves hostpath driver from kubernetes-csi/drivers monorepo #1

vladimirvivien commented Dec 13, 2018 •

edited

Loading

pohly Dec 21, 2018

pohly Dec 21, 2018

vladimirvivien Jan 15, 2019

pohly Jan 16, 2019

pohly Jan 21, 2019

pohly Dec 21, 2018

pohly Dec 21, 2018

vladimirvivien Jan 16, 2019

pohly Jan 21, 2019

pohly Dec 21, 2018

pohly Dec 21, 2018

vladimirvivien Jan 16, 2019

pohly Jan 21, 2019

k8s-ci-robot commented Jan 16, 2019

k8s-ci-robot commented Jan 16, 2019

pohly commented Jan 21, 2019

k8s-ci-robot commented Jan 23, 2019

pohly commented Jan 23, 2019

saad-ali commented Jan 23, 2019

Moves hostpath driver from kubernetes-csi/drivers monorepo #1

Moves hostpath driver from kubernetes-csi/drivers monorepo #1

Conversation

vladimirvivien commented Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 16, 2019

k8s-ci-robot commented Jan 16, 2019

pohly commented Jan 21, 2019

k8s-ci-robot commented Jan 23, 2019

pohly commented Jan 23, 2019

saad-ali commented Jan 23, 2019

vladimirvivien commented Dec 13, 2018 •

edited

Loading