kubernetes-csi/cluster-driver-registrar

DEPRECATED: Sidecar container that registers the CSI driver with the Kubernetes cluster

cluster-driver-registrar's Issues

Deprecate cluster-driver-registrar

Our documentation says:

As of Kubernetes 1.14, this side car container is being refactored.

It has been two releases without any progress in the refactoring / redesign. The documentation also says that CSIDriver instances should be created "by installation manifest".

In addition, the last released cluster-driver-registrar does not support current v1beta1 CSIDriver, it still uses the v1alpha1 one based on CRD. Kubernetes e2e tests use image tagged as "tmp-test-1.14", which was never officially released.

I think it's time to officially declare that cluster-driver-registrar is deprecated.

Code-wise, these things need to be updated:

• Kubernetes e2e tests must create CSIDriver without the registrar.
• Update kubernetes-csi/docs: kubernetes-csi/docs#183
• Update README here: #49
• Close all issues / PRs in this repo.

Creation of CSIDriverInfo object should not loop infinitely

Why are we doing this in kubernetesRegister? After the CSIDriverInfo object is created we should succeed and not continuously retry after sleep.

for {
		verifyAndAddCSIDriverInfo(clientset, csiDriverName, spec)
		time.Sleep(sleepDuration)
}

I have a feeling this hack may have been introduced as a workaround to having flaky object creation because of the flawed retry logic described in #6

/cc @lpabon @msau42

Skip attach should not be a command line argument

It should call the driver to get the driver capabilities to determine the skip attach value

quay.io/k8scsi/csi-cluster-driver-registrar:v1.0.2 not found

docker pull quay.io/k8scsi/csi-cluster-driver-registrar:v1.0.2 not foud

readme file image tag is v1.0.2

but only exec docker pull quay.io/k8scsi/csi-cluster-driver-registrar:v1.0.1

Add k8s version compability matrix in README

Which version of k8s and version of CSI is required to use cluster-driver-registrar? I guess it needs to be v1.13 and CSI v1.0? Can we have some documentations about this?

Broken Link of `contributor cheat sheet` needs to fix

Bug Report

I have observed same kind of issue in various kubernetes-csi project.
this happens because after the localization there are too much modifications done in the various directories.
I have observed same issue in this page also.

It has one broken link of the contributes cheat sheet which needs to fix.
I will try to look in further csi repo as well and try to fix it as soon as I can

/kind bug
/assign

update RBAC file

The RBAC file was copied unmodified from driver-registrar. Is it still correct?

The introduction still refers to "external provisioner" (was already broken when creating that file initially for driver-registrar).

remove CSIDriverInfo when uninstalling

If CSIDriverInfo is supposed to inform users of the cluster about currently installed drivers (is that the goal?), then there has to be a mechanism that ensures that the CSIDriverInfo for a CSI driver gets removed when removing the driver.

There is some support for this in the cluster-driver-registrar:

cluster-driver-registrar/cmd/cluster-driver-registrar/k8s_register.go

Lines 75 to 91 in 0660c6c

	// Set up goroutine to cleanup (aka deregister) on termination.
	c := make(chan os.Signal, 1)
	signal.Notify(c, os.Interrupt)
	go func() {
	<-c
	verifyAndDeleteCSIDriverInfo(
	clientset,
	csiDriverName,
	spec)
	os.Exit(1)
	}()
	// Run forever
	for {
	verifyAndAddCSIDriverInfo(clientset, csiDriverName, spec)
	time.Sleep(sleepDuration)
	}

pmem-csi uses cluster-driver-registrar (https://github.com/intel/pmem-CSI/blob/devel/deploy/kubernetes-1.13/pmem-csi-lvm.yaml#L122-L131), but in my experience the container gets stopped without cluster-driver-registrar removing the CSIDriverInfo for pmem-csi. Because the pod disappears shortly after termination, I've not been able to capture logs yet. Probably need to try with kubectl logs -f...

cluster-driver-registrar permissions

cluster-driver-registrar design feels like it has some issues with respect to permissions and encourages what I think is an antipattern.

The trend of loading controllers into k8s and giving them enough access to self register crd's and other things has always felt a little bad. If your deploying with helm, helm is already dealing with registering objects and has all the permissions needed to register them. The permissions to do anything stop after its installed, so it can't be used to do further things to the cluster. This seems good. But, if you give a long running process permissions to do such things, then it can be used to do bad things later. Another facet is auditing. If its in helm, it can be easily parsed by a human and verified its safe before running. This is very hard to do if its buried inside code.

RetryOnConflict not correct retry method

Looks like verifyAndDeleteCSIDriverInfo uses RetryOnConflict to control retries on deleting API Objects, RetryOnConflict is likely also used elsewhere.

However this is probably not the right thing to use as it only checks for the "Conflict Error" type returned from Update and retries when that is encountered, it does not retry for any other type of error and therefore if Delete fails for any other error (such as API Server unreachable) this will not retry and we will have orphaned objects.

All instances of this should be changed to some other generic error handling function that has exponential backoff and jitter, please take a look at the recent changes made to retry logic for "CSINodeInfo" in "nodeinfomanager" in Kubernetes for a good example

/cc @lpabon @verult @msau42