ucphhpc / docker-migrid

Containerized MiG
GNU General Public License v2.0
3 stars 7 forks source link

Improve Docker handling in Makefile #13

Closed benibr closed 1 year ago

benibr commented 1 year ago

This PR consists of some smaller changes to the docker handling which I made during working with the docker-migrid test setups.

rasmunk commented 1 year ago

Would it be possible and perhaps better to use a variable arch instead as in https://stackoverflow.com/questions/60251383/dockerfile-from-platform-option to avoid having it hard-coded in Dockerfile(s)?

Configurable OWNER makes sense.

I suppose the next stage markers are just to make build output more clear. No problem.

docker compose as fallback is good.

stateclean target is fine, real prompt before destructive clean up and sensible target reuse is better :-)

I remember @rasmunk had some reservations about cleaning up other local containers in certain multi purpose setups, but I hope it's okay with dockerclean. Otherwise please protest, @rasmunk .

I don't see any other blockers for merging.

Yes,

I would prefer if we didn't do ${DOCKER} system prune -f as part of the dockerclean target. I know it only removes stopped and dangling containers, but sometimes it is nice to have the stopped containers around for checking logs and such. In addition, it might inadvertently remove a custom created Docker network that is to be used by another stack.

Other than that, I'm happy with it:) GJ!

benibr commented 1 year ago

Hi @rasmunk, I see the point that removing stopped containers is not good as part of dockerclean. I added it cause I wanted to get rid of the build cache to completely rebuild from scratch.

I guess it could make sense to replace it with docker image prune instead to just remove dangling images. Would that be alright?

benibr commented 1 year ago

@jonasbardino I added the $ARCH argument to Dockerfiles and set the default to amd64.

I don't see a reason for now to add it to all docker-compose files yet though. As long as it's explicitly defined, my macbook is happy :-)

rasmunk commented 1 year ago

Hi @rasmunk, I see the point that removing stopped containers is not good as part of dockerclean. I added it cause I wanted to get rid of the build cache to completely rebuild from scratch.

I guess it could make sense to replace it with docker image prune instead to just remove dangling images. Would that be alright?

@benibr,

Hmm, yes I think that should work well, if you just use it to get rid of the dangling image layers:))

benibr commented 1 year ago

@rasmunk I changed the commit accordingly.

rasmunk commented 1 year ago

@rasmunk I changed the commit accordingly.

Great @benibr! But it looks like on my end that you added ${DOCKER} container prune -f instead of ${DOCKER} image prune -f. Is this just a typo?

benibr commented 1 year ago

Whoa, thx, yep that was a typo. fixed