GeoIP2 auto_reload doesn't work

[isniukArte]

What happened:
After update to the newest version, I added geoip2-autoreload-in-minutes option in ConfigMap.
apiVersion: v1 data: allow-snippet-annotations: "true" geoip2-autoreload-in-minutes: "2" use-geoip2: "true" ... kind: ConfigMap metadata: name: ingress-nginx-controller namespace: ingress-nginx

But in nginx.conf I still can't find a line with auto_reload param:
` # https://github.com/leev/ngx_http_geoip2_module#example-usage

geoip2 /etc/ingress-controller/geoip/GeoLite2-City.mmdb {
	$geoip2_city_country_code source=$remote_addr country iso_code;
	$geoip2_city_country_name source=$remote_addr country names en;
	$geoip2_city_country_geoname_id source=$remote_addr country geoname_id;
	$geoip2_city source=$remote_addr city names en;
	$geoip2_city_geoname_id source=$remote_addr city geoname_id;
	$geoip2_postal_code source=$remote_addr postal code;
	$geoip2_dma_code source=$remote_addr location metro_code;
	$geoip2_latitude source=$remote_addr location latitude;
	$geoip2_longitude source=$remote_addr location longitude;
	$geoip2_time_zone source=$remote_addr location time_zone;
	$geoip2_region_code source=$remote_addr subdivisions 0 iso_code;
	$geoip2_region_name source=$remote_addr subdivisions 0 names en;
	$geoip2_region_geoname_id source=$remote_addr subdivisions 0 geoname_id;
	$geoip2_subregion_code source=$remote_addr subdivisions 1 iso_code;
	$geoip2_subregion_name source=$remote_addr subdivisions 1 names en;
	$geoip2_subregion_geoname_id source=$remote_addr subdivisions 1 geoname_id;
	$geoip2_city_continent_code source=$remote_addr continent code;
	$geoip2_city_continent_name source=$remote_addr continent names en;
}

geoip2 /etc/ingress-controller/geoip/GeoLite2-ASN.mmdb {
	$geoip2_asn source=$remote_addr autonomous_system_number;
	$geoip2_org source=$remote_addr autonomous_system_organization;
}

geoip2 /etc/ingress-controller/geoip/GeoLite2-Country.mmdb {
	$geoip2_country_code source=$remote_addr country iso_code;
	$geoip2_country_name source=$remote_addr country names en`

What you expected to happen:

I can't detect the problem. I expect it to work according to #11079 which was a part of the latest release. I saw no related errors in logs on startup.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

NGINX Ingress controller
Release: v1.10.1
Build: 4fb5aac
Repository: https://github.com/kubernetes/ingress-nginx
nginx version: nginx/1.25.3

---

Kubernetes version (use kubectl version):
Client Version: v1.27.4
Kustomize Version: v5.0.1
Server Version: v1.28.5

Environment:

• Cloud provider or hardware configuration: Azure
• OS (e.g. from /etc/os-release):
• Kernel (e.g. uname -a):
• Install tools: AKS
• How was the ingress-nginx-controller installed:
  Helm wasn't used. Nginx was installed with our own solution. deploy.yaml was applied.
  Arguments:
  --maxmind-mirror=https://working-mirror.local,
  --maxmind-edition-ids=GeoLite2-City,GeoLite2-ASN,GeoLite2-Country

How to reproduce this issue:
I tried it on minikube. Installed nginx by applying command from the description. Next added a secret with my MaxMind license key and edited deployment and cm.

`      - args:
        - /nginx-ingress-controller
        - --election-id=ingress-nginx-leader
        - --controller-class=k8s.io/ingress-nginx
        - --ingress-class=nginx
        - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
        - --validating-webhook=:8443
        - --validating-webhook-certificate=/usr/local/certificates/cert
        - --validating-webhook-key=/usr/local/certificates/key
        - --enable-metrics=false
        - --maxmind-edition-ids=GeoLite2-City,GeoLite2-ASN,GeoLite2-Country
        - --maxmind-license-key=$(MAXMIND_LICENSE_KEY)
        ...
        ...
        env:
        - name: MAXMIND_LICENSE_KEY
          valueFrom:
            secretKeyRef:
              key: MAXMIND_LICENSE_KEY
              name: maxmind-license-key
`

ConfigMap:

data:
  allow-snippet-annotations: "false"
  geoip2-autoreload-in-minutes: "2"
  use-geoip2: "true"

Behavior is the same.

[longwuyuan]

• 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;)

[longwuyuan]

/assign

[longwuyuan]

@isniukArte did you use geoip1 earlier ?

[longwuyuan]

@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

ingress-nginx/rootfs/etc/nginx/template/nginx.tmpl

Line 204 in 4e11074

auto_reload {{ $cfg.GeoIP2AutoReloadMinutes }}m;

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

[longwuyuan]

/remove-priority important-longterm

[longwuyuan]

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

[isniukArte]

@isniukArte did you use geoip1 earlier ?

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

[longwuyuan]

ok

[Gacko]

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

[k8s-ci-robot]

@Gacko: Closing this issue.

In response to this:

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

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[isniukArte]

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.

[Gacko]

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