Push docker images to Azure Container Registry instead of personal docker hub, Fix our docker image and add docker-compose files. #178
Conversation
…try instead of DockerHub Change docker image location from DockerHub to Azure Container Registry
Mathos1432
requested review from
Aniruddh25,
JelteF,
gledis69,
jarupatj,
mbhaskar and
seantleonard
as code owners
seantleonard
left a comment
seantleonard
left a comment
There was a problem hiding this comment.
Looks good, Matt. Do you mind updating PR description to include sentence how this new registry keeps the container private internally until we are ready for preview? I'm not familiar with whether that folder is public or not.
|
Can you update README on how to consume/use this docker image? |
Co-authored-by: Aniruddh Munde <[email protected]>
Mathos1432
changed the title
|
@Aniruddh25 I'll need to update the config files I'm linking to in the docker compose files I added once #181 is checked in. |
seantleonard
left a comment
seantleonard
left a comment
There was a problem hiding this comment.
Thanks for validating! just a few clarification comments as follow up to your recent commits.
Aniruddh25
left a comment
Aniruddh25
left a comment
There was a problem hiding this comment.
Overall looks good, need few more config file changes.
| # Build a docker image and push it to the container registry. | ||
| - task: Docker@2 | ||
| displayName: "Build and push docker image to Azure Container Registry" |
There was a problem hiding this comment.
Thought for future changes: this step could be set to only run upon merging to main instead of subsequent updates to a pending PR. Not sure we'd want pending PR changes to be published in an image to our ACR.
There was a problem hiding this comment.
I agree, but I'd probably do that in a separate change since I need to look into how it would work.
| </Content> | ||
| <Content Include="PostgreSqlBooks.sql"> | ||
| <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> | ||
| <CopyToOutputDirectory>$(CopyToOutputDirectoryAction)</CopyToOutputDirectory> |
There was a problem hiding this comment.
How accessible are the files within a container image? the .sql files are for configuring a database prior to starting the runtime, so may not have much utility being copied to output directory.
There was a problem hiding this comment.
Ah, then we probably don't need them to be copied to the output directory. I kept the existing behavior, but I can change it to never copy
| 5. The container should be accessible at localhost:5000 | ||
|
|
||
| ## Managing the Pipeline | ||
| The pipeline has permissions to push to this ACR through this service connection in ADO: https://msdata.visualstudio.com/CosmosDB/_settings/adminservices?resourceId=6565800e-5e71-4e19-a610-6013655382b5. |
There was a problem hiding this comment.
Thanks for adding this :)
…instead of using release for that
Why is this change being made?
We're currently pushing our images to my personal dockerhub, but we should have them in ACR.
This will allow us to give access to people who need them and move us away from using a personal dockerhub.
Our docker image isn't working currently, it's trying to run dotnet DataGateway.Service.dll, but that dll has been renamed Azure.DataGateway.dll.
What changed?
How was this validated?
docker compose -f docker-compose-cosmos.yml upbooted a container image and was accessible through postman.