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

Swarm csi #173

Merged
merged 13 commits into from
Jul 18, 2024
Merged

Swarm csi #173

merged 13 commits into from
Jul 18, 2024

Conversation

Gradlon
Copy link
Contributor

@Gradlon Gradlon commented Jul 17, 2024

Build scripts and Documentation for docker Swarm CSI.

deploy/swarm/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

this file mostly duplicates content in cmd/seaweedfs-csi-driver/swarm/README.md

maybe just remove this, or add a reference to the other file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.
Can you do this?

@chrislusf
Copy link
Contributor

chrislusf commented Jul 18, 2024

Thanks for this PR! In later PRs, I wonder whether there are some way to add an integration test for this, similar to https://github.com/seaweedfs/seaweedfs/blob/master/.github/workflows/e2e.yml , which can also serve as a usage example.

@chrislusf chrislusf merged commit 22f52af into seaweedfs:master Jul 18, 2024
1 of 4 checks passed
@Gradlon
Copy link
Contributor Author

Gradlon commented Jul 22, 2024

I will delete my fork.

About e2e Test.
A real E2E test would mean spinning up a docker swarm cluster.
We would maybe need to discuss/you tel me you would go about this.

Thank you for this Project, I am using seaweed in my Cluster, and I am pretty happy with it!

If I can contribute more, I will.

One thing that I see is that the log output of the plugin appears as errors in docker logs.
This is not a big issue but also not right ;)

can this be adjusted?

@chrislusf
Copy link
Contributor

A real E2E test would mean spinning up a docker swarm cluster. We would maybe need to discuss/you tel me you would go about this.

Yes. That would be nice. I do not know much here though.

One thing that I see is that the log output of the plugin appears as errors in docker logs. can this be adjusted?

Yes. weed -logtostderr=false ...

@blib
Copy link

blib commented Sep 8, 2024

please note that in docker 27.x.x CSI plugin interface is broken

moby/moby#48133
moby/swarmkit#3181

@Gradlon
Copy link
Contributor Author

Gradlon commented Sep 16, 2024

@blib You are right.
This is the issue for several ported CSI Plugins, e.g. Hetzner CSI is also broken.
@chrislusf Is there a fix planet?

@blib
Copy link

blib commented Sep 17, 2024

it's not a seaweedfs problem. I submitted PR moby/swarmkit#3181 you can up vote it :)

@Gradlon
Copy link
Contributor Author

Gradlon commented Sep 17, 2024

@blib Ah thank you for the correction, i got that wrong then.
I upvoted the PR ;)

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