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

[charts/redis-ha] fix: redis and haproxy and busybox image was migrated from docker hub to public ecr for docker hub rate limit #214

Merged
merged 5 commits into from
Apr 8, 2024

Conversation

kahirokunn
Copy link
Contributor

@kahirokunn kahirokunn commented Jul 29, 2022

What this PR does / why we need it:

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes #

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [stable/mychartname])

@kahirokunn kahirokunn changed the title fix: redis and haproxy and busybox image was migrated from docker hub to public ecr for docker hub rate limit [charts/redis-ha] fix: redis and haproxy and busybox image was migrated from docker hub to public ecr for docker hub rate limit Jul 29, 2022
…cr for docker hub rate limit

Signed-off-by: kahirokunn <[email protected]>
@DandyDeveloper
Copy link
Owner

@kahirokunn I'm not seeing any mentions of this being an official migration?

https://hub.docker.com/_/redis

Can you throw me something confirming this as an official migration?

@kahirokunn
Copy link
Contributor Author

@mhkarimi1383
Copy link
Contributor

mhkarimi1383 commented Aug 22, 2022

What about Google's cache proxy for DockerHub?
But this is a bit specific to users for example I'm using my own docker registry docker.karimi.dev :)
or some users bought docker hub account

@DandyDeveloper
Copy link
Owner

@kahirokunn I'm gonna see if I can dig and find an official word that its right to use this. Sorry for the delay.

@34fathombelow
Copy link
Contributor

@DandyDeveloper I did find an official announcement from docker.

I think it's best to leave it to the default of docker.io/library to avoid confusion. Personally it makes no difference to me, but I do not believe public.ecr.aws/docker/library/ is widely known or adopted in the community yet.

@kahirokunn
Copy link
Contributor Author

If you are considering a rate limit, I would suggest using a registry that is mirrored by docker.io. AWS, GCP, GitHub, whatever, but a trusted and proven organization is preferred.

In fact I got stuck with docker.io's rate limit and failed in production.

@DandyDeveloper
Copy link
Owner

I think we're in a significantly better place now than we were 2 years ago to merge this in :) I've updated to the official ECR repos now.

DandyDeveloper
DandyDeveloper previously approved these changes Apr 7, 2024
@mhkarimi1383
Copy link
Contributor

@kahirokunn
Are you sure that bitnami images are Ok?

@DandyDeveloper
Copy link
Owner

@kahirokunn CI says no, and that was a mistake on my part! Sorry!

@mhkarimi1383
Copy link
Contributor

@kahirokunn CI says no, and that was a mistake on my part! Sorry!

That's because paths and user accesses are not standard.
Why using Bitnami?

@DandyDeveloper
Copy link
Owner

@mhkarimi1383 Because I messed up the commit. :)

@mhkarimi1383
Copy link
Contributor

@mhkarimi1383 Because I messed up the commit. :)

I mean I would prefer using official images since bitnami images are non root and non standard

@DandyDeveloper
Copy link
Owner

@mhkarimi1383 Yes. I am using the official ones. We don't want Bitnami. I committed the wrong images (See latest commit)

@DandyDeveloper DandyDeveloper merged commit 0fe831e into DandyDeveloper:master Apr 8, 2024
2 checks passed
@kahirokunn kahirokunn deleted the change-repo branch April 8, 2024 11:59
@kahirokunn
Copy link
Contributor Author

Thx!

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.

4 participants