-
Notifications
You must be signed in to change notification settings - Fork 286
fix(osmap.yaml): fix PGDG repo file creation for Amazon Linux 2 #314
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
fix(osmap.yaml): fix PGDG repo file creation for Amazon Linux 2 #314
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, @Niyakiy. Please see the requested change below.
postgres/osfamilymap.yaml
Outdated
{%- if grains.get('osfullname', "") == "Amazon Linux" and grains.get('osmajorrelease', 0) == 2 %} | ||
baseurl: 'https://download.postgresql.org/pub/repos/yum/{{ repo.version }}/redhat/rhel-7-$basearch' | ||
{%- else %} | ||
baseurl: 'https://download.postgresql.org/pub/repos/yum/{{ repo.version }}/redhat/rhel-$releasever-$basearch' | ||
{%- endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Niyakiy This if
block can be avoided by adding this change to osmap.yaml
instead, in a similar manner to this block:
postgres-formula/postgres/osmap.yaml
Lines 6 to 9 in 43c5696
Fedora: | |
pkg_repo: | |
# yamllint disable-line rule:line-length | |
baseurl: 'https://download.postgresql.org/pub/repos/yum/{{ repo.version }}/fedora/fedora-$releasever-$basearch' |
I.e. Probably:
Amazon:
pkg_repo:
baseurl: 'https://download.postgresql.org/pub/repos/yum/{{ repo.version }}/redhat/rhel-7-$basearch'
This isn't perfect, since it doesn't specify the version of Amazon Linux but it is probably sufficient, since we're no longer supporting Amazon Linux 1 (2018). We could use the oscodename
grain (Amazon Linux 2
) in codenamemap.yaml
but that's overkill in my opinion. Ultimately, we have a new map.jinja
that is being introduced across formulas and we can provide the best solution when that becomes available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@myii Thanks for the heads up on a better and easier way to introduce a fix. Having AL1 already deprecated it looks reasonable to leave Amazon
inside osmap.yaml
as we have only the 2nd version available. However, it will require additional code changes here once AWS releases a new major version of AL.
I'm about to push the requested code changes along with reworded commit message.
Just another note, doesn't necessarily have to be addressed by this PR. We're not currently testing postgres-formula/test/salt/pillar/postgres.sls Lines 10 to 18 in d16e91c
I'll try to run some tests when I get a chance. Let's not make this a blocker for this PR, if I don't report back soon. @Niyakiy Please don't go out of your way to run these tests, what you've already provided is more than enough, thanks. |
43c5696
to
2de775b
Compare
Thanks, @Niyakiy -- merged. Out of interest, did you need to adjust the pkgs_deps:
- libicu
- systemd-sysv I had to do that to get PostgreSQL 13 installed on Amazon Linux 2: |
🎉 This PR is included in version 0.41.9 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fix improper pgdg repo file creation for Amazon Linux 2.
Description:
Current formula implementation uses
baseurl: 'https://download.postgresql.org/pub/repos/yum/{{ repo.version }}/redhat/rhel-$releasever-$basearch'
setting $releasever to 2 for Amazon Linux 2 and making formula install fail due to missing rhel-2 packages.
The fix makes a newly created formula repo file to use rhel-7 as the proper repository to use during install.