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

[RFC] get containerID from /cat/proc/mountinfo #335

Closed

Conversation

steffen-p
Copy link

Hello, this is my first PR in this project. I had some problems with https://github.com/nginx-proxy/nginx-proxy on newer machines running Fedora 33 (nginx-proxy/nginx-proxy#1548) and invested some time for investigation. I just want to give it a shot and share my results.

Analysis

On new distributions like Fedora Server 33 '/cat/proc/cgroup'
does not contain expected container hash. In fact, it remains empty:

$ cat /proc/self/cgroup
0::/

Only guess is that recent support for cgroupv2 in Docker 20.10.0
December 9, 2020 release has some flaws.

Therefore, with this change current container ID is read from
'/cat/proc/mountinfo'. I'm not sure if this information reliable though, therefore RFC.
However, it is the only place containing the current container ID. Also "/proc/1/cpuset" is empty (as proposed in [1]).
I successfully tested on Debian Stretch, running cgroup v1, that '/cat/proc/mountinfo' is working there as well.
In case I should proceed, some alignment with #324 will be needed as well.

Thanks in advance for any comments.

[1] https://github.com/buchdag/docker-gen/commit/8ca1084a608784c5d03692f20040a2984ed3922d

On new distributions like Fedora Server 33 '/cat/proc/cgroup'
does not contain expected container hash. In fact it remains empty:

```
$ cat /proc/self/cgroup
0::/
```

Only guess is that recent support for cgroupv2 in Docker 20.10.0
December 9, 2020 release has some flaws.

Therefore, with this change current container ID is read from
'/cat/proc/mountinfo'.

Signed-off-by: Steffen Pengel <steffen.pengel@gmail.com>
@BrenoMazieiro
Copy link

Please add this! It's a very important change and will affect others distros in the future.

@BrenoMazieiro
Copy link

@steffen-p I believe that this is an lost repo, there is no update since 2018.
Do you recommend any solution?

@steffen-p
Copy link
Author

steffen-p commented Jan 7, 2021

@BrenoMazieiro Thanks for taking a look. Releases of this repository are used in nginx-reverse project, which is definitely active (https://github.com/nginx-proxy/nginx-proxy/blob/master/Dockerfile#L23).

About the patch: It could be seen as just a workaround for a Docker/cgroup2 issue, or as an approach to retrieve container ID for both cgroup versions. Personally, I have some open questions like 'Is this information reliable in any scenarios?' but I'm no cgroup/docker expert and don't want to invest more time in this topic.

@christianuhlmann
Copy link

@steffen-p
can you explain me how to build docker-get with this PR?
i have the same error and want to replace the jwilder/docker-gen with my own sources with this PR.
as alternative can you provide the tar.gz so that i can replace

https://github.com/jwilder/docker-gen/releases/download/$DOCKER_GEN_VERSION/docker-gen-alpine-linux-amd64-$DOCKER_GEN_VERSION.tar.gz
from https://github.com/nginx-proxy/nginx-proxy/blob/master/Dockerfile#L23
with the patched one?

thanks christian

@kanzie
Copy link

kanzie commented Jan 15, 2021

I can confirm this is related to the recent updates to docker itself. I'm on a raspberry pi so I'm struggling with trying to build this for that architecture but it needs this patch either way

@steffen-p
Copy link
Author

I compiled and uploaded a pre-release for testing this patch more easily: https://github.com/steffen-p/docker-gen/releases/tag/0.7.3-29-g84b3509. You can modify the path as @christianuhlmann suggested, or create your own Dockerfile and use the un-tared binary:

Dockerfile

 FROM jwilder/nginx-proxy
 
 COPY docker-gen /usr/local/bin

FYI, I only tested docker-gen-alpine-linux-amd64-0.7.3-29-g84b3509.tar.gz.

HTH and best regards
Steffen

@christianuhlmann
Copy link

christianuhlmann commented Jan 17, 2021

@steffen-p
thank you for the pre-release files
i build my own version with your docker-gen
docker-gen -version 0.7.3-29-g84b3509
but i get the same error as before:
Error running notify command: nginx -s reload, exit status 1 2021/01/17 10:42:40 [emerg] 60#60: no servers are inside upstream in /etc/nginx/conf.d/default.conf:58 nginx: [emerg] no servers are inside upstream in /etc/nginx/conf.d/default.conf:58

any idea?

@steffen-p
Copy link
Author

steffen-p commented Jan 18, 2021

@christianuhlmann

any idea?

Not without knowing the full picture. I can give some more details on my test though:

reverse-proxy-test ▶  tree
.
├── docker-compose.yml
└── reverse-proxy
    ├── Dockerfile
    ├── docker-gen
    └── docker-gen-alpine-linux-amd64-0.7.3-29-g84b3509.tar.gz

1 directory, 4 files
reverse-proxy-test ▶  cat docker-compose.yml 
version: '3'

services:
  proxy:
    build: ./reverse-proxy
    ports:
      - "80:80"
    volumes:
      - /var/run/docker.sock:/tmp/docker.sock:ro

  whoami:
    image: jwilder/whoami
    environment:
      - VIRTUAL_HOST=whoami.local
      - VIRTUAL_PORT=80

Please check if docker-gen version running inside your container shows the expected version

reverse-proxy-test ▶ docker exec -it reverse-proxy-test_proxy_1 docker-gen --version
0.7.3-29-g84b3509

If upstream is set correct by docker-gen you should be able to do:

reverse-proxy-test ▶  curl -H "Host: whoami.local" localhost
I'm d99e46499ce0

@christianuhlmann
Copy link

christianuhlmann commented Jan 20, 2021

@steffen-p

hi

i did the test with the same parameter and but the error is not gone.

os: debian bullseye (testing)
docker: 20.10.23-0debian-buster
kernel: 5.10.0-1-amd64

docker exec -it reverse-proxy-test_proxy_1 docker-gen --version 0.7.3-29-g84b3509

curl -H "Host: whoami.local" localhost curl: (7) Failed to connect to localhost port 80: Verbindungsaufbau abgelehnt

docker container logs -f --details reverse-proxy-test_proxy_1
WARNING: /etc/nginx/dhparam/dhparam.pem was not found. A pre-generated dhparam.pem will be used for now while a new one
is being generated in the background. Once the new dhparam.pem is in place, nginx will be reloaded.
forego | starting dockergen.1 on port 5000
forego | starting nginx.1 on port 5100
dockergen.1 | 2021/01/20 08:37:22 Generated '/etc/nginx/conf.d/default.conf' from 41 containers
dockergen.1 | 2021/01/20 08:37:22 Error running notify command: nginx -s reload, exit status 1
dockergen.1 | 2021/01/20 08:37:22 Watching docker events
dockergen.1 | 2021/01/20 08:37:23 Contents of /etc/nginx/conf.d/default.conf did not change. Skipping notification 'nginx -s reload'
dockergen.1 | 2021/01/20 08:37:23 Contents of /etc/nginx/conf.d/default.conf did not change. Skipping notification 'nginx -s reload'
2021/01/20 08:37:28 [emerg] 56#56: no servers are inside upstream in /etc/nginx/conf.d/default.conf:58
nginx: [emerg] no servers are inside upstream in /etc/nginx/conf.d/default.conf:58

Generating DSA parameters, 4096 bit long prime
dhparam generation complete, reloading nginx
dockergen.1 | 2021/01/20 08:37:38 Received event start for container bf19c840f4ce
dockergen.1 | 2021/01/20 08:37:39 Received event die for container bf19c840f4ce

is this problem dist specific?

do you have any idea?

@NicolasDorier
Copy link
Contributor

NicolasDorier commented Jan 21, 2021

NicolasDorier added a commit to btcpayserver/docker-gen that referenced this pull request Jan 21, 2021
NicolasDorier added a commit to btcpayserver/docker-gen that referenced this pull request Jan 21, 2021
NicolasDorier added a commit to btcpayserver/docker-gen that referenced this pull request Jan 21, 2021
@NicolasDorier
Copy link
Contributor

I made an alternative PR which should do the work.

shyim pushed a commit to shyim/docker-gen that referenced this pull request Jan 22, 2021
@develroo
Copy link

Have to weigh in here because same issue happening to me but I am on docker-gen --version 0.7.4

Host is Debian/Testing and uses docker Docker version 20.10.2+dfsg1, build 2291f61

Same error when passing VIRTUAL_HOST and VIRTUAL_PORT from the app container.

Pulled the latest image

  sha256:5e6f392e99e5454851caa8a3dbb021560a32375edff772b64f862c4d7831d038

@rodrigoaguilera
Copy link

I plan to install @steffen-p fork on the nginx-proxy image and test with cgroup v1 and v2 but I wonder why it is based on 0.7.3 instead of 0.7.4. Any reason for that?

@develroo
Copy link

Dunno.. but I was hoping the newer version was going to solve the issue but it did not. nginx-proxy image gives that docker-gen version but it fails exactly because of that. And in fact cat /proc/mountinfo doesn't even exist in my container

 nginx -t
2021/02/18 14:22:44 [emerg] 149#149: no servers are inside upstream in /etc/nginx/conf.d/default.conf:68
nginx: [emerg] no servers are inside upstream in /etc/nginx/conf.d/default.conf:68
nginx: configuration file /etc/nginx/nginx.conf test failed

I can see from strace it is still opening /proc/self/cgroup which is still failing as described here

openat(AT_FDCWD, "/proc/sys/net/core/somaxconn", O_RDONLY|O_CLOEXEC) = 3
openat(AT_FDCWD, "/proc/self/cgroup", O_RDONLY|O_CLOEXEC) = 3
read(5, "\"OM_HOME=/opt/openmeetings\",\"MYS"..., 4096) = 2199

bash-5.0# cat /proc/self/cgroup 
0::/


I wonder if it has to do with the containment on the VPS? I am going to try it on a different provider and see what happens. 

@rodrigoaguilera
Copy link

🤔
I see /proc/self/mountinfo in the containers running on docker 20.10.3 with cgroups v2

 Server Version: 20.10.3
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2

and also the containers running on 19.03.10 with cgroup v1

Server Version: 19.03.10
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: cgroupfs

@NicolasDorier I'm not sure about using /etc/hostname as that can be overridden by passing a different hostname and it won't be the container ID, but in the case of nginx-proxy it will work because the container will be visible in the network by that hostname.

The mountinfo solution looks better but I am worried about the cases in which mountinfo is not available inside the container.

@steffen-p
Copy link
Author

@rodrigoaguilera I based it on master rather than on 0.7.4. Reason is, all changes form 0.7.4 are contained in master branch, except 6455e0c ("Update 0.7.4") is not part of master branch. If I rebased onto tag 0.7.4 a some fixes would be left out.

@rodrigoaguilera
Copy link

Now I understand by inspecting git history of the repo, 7.3 is hardcoded on master.

I hope this issue gets some attention.

bugficks pushed a commit to bugficks/docker-gen that referenced this pull request Feb 28, 2021
@mejo-
Copy link

mejo- commented Mar 9, 2021

I can confirm that this patch fixes nginx-proxy/nginx-proxy#1548.

@hathagat
Copy link

I have the same problem as @christianuhlmann.
I run docker-gen and Nginx in two seperate containers and use @steffen-p's build just like here, but upstream remains empty.

$ uname -v
#1 SMP Debian 5.10.13-1~bpo10+1 (2021-02-11)
$ docker -v
Docker version 20.10.5, build 55c4c88
$ docker exec -it docker-gen docker-gen --version
0.7.3-29-g84b3509

Do you guys have any suggestions on what I can do to get my proxy back in use?

@hathagat
Copy link

hathagat commented Mar 29, 2021

If anyone else is also having the problem, I fixed the error I got:
PR steffen-p#1
Binary https://github.com/hathagat/docker-gen/releases/tag/0.7.3-d720016
Image hathagat/docker-gen:0.7.3-d720016

@pentix
Copy link

pentix commented Apr 2, 2021

After modifying the original nginx-proxy Dockerfile to get the sources from @steffen-p and @hathagat I still faced the same error.

Creating my own Dockerfile as suggested here didn't work either, despite the fact that the right binary there. (Verified, by running docker-gen --version)

/proc/self/mountpoints actually lists the container ID in a way @steffen-p's Regex should be able to catch it, I have no idea why it doesn't.

Networks are setup correctly, other containers are reachable from the ǹginx-proxycontainer; Also, setting the server's IP manually in theupstreamarea of thedefault.conf` fixes the problem, so it's only the discovery, that doesn't really work... 😬

jwilder added a commit that referenced this pull request Apr 3, 2021
Fix docker-gen breaking on new docker version (See #335)
@jwilder
Copy link
Collaborator

jwilder commented Apr 3, 2021

Fixed via #336

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.