filanov / bm-inventory

Apache License 2.0
3 stars 22 forks source link

skipper make all fails if postgres is running locally #494

Open carbonin opened 3 years ago

carbonin commented 3 years ago
$ skipper make all
[skipper] No build container tag was provided
[skipper] Building image using docker file: Dockerfile.bm-inventory-build
Sending build context to Docker daemon  72.29MB
Step 1/8 : FROM golang:1.14.3
 ---> 7e5e8028e8ec
Step 2/8 : ENV GO111MODULE=on
 ---> Using cache
 ---> b669f93357ca
Step 3/8 : RUN apt-get update && apt-get install -y docker.io libvirt-clients awscli python3-pip postgresql  && rm -rf /var/lib/apt/lists/*
 ---> Using cache
 ---> 6f2d7fa7cbf9
...
docker stop postgres || true
Error response from daemon: No such container: postgres
docker run -d  --rm --name postgres -e POSTGRES_PASSWORD=admin -e POSTGRES_USER=admin -p 127.0.0.1:5432:5432 postgres:12.3-alpine -c 'max_connections=10000'
Unable to find image 'postgres:12.3-alpine' locally
12.3-alpine: Pulling from library/postgres
df20fa9351a1: Pull complete 
600cd4e17445: Pull complete 
04c8eedc9a76: Pull complete 
5297ada89a4c: Pull complete 
98abddcccd61: Pull complete 
e1c4a715559d: Pull complete 
45b14c068d3c: Pull complete 
b5953399c544: Pull complete 
Digest: sha256:7693f2082c681571d1dfa66d63f21689192c0c36108f4eb28be0aee0dc285921
Status: Downloaded newer image for postgres:12.3-alpine
428aa15dc79eb2856b6c1e45137b81e62e58e708f3bae29ee592b4e10eec3c62
docker: Error response from daemon: driver failed programming external connectivity on endpoint postgres (f23d63aef7427fb1e4862297b32a4a36db8488beccde287375ae8b1f3d8cdb70): Error starting userland proxy: listen tcp 127.0.0.1:5432: bind: address already in use.
make: *** [Makefile:156: unit-test] Error 125

I guess this is to run tests (I assume a running PG instance isn't needed to build the images)? It would be nice if it could run on a non-standard port so I can also run postgres locally for other applications.

Other options could be to configure against an existing postgres instance or at least call out that postgres cannot be running for skipper make all to succeed in the README.

filanov commented 3 years ago

Maybe we can and env to set optional port?

carbonin commented 3 years ago

I was able to set up my local instance so that the tests would run against that also. But that also requires bypassing skipper (and by extension, make) as make unit-test always starts the container.

We could also document that, but I'm not sure if that's something you want user's doing?

IMO if we're starting our own PG container we should be running it on a non-standard port so as not to conflict with anything. In prod we can, and should, deploy against the standard port, but I don't think it makes sense to pass on the configuration to the person running the tests.

If that's not something you want to do, I think the best option would be just to call out that postgres can't be running locally for skipper to run unit tests.

ronniel1 commented 3 years ago

Maybe we should change the port just for the unit-test

carbonin commented 3 years ago

Maybe we should change the port just for the unit-test

:+1: Yeah, that's what I was thinking.

filanov commented 3 years ago

It's not a parameter supported by default. We will have to change the configuration file inside the container, it's not impossible - just complicate things