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

Switch to using the sclorg redis image for consistency #1531

Merged
merged 1 commit into from Aug 25, 2023

Conversation

rooftopcellist
Copy link
Member

SUMMARY

The eda-server-operator uses the sclorg redis image as well, which is closer to what we use downstream. Consistency will improve maintainability.

New image:

  • quay.io/sclorg/redis-6-c8s:latest
ISSUE TYPE
  • Bug, Docs Fix or other nominal change

@@ -237,8 +237,8 @@ extra_volumes: ''

_image: quay.io/ansible/awx
_image_version: "{{ lookup('env', 'DEFAULT_AWX_VERSION') or 'latest' }}"
_redis_image: docker.io/redis
_redis_image_version: 7
_redis_image: quay.io/sclorg/redis-6-c8s
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_redis_image: quay.io/sclorg/redis-6-c8s
_redis_image: quay.io/sclorg/redis-6-c9s

Since the postgres 15 PR will use a c9s based container image then we should have the same thing everywhere.
What do you think ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, I just changed this to c9s. I'll make the same changes for eda-server-operator.

@Denney-tech
Copy link

Denney-tech commented Aug 24, 2023

This requires a change to the redis.conf file to specify dir /var/lib/redis/data due to how sclorg/redis is built vs the docker ones.

Completing pr #1427 would allow users to define redis.conf with or without the dir line as needed.

@rooftopcellist
Copy link
Member Author

Good point @Denney-tech . The path is changed in this PR, so it will work out of the box. But, I suppose there may be users who want to use redis:7 from docker, so #1427 would allow them to set a custom path for the redis data dir (in this case /var/lib/redis/data instead of /data).

That other PR is largely ready to go, it just needs the merge conflicts to be resolved by the contributor. If someone has time to take over that PR and finish it up, that would be welcome.

@rooftopcellist rooftopcellist merged commit 48dcb08 into ansible:devel Aug 25, 2023
6 checks passed
@Denney-tech
Copy link

Sorry, I must have been tired yesterday when I was looking at this. Had spent all day figuring out why the sclorg images didn't work, figured it out, then found these two PR's. For some reason I didn't see the dir line added among the files changed here. This PR satisfies my immediate need, though.

Thank you.

@Denney-tech
Copy link

Actually, no I wasn't crazy, the /etc/redis.conf has not been modified by this pull request and the devel image from quay now deploys the sclorg/redis image without the proper config line.

roles/installer/templates/configmaps/config.yaml.j2 needs dir /var/lib/redis/data appended still.

TheRealHaoLiu added a commit to TheRealHaoLiu/awx-operator that referenced this pull request Aug 28, 2023
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.

None yet

4 participants