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

Adding Windows Support #194

Closed
wants to merge 4 commits into from
Closed

Adding Windows Support #194

wants to merge 4 commits into from

Conversation

saedx1
Copy link

@saedx1 saedx1 commented Mar 3, 2023

Issue #, if available:
#131

Description of changes:

  • Closing a file before renaming it as this causes a problem on windows: windows cannot access file because another process is using it.
  • In the translation of paths, unfortunately, os.PathSeparator isn't sufficient for windows. This is because windows also accepts / as a separator. Go's official path_windows.go actually does the same thing I'm adding https://go.dev/src/os/path_windows.go - see the first 16 lines of that file.
  • I did build the image and push it to my personal github public container registry, so I can confirm that Dockerfile.win has been tested.
  • I tested this on EKS with a windows node running 6 or more pods using secrets (as volumes and as env vars).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@saedx1 saedx1 force-pushed the main branch 2 times, most recently from 00cc12e to 0e87292 Compare March 5, 2023 16:51
- Adjust Go Code to Support Windows by adjusting path separator and closing the file before renaming it
- Add dockerfile
- Add k8s manifest
@jbct
Copy link

jbct commented Mar 6, 2023

Thank you for the PR. I'm going to close it at the moment, because we don't currently have plans to add Windows support. As an option, you may investigate using the driver-writes-secrets flag which may work around the issue you're encountering. Let's use #131 for discussion.

@jbct jbct closed this Mar 6, 2023
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.

2 participants