GeoIP2 auto_reload doesn't work about ingress-nginx HOT 12 CLOSED

• I can reproduce this with value set to 2, "2" or 2m
• Wait for comments from @rikatz @tao12345666333
  cc @cpanato

/triage accepted
/priority important-longterm

I suspect that geoip1 was accepting int value but i think upstream changed and that geoip2 module expects "time" value like "2m", as per this https://github.com/leev/ngx_http_geoip2_module#example-usage (like auto_reload 2m;)

from ingress-nginx.

/assign

from ingress-nginx.

@isniukArte did you use geoip1 earlier ?

from ingress-nginx.

@rikatz @tao12345666333 @cpanato @strongjz The nginx.tmpl in the image is not even containing the if condition for the reload value

But the src of the base image has the if condition

So this is confirmed a bug. The annotation https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#geoip2-autoreload-in-minutes is definitely broken

/priority important-soon

from ingress-nginx.

/remove-priority important-longterm

from ingress-nginx.

ok, the root-cause is clear now. The PR #11079 that introduces that if condition for the reload time is not cherry-picked into the branch release-1.10, from which this controller version was released

@Gacko @strongjz @rikatz @tao12345666333 @cpanato Please comment on chery-picking PR #11079 (or using another way) to pull these changes to the branch release-1.10

from ingress-nginx.

@isniukArte did you use geoip1 earlier ?

Nope, but I think it already doesn't matter.

from ingress-nginx.

ok

from ingress-nginx.

This is not a feature of the latest release. There has been some trouble with the tag, but the actual release and the according release branch release-1.10 do not include that feature for good, as this is a feature and not only a patch.

/close

from ingress-nginx.

@Gacko: Closing this issue.

from ingress-nginx.

This is not a feature of the latest release. There has been some trouble with the tag, but the actual release and the according release branch release-1.10 do not include that feature for good, as this is a feature and not only a patch.

/close

I saw that it wasn't mentioned in the release notes.
But, I picked it up from the official documentation: https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/ That's why I decided it should work.

from ingress-nginx.

The documentation is built on the main branch. That's an issue we are aware of and want to improve in the future.

from ingress-nginx.