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

ceph-csi-cephfs helm chart still contains provisioner-rules-clusterrole.yaml template #3329

Closed
geraldwuhoo opened this issue Aug 19, 2022 · 8 comments · Fixed by #3371
Closed
Labels
component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Milestone

Comments

@geraldwuhoo
Copy link

Describe the bug

The latest ceph-csi-cephfs chart version, 3.7.0 still has the provisioner-rules-clusterrole.yaml template, despite
its removal here. This is causing deployments using the default values.yaml to fail.

Environment details

  • Image/version of Ceph CSI driver : 3.7.0
  • Helm chart version : 3.7.0
  • Kernel version : N/A
  • Mounter used for mounting PVC (for cephFS its fuse or kernel. for rbd its
    krbd or rbd-nbd) : N/A
  • Kubernetes cluster version : N/A
  • Ceph cluster version : N/A

Steps to reproduce

Steps to reproduce the behavior:

  1. Add the helm repo and update it to 3.7.0.
  2. Run helm template ceph-csi/ceph-csi-cephfs with no other arguments.
  3. See error.

Actual results

Error rendering the helm chart:

λ › helm search repo ceph-csi
NAME                    	CHART VERSION	APP VERSION	DESCRIPTION
ceph-csi/ceph-csi-cephfs	3.7.0        	3.7.0      	Container Storage Interface (CSI) driver, provi...
ceph-csi/ceph-csi-rbd   	3.7.0        	3.7.0      	Container Storage Interface (CSI) driver, provi...
λ › helm template ceph-csi/ceph-csi-cephfs
Error: template: ceph-csi-cephfs/templates/provisioner-rules-clusterrole.yaml:41:14: executing "ceph-csi-cephfs/templates/provisioner-rules-clusterrole.yaml" at <.Values.provisioner.attacher.enabled>: nil pointer evaluating interface {}.enabled

Expected behavior

The helm rendering should succeed with the default values.yaml

Logs

N/A

Additional context

I can confirm manually that the provisioner-rules-clusterrole.yaml is still in the templates directory:

λ › helm search repo ceph-csi
NAME                    	CHART VERSION	APP VERSION	DESCRIPTION
ceph-csi/ceph-csi-cephfs	3.7.0        	3.7.0      	Container Storage Interface (CSI) driver, provi...
ceph-csi/ceph-csi-rbd   	3.7.0        	3.7.0      	Container Storage Interface (CSI) driver, provi...
λ › helm pull ceph-csi/ceph-csi-cephfs --untar
λ › ls ceph-csi-cephfs/templates | grep provisioner-rules
provisioner-rules-clusterrole.yaml
@humblec
Copy link
Collaborator

humblec commented Aug 22, 2022

The templates looks to be not in sync between this and charts repo which caused the issue.
If you remove the attacher session https://github.com/ceph/csi-charts/blob/master/docs/cephfs/ceph-csi-cephfs/templates/provisioner-rules-clusterrole.yaml#L41 in the extracted directory and render it , is it working @geraldwuhoo ?

@humblec
Copy link
Collaborator

humblec commented Aug 22, 2022

Cc @ceph/ceph-csi-contributors

@nixpanic nixpanic added the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Aug 22, 2022
@geraldwuhoo
Copy link
Author

The templates looks to be not in sync between this and charts repo which caused the issue. If you remove the attacher session https://github.com/ceph/csi-charts/blob/master/docs/cephfs/ceph-csi-cephfs/templates/provisioner-rules-clusterrole.yaml#L41 in the extracted directory and render it , is it working @geraldwuhoo ?

@humblec While this may work, I think there is a bigger underlying issue. It seems both the provisioner-rules-clusterrole.yaml and nodeplugin-rules-clusterrole.yaml files were removed entirely years ago: cc0f0b8. Yet these files remain in the charts repo: 1, 2. Seems that there is something wrong with the process used to sync between this repo and the charts repo when a file is deleted from this repo. If I had to guess, I would assume it's here:

cp -R "./charts/ceph-csi-${PACKAGE}" "${CHARTDIR}/csi-charts/docs/${PACKAGE}"

This line copies all the files over recursively, but does not handle deletion of files on the remote if local files were deleted. I would use something like rsync --archive --delete instead of cp -R to keep the directories in sync with deletion, but at this point you know about the codebase+process than myself, so I will defer actual implementation to the Ceph team.

Out of curiosity, why is there a separate charts repo in the first place?

@humblec
Copy link
Collaborator

humblec commented Aug 22, 2022

The templates looks to be not in sync between this and charts repo which caused the issue. If you remove the attacher session https://github.com/ceph/csi-charts/blob/master/docs/cephfs/ceph-csi-cephfs/templates/provisioner-rules-clusterrole.yaml#L41 in the extracted directory and render it , is it working @geraldwuhoo ?

@humblec While this may work, I think there is a bigger underlying issue.

Indeed.

It seems both the provisioner-rules-clusterrole.yaml and nodeplugin-rules-clusterrole.yaml files were removed entirely years ago: cc0f0b8. Yet these files remain in the charts repo: 1, 2.

Yes, noticed the same while looking at this issue.

Seems that there is something wrong with the process used to sync between this repo and the charts repo when a file is deleted from this repo. If I had to guess, I would assume it's here:

cp -R "./charts/ceph-csi-${PACKAGE}" "${CHARTDIR}/csi-charts/docs/${PACKAGE}"

This line copies all the files over recursively, but does not handle deletion of files on the remote if local files were deleted. I would use something like rsync --archive --delete instead of cp -R to keep the directories in sync with deletion, but at this point you know about the codebase+process than myself, so I will defer actual implementation to the Ceph team.

Above solution of "rsync" looks to be on spot to keep this repo files and chart repo files in parity. We definitely have to keep these repos in sync or single source is better.

While we wait for team's input, I would appreciate if you can file a PR and we can discuss the approach on the same too.

@KarlHerler
Copy link

Is there a non-invasive work around for this issue?

@jaricek
Copy link

jaricek commented Sep 5, 2022

Is possible solve this problem please ?

Madhu-1 added a commit to Madhu-1/ceph-csi that referenced this issue Sep 6, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: ceph#3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Madhu-1 added a commit to Madhu-1/ceph-csi that referenced this issue Sep 6, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: ceph#3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@mergify mergify bot closed this as completed in #3371 Sep 6, 2022
mergify bot pushed a commit that referenced this issue Sep 6, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: #3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
mergify bot pushed a commit that referenced this issue Sep 6, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: #3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
(cherry picked from commit 9d46478)
Madhu-1 added a commit that referenced this issue Sep 7, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: #3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
(cherry picked from commit 9d46478)
mergify bot pushed a commit that referenced this issue Sep 7, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: #3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
(cherry picked from commit 9d46478)
@jcollie
Copy link

jcollie commented Sep 7, 2022

This issue does not appear to be solved yet. I cannot upgrade to 3.7.0, I am getting the same errors as everyone else. Has the job to resync the helm charts not been done yet? Is there a change that I can add to my values file to get this to work temporarily?

@nixpanic
Copy link
Member

nixpanic commented Sep 8, 2022

There is no 3.7.1 release yet. Once that is available, the removed files should be gone, finally.

@nixpanic nixpanic added this to the release-3.7.1 milestone Sep 8, 2022
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/ceph-csi that referenced this issue Sep 13, 2022
When a file on source is deleted same
need to be deleted on the destination,
with rsync we can achieve it.

fixes: ceph#3329

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants