we should use
out.Styled(style.Notice, Kicbase images have not been deleted. To delete images run:)

I supsect there are other places we could that

do you putting a Before/Aeter this PR,

you can simulate the rate limit by making a Dummy docker binary that always returns with rate limit...
howevever we also have pulling images using go library , you mock those libraries for full coverage of everywhere we pull image

https://github.com/google/go-containerregistry

also in our addons we need to check if addons which docker images if they are rate limitted or not.

so we should add code to where we enable the addons (and verify that the image is not rate limitted) and if it is we should tell the users nicely

OK, thanks for the clarification!
For this PR, I can switch the logging in cache_images.go from klog to out.Styled(style.Notice, ...) where appropriate.
Do you suggest I update all klog calls in that file, or just the ones related to rate limit messages?

Also, I’ll look into adding tests by mocking docker/go-containerregistry to simulate the rate limit scenario.

Got it!
So for addons, I’ll add a check when enabling them to verify the image is not rate-limited, and if it is, we’ll surface a friendly notice to the user.

we should use out.Styled(style.Notice, Kicbase images have not been deleted. To delete images run:)

I supsect there are other places we could that

do you putting a Before/Aeter this PR,

you can simulate the rate limit by making a Dummy docker binary that always returns with rate limit... howevever we also have pulling images using go library , you mock those libraries for full coverage of everywhere we pull image

https://github.com/google/go-containerregistry

Thanks for the suggestion! I'm not sure how to properly mock go-containerregistry's remote.Image in our current test setup. Could you give me a pointer or a small example of how you'd recommend doing that?

So far, I tried:

• Using a dummy Docker binary (which worked for simulating errors).
• Replacing the function with a variable to trigger the error path, but it didn't enter the if block as expected.

At this point, I'm not sure how to apply go-containerregistry to simulate the rate-limit case in tests. Any guidance or example would be really helpful.

If you'd prefer, I can open a follow-up issue for this discussion.

Using strings.Contains() means we are not using a public API. This is too fragile to depend on. Do we have an error code field or error name we can check instead?

Using strings.Contains() means we are not using a public API.
This is too fragile to depend on.
Do we have an error code field or error name we can check instead?

Thanks for the feedback! 🙏
I agree that relying on strings.Contains() is fragile.

In my opinion, we could consider using go-containerregistry, since its API exposes structured errors. For example:

if terr, ok := err.(*transport.Error); ok {
 if terr.StatusCode == http.StatusTooManyRequests {
 // This confirms it's a 429 rate limit error
 }
}

This way we can explicitly check the HTTP status code instead of depending on error strings.
Do you think this approach would be acceptable for minikube?

This looks good, this is way to handle errors.