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

Update csi driver and other components #32

Merged
merged 4 commits into from
Jun 25, 2024
Merged

Conversation

iuriaranda
Copy link
Contributor

@iuriaranda iuriaranda commented Jun 18, 2024

This updates the csi-driver to the latest version, and syncs the versions of the other components with the ones defined in the upstream azurefile-csi chart.

The csi-attacher component has been removed from the upstream chart, see kubernetes-sigs/azurefile-csi-driver#1664 . Should it be removed from here as well? I couldn't find any info on why it was removed nor if it should be replaced by something else. For now I've bumped the version to the latest release. I've ported all the relevant upstream changes, including the csi-attacher removal.

As per https://github.com/giantswarm/giantswarm/issues/30774

This updates the csi-driver to the latest version, and syncs the versions of the other components with the ones defined in the upstream azurefile-csi chart.

The `csi-attacher` component has been removed from the upstream chart, see kubernetes-sigs/azurefile-csi-driver#1664 . Should it be removed from here as well? I couldn't find any info on why it was removed nor if it should be replaced by something else. For now I've bumped the version to the latest release.

As per giantswarm/giantswarm#30774
@iuriaranda iuriaranda requested a review from a team as a code owner June 18, 2024 11:09
@vxav
Copy link

vxav commented Jun 18, 2024

According to this cherrypick:

cleanup: remove csi-attacher config, ControllerPublishVolume capability is removed starting from v1.29.1

kubernetes-sigs/azurefile-csi-driver#1468
kubernetes-sigs/azurefile-csi-driver#1467

It seems that this capability is the job of the attacher container.
https://github.com/kubernetes-csi/external-attacher?tab=readme-ov-file#csi-attacher

The external-attacher is a sidecar container that attaches volumes to nodes by calling ControllerPublish and ControllerUnpublish functions of CSI drivers. It is necessary because internal Attach/Detach controller running in Kubernetes controller-manager does not have any direct interfaces to CSI drivers.

I don't know enough about the inner workings of CSI drivers to comment further though 😄.


However we are not on 1.29 yet. Is there a way to tie a release to Kubernetes version compatibility somehow?

EDIT: The table says Kubernetes 1.21+. Maybe we just don't need this thing then. A test will tell

@vxav
Copy link

vxav commented Jun 18, 2024

I'm not familiar with that repo but I would assume that we also need to update the manifests to 1.30.2.

@iuriaranda
Copy link
Contributor Author

It seems that this capability is the job of the attacher container.

That's precisely the container that has been removed from the upstream chart 🤔

However we are not on 1.29 yet. Is there a way to tie a release to Kubernetes version compatibility somehow?

EDIT: The table says Kubernetes 1.21+. Maybe we just don't need this thing then. A test will tell

When they mention that "ControllerPublishVolume capability is removed starting from v1.29.1" they are referring to the Azurefile CSI driver version I think, not the k8s one. In any case, as you mentioned, all CSI versions are compatible starting from k8s 1.21 so we should be good.

I'm not familiar with that repo but I would assume that we also need to update the manifests to 1.30.2.

Ok so our chart should match the upstream one as much as possible, while keeping our own customisations I assume? I'll see to port all changes then.

Maybe someone from @giantswarm/team-phoenix can chime in regarding what to do with the external-attacher sidecar?

This commit syncs all changes made in the upstream chart since the last update.
@iuriaranda
Copy link
Contributor Author

I've tested this by creating a new cluster and updating the CSI helm release to this dev version. I've created a pod with an azurefile PVC and everything works as expected

@iuriaranda iuriaranda merged commit 6f0a233 into main Jun 25, 2024
5 checks passed
@iuriaranda iuriaranda deleted the update-csi-1-30-2 branch June 25, 2024 09:17
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