WIP: One compose to rule them all #150

Tyrrrz · 2021-05-16T18:37:43Z

Moved dockerfiles to root because:

NuGetTrends.Web depends on other projects outside of its own directory
It's a bit cleaner this way

Also I noticed that the existing dockerfile in NuGetTrends.Web didn't actually build the project but merely copied the publish output produced by dotnet. This works for CI, but makes local development harder. So I integrated the build process inside the dockerfile.

Because /Portal is inside /NuGetTrends.Web, I did some naive filtering to avoid copying unnecessary files into the image. Long term this could be improved with a dockerignore file. Also, I would suggest to move the frontend project to a separate top level directory next to /NuGetTrends.Web and others.

Update: just realized that you're serving the angular project from asp.net core. This might make it difficult to separate the two services. I have never used this approach so I'm not really sure what's the best solution in this case.

So far, I got the angular project running, but backend fails with 500 most likely because it can't serve the spa. What do you say about divorcing the two services and keeping them entirely separate? This would allow you to host them separately as well, making use of CDN on frontend with platforms like Netlify and Vercel.

Closes #147

codecov · 2021-05-16T18:42:29Z

Codecov Report

Merging #150 (403bd1b) into main (1b9b14c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #150   +/-   ##
=======================================
  Coverage   90.97%   90.97%           
=======================================
  Files          28       28           
  Lines         432      432           
  Branches       32       32           
=======================================
  Hits          393      393           
  Misses         29       29           
  Partials       10       10

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b9b14c...403bd1b. Read the comment docs.

Tyrrrz · 2021-05-16T19:05:19Z

Bruno told me that you guys had separate SPA/Backend before but decided to merge it. I think it would not be possible to launch backend separate from SPA or vice versa with this approach. It would still be possible to have docker compose that runs everything at once, but without the docker-compose-without-portal.ps1 etc scripts.

joaopgrassi · 2021-05-16T19:31:30Z

Yes we merged them a while ago and now the API serves the SPA in prod. If running in Development, the API just proxies to the angular server (spa.UseProxyToSpaDevelopmentServer("http://localhost:4200");. This is so it's faster to change the front-end and hot-reload. The experience by having the API serve the front-end also in dev was really bad, slow times to rebuild and refresh.

For purpose of documentation, the reason why we merged them was mainly because the deployment process was not nice and since the API is pretty much tailored for the front-end, doesn't make much sense to have them separated. Now with one docker image we have both running and publishing a new version to Prod is just fetching the new image.

I think your initial idea makes sense to improve the contributing experience:

Have the Dockerfile for the SPA, exposing port 4200 (same when running with Angular serve)
Have the Dockerfile for the API, like you did
Add both SPA and API to compose and control which to run with the ps1/sh scripts

if we do this, I think it already improves things because:

People working only on the SPA, can just run compose for the db and API. They will run the SPA via ng serve
People working on the API (or Scheduler..) and want to test it, can run compose for the db and SPA.

The CI build was done in this way (copy files) mainly because the .Web project expects the dist folder of the SPA files in place before doing dotnet publish (see the PublishRunWebpack task in the csproj) and we didn't want to run ng build together with the dotnet commands since that is just slow and no real output of what is going on. If you build on VS/Rider it's even worse as it takes ages and there's no indication in the Output =/. And since we build the SPA to run the tests in CI, didn't make sense to run it twice (in CI and inside the container) which is doing it now, at least for the API I guess?

joaopgrassi · 2021-05-16T19:37:12Z

I would still have the individual docker files, like you did, but I would keep the CI build and the Dockerfile inside the Web project the same as it was, since those are just for our deployments and not for local development.

joaopgrassi · 2021-05-16T19:41:47Z

NuGetTrends.Web.Portal.dockerfile

+
+COPY --from=build /app/dist /usr/share/nginx/html
+
+ENTRYPOINT ["nginx", "-g", "daemon off;"]


If this is just for local development, couldn't we just use ng serve as the entrypoint and use the Angular dev server instead of nginx?

joaopgrassi · 2021-05-16T19:43:34Z

NuGetTrends.Web.dockerfile

+
+COPY --from=build /src/NuGetTrends.Web/artifacts ./
+
+ENTRYPOINT ["dotnet", "NuGetTrends.Web.dll"]


Like I mentioned in the comments, this docker file is used to produce our image that we use for deployment. I guess now it's broken since we don't have the dist folder present here. I don't think it's that problematic that the files are copied. dotnet publish does not include the node_modules folder already, so maybe we could also enhance it to exclude the rest if we want to be optimal.

Edit: I just tested and dotnet publish only includes the dist folder, so we are good.

joaopgrassi · 2021-05-16T19:55:06Z

docker-compose-without-portal.ps1

@@ -0,0 +1 @@
+& docker compose up rabbitmq postgres pgadmin web --build


Would be nice to also have the same in bash scripts :)

joaopgrassi · 2021-05-16T19:55:52Z

src/NuGetTrends.Web/Dockerfile

@@ -1,5 +0,0 @@
-FROM mcr.microsoft.com/dotnet/aspnet:5.0


I would keep this so our deployment/CI process doesn't change.

Tyrrrz · 2021-05-20T17:30:37Z

Just noting here that I didn't really have time to spend on it this week, will come back to it a bit later~

tonyqus · 2022-01-17T02:05:27Z

What's the status of this PR? It has been there for a while.

Tyrrrz · 2022-01-17T13:34:20Z

What's the status of this PR? It has been there for a while.

I completely forgot what I was doing 😅
I'll close this PR because in all honesty I don't see myself returning to it any time soon.

tonyqus · 2022-01-17T19:29:21Z

This PR still makes sense although it's not complete.

@Tyrrrz Btw, I notice you are also a MVP. Maybe you can join .NET foundation although it's somewhat in mess so far. We need hands to make it better.

Tyrrrz · 2022-01-18T17:39:35Z

This PR still makes sense although it's not complete.

It can be reopened if someone wishes to continue, but I feel like it'd be easier to start over and maybe copy things over from here.

@Tyrrrz Btw, I notice you are also a MVP. Maybe you can join .NET foundation although it's somewhat in mess so far. We need hands to make it better.

Thanks, but I have no faith nor interest in .NET foundation 🙂

One compose to rule them all

e80219b

Tyrrrz requested review from bruno-garcia and joaopgrassi as code owners May 16, 2021 18:37

Tyrrrz changed the title ~~One compose to rule them all~~ WIP: One compose to rule them all May 16, 2021

Tyrrrz added 2 commits May 16, 2021 21:37

asd

3a809bb

adjust ports

951cd50

wip

403bd1b

joaopgrassi requested changes May 16, 2021

View reviewed changes

Tyrrrz closed this Jan 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: One compose to rule them all #150

WIP: One compose to rule them all #150

Tyrrrz commented May 16, 2021 •

edited

Loading

codecov bot commented May 16, 2021 •

edited

Loading

Tyrrrz commented May 16, 2021

joaopgrassi commented May 16, 2021 •

edited

Loading

joaopgrassi commented May 16, 2021 •

edited

Loading

joaopgrassi May 16, 2021

joaopgrassi May 16, 2021

joaopgrassi May 16, 2021

joaopgrassi May 16, 2021

Tyrrrz commented May 20, 2021

tonyqus commented Jan 17, 2022

Tyrrrz commented Jan 17, 2022

tonyqus commented Jan 17, 2022

Tyrrrz commented Jan 18, 2022


		COPY --from=build /app/dist /usr/share/nginx/html

		ENTRYPOINT ["nginx", "-g", "daemon off;"]


		COPY --from=build /src/NuGetTrends.Web/artifacts ./

		ENTRYPOINT ["dotnet", "NuGetTrends.Web.dll"]

		@@ -0,0 +1 @@
		& docker compose up rabbitmq postgres pgadmin web --build

WIP: One compose to rule them all #150

WIP: One compose to rule them all #150

Conversation

Tyrrrz commented May 16, 2021 • edited Loading

codecov bot commented May 16, 2021 • edited Loading

Codecov Report

Tyrrrz commented May 16, 2021

joaopgrassi commented May 16, 2021 • edited Loading

joaopgrassi commented May 16, 2021 • edited Loading

joaopgrassi May 16, 2021

Choose a reason for hiding this comment

joaopgrassi May 16, 2021

Choose a reason for hiding this comment

joaopgrassi May 16, 2021

Choose a reason for hiding this comment

joaopgrassi May 16, 2021

Choose a reason for hiding this comment

Tyrrrz commented May 20, 2021

tonyqus commented Jan 17, 2022

Tyrrrz commented Jan 17, 2022

tonyqus commented Jan 17, 2022

Tyrrrz commented Jan 18, 2022

Tyrrrz commented May 16, 2021 •

edited

Loading

codecov bot commented May 16, 2021 •

edited

Loading

joaopgrassi commented May 16, 2021 •

edited

Loading

joaopgrassi commented May 16, 2021 •

edited

Loading