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

Docker support #406

Closed
wants to merge 5 commits into from
Closed

Docker support #406

wants to merge 5 commits into from

Conversation

dumbasPL
Copy link

@dumbasPL dumbasPL commented Apr 1, 2023

This PR adds a Dockerfile as well as CI to build and publish the file to docker hub and GitHub container registry. closes #404

The ci automatically builds and publishes images for all pushes to the master branch (under the master doker tag) as well as only builds (but does not push) images for every pull request to the master branch. On top of that, it builds versioned releases for every version tag in the format of v2.4.7 as well as updates the latest docker tag whenever there is a versioned release.

To make this work maintainers need to set DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets on the repository or organization as well as enable write access to the Github token used in actions to allow it to push to ghcr.io

Multi-platform builds have been temporarily disabled since .NET 6.0 isn't ready for them yet and improved ways of building them have only recently become available.

ref: https://devblogs.microsoft.com/dotnet/improving-multiplatform-container-support/

@yaakov-h
Copy link
Member

yaakov-h commented Apr 2, 2023

since .NET 6.0 isn't ready for them yet

Can you elaborate on this?

I've used docker buildx for this quite nicely in the past, though it fails on the .NET 7 SDK due to interoperability issues with QEMU.


I think the other two important things, other than just having an image available is:

  1. Docs - at least just in the readme - how do we (expect people to) use this?
  2. Storage - how many volumes do users need for persistence (including persistent login), which paths to they map to, and do we need to do anything to simplify this first?

branches:
- 'master'
tags:
- 'v*'
Copy link
Member

Choose a reason for hiding this comment

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

Our releases are usually tagged as DepotDownloader_*.

@dumbasPL
Copy link
Author

dumbasPL commented Apr 2, 2023

though it fails on the .NET 7 SDK due to interoperability issues with QEMU.

Well, the same seems to happen in .NET 6. The job just gets stuck indefinitely. Take a look at the Microsoft article I linked. Once that becomes stable we could move to .NET 8 and use that

Docs - at least just in the readme - how do we (expect people to) use this?

The use cases I see are:

  1. Using the docker container as an "I don't want to install .NET" solution to quickly download and run the app locally. In this case, the user would need to mount a volume for the files to be downloaded to + an optional one for persistent login
  2. In CI (my current use case) to download files and analyze them. This is heavily dependent on what CI solution you use, so I assume that all the user needs to know is where to find the executable and that is already shown as part of the ENTRYPOINT which they will be overwriting anyway.
  3. As a base to build other images (I have a discord bot in mind that I'll do one day). Maybe It would be a good idea to create a wrapper script in /usr/bin that would allow running it for anywhere on the system.

Let me know which of those should be documented or if you have any other use cases.

Storage - how many volumes do users need for persistence (including persistent login), which paths to they map to, and do we need to do anything to simplify this first?

The funny thing is that storage doesn't actually work for me. The account.config gets created but subsequent runs still ask for password/guard. This has never worked for me. Doesn't work on windows either. I'm doing what the help says -username <username> -remember-password but with no luck. I even tried using the latest master inside the docker container and still broken. No idea if IsolatedStorage is breaking something but as I said, it never worked for me no matter the platform or version.

Also for the storage we might need to pick a better location than IsolatedStorage. I don't think it will play well here. Maybe add an environment variable that lets the user overwrite the storage location? this way we could declare it in the Dockerfile to something nice /sentry or something else (lmk what would be a good name)

Our releases are usually tagged as DepotDownloader_*

Ok, that will be require parsing it and extracting semver manually. Will do it later.


Now that I think about it. We probably shouldn't be running as root. So adding a low-privileged user would be a good idea as well.

@yaakov-h
Copy link
Member

yaakov-h commented Apr 2, 2023

Well, the same seems to happen in .NET 6
I've never seen that indefinite hang before, that's very interesting to me.

On .NET 6 I've had Docker just work with buildx, and on .NET 7 I've had failures with an illegal CPU instruction or NullReferenceException.

The funny thing is that storage doesn't actually work for me.

It's broken at the moment due to server-side changes, see #397 for more info.

Also for the storage we might need to pick a better location than IsolatedStorage

We should use something more *nix-y on *nix platforms, IsolatedStorage is a Windows concept. I wouldn't be opposed to configuring it with an environment variable but a better default would also be nice.

@dumbasPL
Copy link
Author

dumbasPL commented Apr 2, 2023

We should use something more *nix-y on *nix platforms

I think we can just use Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) for that. On windows this defaults to %localappdata% (the same place IsolatedStorage seems to use here) and on Linux this defaults to the standard $XDG_DATA_HOME ($HOME/.local/share by default). So just append the project name to that and we're done. Not sure about other *nix but I don't think we need to think too hard about that if we just use the standard API for getting the path.

On .NET 6 I've had Docker just work with build

I think arm64 worked fine but it got stuck on arm/v7. Take a look at the workflow run I posted previously or try running it yourself.

@dumbasPL dumbasPL closed this Apr 3, 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.

Official docker image?
2 participants