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 Image on Windows Base Image #439 #2332

Merged
merged 18 commits into from
Jan 16, 2025
Merged

Docker Image on Windows Base Image #439 #2332

merged 18 commits into from
Jan 16, 2025

Conversation

jwdb
Copy link
Contributor

@jwdb jwdb commented Jan 3, 2024

With this PR it's possible to build a Windows version of the Azurite container. I've not added the required steps to the package.json since the current steps very much look internally used and I would be unable to test them.

It is based upon the work from @shanrath in the issue (#439), expanding it with a NodeJS baseimage because of the lack of a proper nodejs windows baseimage that can be trusted to be updated regularly.

The image is based upon the LTSC 2022 version of Windows Nanoserver.


Thanks for contribution! Please go through following checklist before sending PR.

PR Branch Destination

  • For Azurite V3, please send PR to main branch.
  • For legacy Azurite V2, please send PR to legacy-dev branch.

Always Add Test Cases

Make sure test cases are added to cover the code change.

Add Change Log

Add change log for the code change in Upcoming Release section in ChangeLog.md.

Development Guideline

Please go to CONTRIBUTION.md for steps about setting up development environment and recommended Visual Studio Code extensions.

@jwdb
Copy link
Contributor Author

jwdb commented Jan 3, 2024

@microsoft-github-policy-service agree

@blueww
Copy link
Member

blueww commented Jan 4, 2024

@jwdb

Thanks for the contribution!

Would you please confirm:
Would you like Azurite team to release windows base docker image, or just keep this script in Azurite to help customer create image locally?

If you would like Azurite to release windows based docker image, please also add:

  1. Update package.json so we can build the docker image from npm command. You can refer this PR: feat: add scripts for building multi-architecture azurite docker images #1569
  2. Add Unit test for windows base docker image build / run. You can refer:
    - job: docker

If you just would like to keep this script in Azurite repo, would you please:
Give the detail steps to build windows based docker image, and better update Readme.md with it (if the step is long, you can add a doc into https://github.com/Azure/Azurite/tree/main/docs/designs, and link Readme.md with it.).

@blueww blueww self-requested a review January 4, 2024 07:47
@blueww blueww self-assigned this Jan 4, 2024
@jwdb
Copy link
Contributor Author

jwdb commented Jan 4, 2024

Hi @blueww,

I would like the image to be released, that way it's easier to use & make it easier for the windows image to stay up-to-date with the latest version of Azurite.

I've added the build steps I think are neccesary to the package.json & azure-pipelines.yml.

@blueww
Copy link
Member

blueww commented Jan 4, 2024

It looks there are something wrong with the pipeline change, only 3 PR checks run, but originally there are 30+ PR checks.

"docker:publish-arm64": "cross-var docker push xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-arm64",
"docker:create-manifest-versioned": "cross-var docker manifest create xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-amd64 xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-arm64",
"docker:create-manifest-latest": "cross-var docker manifest create xstoreazurite.azurecr.io/public/azure-storage/azurite:latest xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-amd64 xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-arm64",
"docker:create-manifest-versioned": "cross-var docker manifest create xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-amd64 xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-windows-amd64 xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-arm64",
Copy link
Member

Choose a reason for hiding this comment

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

Just to check what's the effect after publish the manifest.
With the originally manifest, when user run Azurite docker image without specifying platform, docker will choose the docker image most match the current platform (ARM64 platform will choose AMD64 docker image, AMD64 platform will choose AMD64 docker image). After the windows-amd64 image is added, what will happen when user run Azurite docker image without specifying platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The platform is chooses includes the os name, hence it currently throwing an error on windows ( no matching manifest for windows/amd64 in the manifest list entries) adding this image will add that specific platform to the manifest.

Copy link
Member

Choose a reason for hiding this comment

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

With current Azurite release (before the change), I will install AMD64 docker image in my machine (my machine is windows with AMD64).
So after your change, what will happen on different platform/OS?

And for the error, do you mean you will resolve it, or it's already resolved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are running Docker in Windows Container mode (this is an option within Docker-Desktop), with the current release this will throw an error.
After the change, the other platforms will be unaffected (since they will use the linux/amd64 image).

You can view the platform docker is running internally on with docker info -f "{{.OSType}}". For me this gives windows.

Copy link
Member

@blueww blueww Jan 4, 2024

Choose a reason for hiding this comment

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

Get it. Thanks!

And on my windows machine run docker info -f "{{.OSType}}", will get result "linux".

@blueww
Copy link
Member

blueww commented Jan 4, 2024

I tried to build windows based image in code space, and meet following error:

@blueww ➜ /workspaces/Azurite (main) $ npm run docker:build-windows-amd64

> [email protected] docker:build-windows-amd64
> cross-var docker buildx build --platform windows/amd64 --load --no-cache --rm -f "Dockerfile.Windows" -t xstoreazurite.azurecr.io/public/azure-storage/azurite:$npm_package_version-windows-amd64 .

[+] Building 0.8s (2/2) FINISHED                                                                                                                                                                                                                                                  docker:default
 => [internal] load build definition from Dockerfile.Windows                                                                                                                                                                                                                                0.4s
 => => transferring dockerfile: 1.18kB                                                                                                                                                                                                                                                      0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                           0.4s
 => => transferring context: 282B                                                                                                                                                                                                                                                           0.0s
Dockerfile.Windows:34
--------------------
  32 |     # Production image
  33 |     #
  34 | >>> FROM Node-Windows
  35 |     
  36 |     ENV NODE_ENV=productions
--------------------
ERROR: failed to solve: failed to parse stage name "Node-Windows": invalid reference format: repository name must be lowercase

@jwdb
Copy link
Contributor Author

jwdb commented Jan 4, 2024

Interesting enough I do not encounter that error when running it locally.
I've changed the Dockerfile for a lowercase stage name.
Also I saw that the Azure pipeline error'ed on the curl, I've added a workdir change to hopefully mitigate that issue.

@blueww
Copy link
Member

blueww commented Jan 4, 2024

Interesting enough I do not encounter that error when running it locally. I've changed the Dockerfile for a lowercase stage name. Also I saw that the Azure pipeline error'ed on the curl, I've added a workdir change to hopefully mitigate that issue.

If possible, I would suggest you testing the build docker image step in the github code space (it's linux based), and make sure it passes.

@jwdb
Copy link
Contributor Author

jwdb commented Jan 4, 2024

It is not possible to build or run a Windows container on a Linux machine, as far as I know there isn't a (proper) way to run Windows on a Github codespace.

Dockerfile.Windows Outdated Show resolved Hide resolved
@blueww
Copy link
Member

blueww commented Dec 11, 2024

It looks the PR check still fail.
And please resolve the conflict.

@jwdb
Copy link
Contributor Author

jwdb commented Jan 6, 2025

The PR Checks succeeded, are there any stoppers left for this PR?

ChangeLog.md Show resolved Hide resolved
Dockerfile.Windows Outdated Show resolved Hide resolved
@blueww
Copy link
Member

blueww commented Jan 7, 2025

@XiaoningLiu , @EmmaZhu
Would you please help to review, and see if any potential issue?


WORKDIR C:\\Node\\node-v20.0.0-win-x64\\

CMD "azurite -l c:/data --blobHost 0.0.0.0 --queueHost 0.0.0.0 --tableHost 0.0.0.0"
Copy link
Member

@blueww blueww Jan 7, 2025

Choose a reason for hiding this comment

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

The first docker run command sample in Readme file(link) will fail per my test.
It looks the command in line 71 doesn't work.

D:\code\azurite>docker run -p 10000:10000 -p 10001:10001 -p 10002:10002 xstoreazurite.azurecr.io/public/azure-storage/azurite:latest
'"azurite -l c:/data --blobHost 0.0.0.0 --queueHost 0.0.0.0 --tableHost 0.0.0.0' is not recognized as an internal or external command, operable program or batch file.

If we run with customized command like following, it works:

D:\code\azurite>docker run -p 10000:10000 -p 10001:10001 -p 10002:10002 xstoreazurite.azurecr.io/public/azure-storage/azurite:latest azurite --blobHost 0.0.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should function now again, there is some weird escaping done with the CMD command on Windows containers

@blueww
Copy link
Member

blueww commented Jan 9, 2025

Thanks @jwdb for the fix!
It work well in my testing. I build the docker image with "npm run docker:build-windows", and run the docker image works well.

@XiaoningLiu , @EmmaZhu,

Would you please help to review and see if the change is good for you?

@blueww blueww merged commit c3f9344 into Azure:main Jan 16, 2025
35 checks passed
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