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

Add compose support for cluster volumes #3662

Merged
merged 1 commit into from Nov 2, 2022

Conversation

dperny
Copy link
Contributor

@dperny dperny commented Jun 6, 2022

Adds compose support for CSI/cluster volumes.

Feel free to bikeshed on the UI.

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2022

Codecov Report

Merging #3662 (3a75f29) into master (247f568) will decrease coverage by 0.04%.
The diff coverage is 37.20%.

❗ Current head 3a75f29 differs from pull request most recent head 1b827ae. Consider uploading reports for the commit 1b827ae to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3662      +/-   ##
==========================================
- Coverage   59.21%   59.17%   -0.05%     
==========================================
  Files         288      288              
  Lines       24605    24647      +42     
==========================================
+ Hits        14571    14586      +15     
- Misses       9159     9176      +17     
- Partials      875      885      +10     

@s4ke
Copy link

s4ke commented Jun 28, 2022

I think this is important to get into 22.06

@s4ke
Copy link

s4ke commented Sep 27, 2022

@thaJeztah I think this should be merged. Happy to help with anything regarding CSI support in Swarm Mode.

@thaJeztah
Copy link
Member

I think there's two changes needed for this to be merged;

  • As we discussed in a maintainers call (and because the syntax may still change), change this option to an -x- option (perhaps -x-cluster-volume ?
  • We need to update this PR to take into account the changes from csi to cluster

@dperny do you have time to work on that, to get it merged?

@dperny dperny force-pushed the cluster-volumes-compose branch 2 times, most recently from 81c35e7 to 3a75f29 Compare October 20, 2022 16:20
@cpuguy83
Copy link
Collaborator

Seems like on this one we just need the x- change?

@dperny dperny force-pushed the cluster-volumes-compose branch 2 times, most recently from 26f7b57 to 1b827ae Compare November 1, 2022 17:01
@sam-thibault
Copy link
Contributor

There is a problem with the expectedConfig.Volumes in the test run results. Is there an additional change needed to go with the "spec" vs "x-cluster-spec" change?

Signed-off-by: Drew Erny <derny@mirantis.com>
@dperny
Copy link
Contributor Author

dperny commented Nov 2, 2022

Yeah, I was just missing a struct tag for mapstructure so things weren't parsing out correctly. Should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants