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

feat: Add manila csi #80

Merged
merged 20 commits into from
Jun 1, 2023
Merged

feat: Add manila csi #80

merged 20 commits into from
Jun 1, 2023

Conversation

okozachenko1203
Copy link
Member

@okozachenko1203 okozachenko1203 commented Mar 28, 2023

  • This PR includes manila-csi-plugin deployment. This PR extends magnum.conf by adding [manila_client] section for keystone authtoken. So magnum driver can configure manila_client and use this to interact with sharev2 api.
  • magnum doesn't support manila intergration so I override magnum.common.clients.OpenStackClients to include manila method which returns the manila client.
  • This PR also makes cinder-csi-plugin and manila-csi-plugin as conditional. i.e. even if labels (cinder_csi_enabled, manila_csi_enabled) are set as true, if the underlying openstack has no enabled endpoints for cinder and manila, it will not install those plugins.

fix: #79

In addition, deploy cinder-csi plugin as conditional
Copy link
Member Author

@okozachenko1203 okozachenko1203 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mnaser i added comments where requires more attention.

magnum_cluster_api/conf.py Show resolved Hide resolved
magnum_cluster_api/utils.py Show resolved Hide resolved
@mnaser
Copy link
Member

mnaser commented May 24, 2023

@okozachenko1203 I previously built a tool here which will 'sync' all the manifests and apply all the changes that we use -- https://github.com/vexxhost/magnum-cluster-api/blob/main/tools/sync-csi-manifests

There are other sync jobs too. I think what I would like is that maybe we extend that script, and we also create a job that runs this script in CI and uses git to make sure that no changes happen, which means that no one made changes that were not implmented in this script and we can 'reliabely' get the manifests by running that script.

The potential second step to this is adding this script into the build script so that the manifestis not bundled in the source code but added in build time like we do for charts.

@okozachenko1203
Copy link
Member Author

@okozachenko1203 I previously built a tool here which will 'sync' all the manifests and apply all the changes that we use -- https://github.com/vexxhost/magnum-cluster-api/blob/main/tools/sync-csi-manifests

There are other sync jobs too. I think what I would like is that maybe we extend that script, and we also create a job that runs this script in CI and uses git to make sure that no changes happen, which means that no one made changes that were not implmented in this script and we can 'reliabely' get the manifests by running that script.

The potential second step to this is adding this script into the build script so that the manifestis not bundled in the source code but added in build time like we do for charts.

I think we can consider keeping diff/patch files instead of writing py script.

@okozachenko1203 okozachenko1203 changed the title WIP: feat: Add manila csi feat: Add manila csi May 29, 2023
@mnaser mnaser merged commit 1cd0324 into main Jun 1, 2023
@mnaser mnaser deleted the add-manila-csi branch June 1, 2023 14:22
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.

Add Manila CSI
2 participants