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

Remove sudo requirement (PROJQUAY-4630) #103

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

HammerMeetNail
Copy link
Contributor

@HammerMeetNail HammerMeetNail commented Dec 15, 2022

  • Removes requirements for root
  • Updates default quay-install location to be in a user writeable directory, $HOME/quay-install
    • Users can use the --quayRoot, --quayStorage and pgStorage flags to change install location, but previous root requirement is needed.
      *Example: $ ./mirror-registry install -v --quayRoot /home/doconnor/quay-install --quayStorage /home/doconnor/quay-install/quay-storage --pgStorage /home/doconnor/quay-install/pg-data
  • Stores postgres data in new podman named volume quay-postgres-data
  • Removes actions that require /etc/hosts to be updated
    • Only root users can update /etc/hosts, this is incompatible with a rootless install
  • Also works on RHEL9 (PROJQUAY-3084)
  • Updates mirror-registry to v1.3.0 and includes Quay 3.8.0 for IPv6 support
[doconnor@doconnor-omr-dev-rhel9 ~]$ cat /etc/redhat-release
Red Hat Enterprise Linux release 9.1 (Plow)
[doconnor@doconnor-omr-dev-rhel9 ~]$ podman --version
podman version 4.2.0
[doconnor@doconnor-omr-dev-rhel9 ~]$ systemctl --user status quay-app
● quay-app.service - Quay Container
     Loaded: loaded (/home/doconnor/.config/systemd/user/quay-app.service; enabled; vendor preset: disabled)
     Active: active (running) since Mon 2022-12-19 13:55:52 UTC; 4min 59s ago
    Process: 67021 ExecStartPre=/bin/rm -f /run/user/1007/quay-app.service-pid /run/user/1007/quay-app.service-cid (code=exited, status=0/SUCCESS)

@HammerMeetNail HammerMeetNail added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Dec 15, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 15, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 15, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 15, 2022
@HammerMeetNail HammerMeetNail removed the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Dec 15, 2022
@HammerMeetNail HammerMeetNail added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Dec 16, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 16, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 16, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 16, 2022
@HammerMeetNail HammerMeetNail added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Dec 16, 2022
@HammerMeetNail
Copy link
Contributor Author

HammerMeetNail commented Dec 16, 2022

CI will fail because we don't allow changes to CI from open PRs. Probably going to require a follow up PR or two to sort out CI.

@@ -10,7 +10,7 @@ TimeoutStartSec=5m
ExecStartPre=-/bin/rm -f %t/%n-pid %t/%n-cid
ExecStart=/usr/bin/podman run \
--name quay-postgres \
-v {{ quay_root }}/pg-data:/var/lib/pgsql/data:Z \
-v quay-postgres-data:/var/lib/pgsql/data:Z \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we still keep the named volume somewhere inside quay_root? I think we should keep the ability that a user can specify their pg mount path especially since it can get quite large over time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was running into permissions errors as a non-root user when using bind mounts with the postgres container. The postgres user is different than the non-root user and I was having trouble writing to quay_root. Tried a couple different things, but couldn't get it working. Not sure if it's an selinux thing, but dropping in a named volume worked so I went with it. Maybe you know how I can work around user permissions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SELinux forces you to set special permissions on a directory before being able to mount it. That is the reason for these steps in each of the service setup tasks. https://github.com/quay/mirror-registry/blob/main/ansible-runner/context/app/project/roles/mirror_appliance/tasks/install-postgres-service.yaml#L7 Maybe we have to update the user there?

@HammerMeetNail HammerMeetNail force-pushed the PROJQUAY-4630 branch 3 times, most recently from a8c0a8b to 3a09278 Compare December 22, 2022 16:42
Signed-off-by: Dave O'Connor <doconnor@redhat.com>
Copy link
Collaborator

@jonathankingfc jonathankingfc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HammerMeetNail HammerMeetNail merged commit 0636212 into quay:main Jan 3, 2023
@HammerMeetNail HammerMeetNail deleted the PROJQUAY-4630 branch January 3, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants