sqitchers / docker-sqitch

Docker Image packaging for Sqitch
MIT License
35 stars 39 forks source link

Use of uninitialized value $gid in getgrgid #17

Closed agdespopoulos closed 4 years ago

agdespopoulos commented 4 years ago

Version 1.0.0 Ubuntu 18.04.02 Using docker-sqitch.sh (https://github.com/sqitchers/docker-sqitch/commit/58a5c32473c711bb9da1ade025a4eb3124ddb8b3)

When running commands that perform certain actions on the database, I see a warning about "Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.". It doesn't seem to impact the operation of Sqitch at all, but it is noisy and annoying, and I'm not sure why it's happening.

Running the script with sudo resolves the problem, but I don't have any other problems with things like Sqitch not being able to create files and directories when not running with sudo, just this warning.

Possibly similar to sqitchers/sqitch#459? I tried the debugging steps there and nothing helped. Here's some more info that might hopefully help?

getent output:

backend/db » user=${USER-$(whoami)}; getent passwd "$user"       
adespopoulos:x:1001:1001:Ag Despopoulos,,,:/home/adespopoulos:/usr/bin/zsh

Added an echo to docker-sqitch.sh to print all the environment variables it passes to the docker container:

SQITCH_ORIG_SYSUSER=adespopoulos -e SQITCH_ORIG_EMAIL=adespopoulos@pooter -e TZ=EDT -e LESS=-R -e SQITCH_ORIG_FULLNAME=Ag Despopoulos -u 1001:1001 -e HOME=/home

Example:


backend/db » sqitch --version
sqitch (App::Sqitch) v1.0.0

backend/db » sqitch revert HEAD^
Revert changes to appschema from local? [Yes]
  - inspection .. Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.
ok

backend/db » sqitch verify
Verifying local
  * appschema .. Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.
ok
Undeployed change:
  * inspection
Verify successful

backend/db » sqitch deploy
Deploying changes to local
  + inspection .. Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.
Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.
ok
``
theory commented 4 years ago

I believe that PgCommon is some custom library Debian/Ununtu provides. I don’t know what it is, what it does, or why it exists. I’m starting to regret using Ubuntu for the docker image. sigh. Anyway, I’ve never been able to chase down the source of this issue, but I think it’s because when you run the Docker image on Linux, the sqitch-docker.sh script sets the uid and gid to the values on the host, and those don’t exist inside the container. When you run it as root, it gets set to root, which of course does exist inside the container. It’s essential to set the UID properly, however, because otherwise sqitch add would fail, because it wouldn’t be able to create the files in the current directory (or sub directory, ./deploy and friends) with some other UID and GID that don’t exist on your host. Running with sudo would likely be annoying, too, because then you’d need to chown the just-added files owned by root.

I’m not sure what the fix is here.

agdespopoulos commented 4 years ago

Yep, seems that you're right. PgCommon is some helper library for Debian's postgresql-client package, I'm guessing it gets invoked when Sqitch goes out and uses psql. And, as you say, my UID and GID don't exist inside the container.

So why not create the proper user and group? I'm going to preface this with the fact that this probably has other problems, and its also super hacky, half because I was only editing docker-sqitch.sh instead of also messing with the dockerfile and a new entrypoint script, etc. That being said, changing the end of docker-sqitch.sh to the following resolves the problem for me.

# Run the container with the current and home directories mounted.
docker run -it --rm --network host \
    --mount "type=bind,src=$(pwd),dst=/repo" \
    --mount "type=bind,src=$HOME,dst=$homedst" \
    "${passopt[@]}" \
    `#override the -u flag in passopts and dockerfile, we need root` \
    -u 0:0 \
    `#override the entrypoint to get a shell` \
    --entrypoint "/bin/bash" \
    "$SQITCH_IMAGE" \
    -c \
        `# create me and my group, with proper uid and gid, in the container` \
        "groupadd --gid $(id -g ${user}) ${user} \
        && useradd --uid $(id -g ${user}) --gid $(id -g ${user}) -M ${user} && \
        `# now run sqitch as me` \
        su -c '/bin/sqitch $@' ${user}"

I could clean it up and do it "properly" and submit a PR, if you like.

theory commented 4 years ago

That still runs it as root, AFAICT. There are ways to make the entrypoint script create a user, but it seems super wasteful for every invocation.

agdespopoulos commented 4 years ago

Yes, the container does still run as root, although Sqitch doesn't. And yeah, it does seem wasteful... but I'm not sure what other solution there is.

theory commented 4 years ago

I'm gonna move this issue over to the docker-sqitch project.

theory commented 4 years ago

I made a new docker image today based on Debian buster, and it no longer seems to exhibit this warning. Here's what I get with sqitch/sqitch:0.9999:

SQITCH_IMAGE=sqitch/sqitch:0.9999 ../sqitch deploy db:pg://sqitch@db.example.com/dwheeler
Deploying changes to db:pg://sqitch@db.example.com/dwheeler
  + foo .. Use of uninitialized value $gid in getgrgid at /usr/share/perl5/PgCommon.pm line 783.
ok

But with the 1.0.0 image made today:

../sqitch deploy db:pg://sqitch@db.example.com/dwheeler
Deploying changes to db:pg://sqitch@db.example.com/dwheeler
  + foo .. ok

No more warning. I guess @df7cb made a change to PgCommon at some point to eliminate this warning?

df7cb commented 4 years ago

I can't remember any change in that area, but if it works now... let me know if there's anything I need to fix.

agdespopoulos commented 4 years ago

Yep, whatever the change was, this is fixed for me now. The issue is gone with the latest Docker Hub images. I'm glad my whole thing of creating the user isn't necessary. :)

Thanks for the help!

theory commented 4 years ago

@df7cb Yeah, I’m not sure what changed, but the issue definitely went away. I believe you can still see it if you build an image on Debian 9 (I use Debian-slim) and connect to the image using the docker host UID and GID, which don’t exist inside the container, and then run whatever Postgres thing triggers PgCommon.

I’m not even clear on the purpose of PgCommon or when it runs. Does it get triggered by psql? Perl Postgres apps? Something else? I assume that Postgres on Debian would work fine without it; true?

df7cb commented 4 years ago

psql is invoked through /usr/share/postgresql-common/pg_wrapper which uses PgCommon.pm.

theory commented 4 years ago

Ah, okay. What’s psql missing that requires it?

df7cb commented 4 years ago

Cluster integration with postgresql-common, psql --cluster 11/main. See pg_wrapper(1).

https://manpages.debian.org/buster/postgresql-client-common/pg_wrapper.1.en.html

theory commented 4 years ago

Doesn't seem necessary just to use psql to connect to a database running elsewhere, no? If not, what's the simplest way to remove it? Just link to the proper version of psql from /usr/local?

df7cb commented 4 years ago

I'm not sure which problem you are trying to fix here?

theory commented 4 years ago

I would like to remove PgCommon as a vector for issues. It occasionally emits warnings that confuse users (and me) because it’s not really visible. Sqitch just needs plain old psql and nothing special.

df7cb commented 4 years ago

If it's causing issues, please file a bug. If you try to hack around the way PostgreSQL works on Debian, it's likely to cause more issues on another level.

theory commented 4 years ago

Don't need PostgreSQL, just psql. Is there something lacking about the regular psql client?