Align graceful termination configuration with changes made in upstrea…

…m ingress-nginx 0.26.0 (#46)

CHANGELOG.md

@@ -9,6 +9,11 @@ and this project's packages adheres to [Semantic Versioning](http://semver.org/s
 
 ### Changed
 
+- Align graceful termination configuration with changes made in upstream ingress-nginx 0.26.0 (see [related PR #4487](https://github.com/kubernetes/ingress-nginx/pull/4487#issuecomment-525588554) and important section in [0.26.0 release notes](https://github.com/kubernetes/ingress-nginx/releases/tag/nginx-0.26.0)).
+  - Make NGINX IC Deployment's `terminationGracePeriodSeconds` configurable and align its default with `configmap.worker-shutdown-timeout`
+  - Make NGINX IC controller container lifecycle hooks configurable, and change from `sleep 60` to using `/wait-shutdown` as preStop hook.
+- Make `controller.minReadySeconds` configurable.
+
 ## [v1.6.6] 2020-04-01
 
 ### Changed

helm/nginx-ingress-controller-app/Configuration.md

@@ -25,6 +25,7 @@ Parameter | Description | Default
 `configmap.server-name-hash-bucket-size` | Sets the size of the bucket for the server names hash tables. | "1024"
 `configmap.server-tokens` | Controlls whether to send NGINX Server header in responses and display NGINX version in error pages. | "false"
 `configmap.worker-processes` | Sets the number of worker processes. | "1"
+`configmap.worker-shutdown-timeout` | Maximum amount of time NGINX worker processes should give active connections to drain. This should not be higher than `controller.terminationGracePeriodSeconds` | "240s"
 `configmap.use-forwarded-headers` | If true, NGINX passes the incoming `X-Forwarded-*` headers to upstreams. | "true"
 `controller.annotationsPrefix` | Prefix of the Ingress annotations specific to the NGINX controller. This is a replacement for deprecated `configmap.annotations-prefix` configuration property; if both are configured, `configmap.annotations-prefix` has precedence. | `nginx.ingress.kubernetes.io`
 `controller.autoscaling.enabled` | Enables or disables Horizontal Pod Autoscaler (HPA) for NGINX Ingress Controller Deployment. This is a replacement for deprecated `configmap.hpa-enabled` configuration property; if both are configured, `configmap.hpa-enabled` has precedence. | `true`
@@ -35,10 +36,14 @@ Parameter | Description | Default
 `controller.defaultSSLCertificate` | The Secret referred to by this flag contains the default certificate to be used when accessing the catch-all server. If this flag is not provided NGINX will use a self-signed certificate. Example value: "default/foo-tls". This is a replacement for deprecated `configmap.default-ssl-certificate` configuration property; if both are configured, `configmap.default-ssl-certificate` has precedence. | ""
 `controller.ingressController.legacy` | Legacy or node pools cluster. On aws provider node pool clusters LoadBalancer service gets created. Dynamically calculated during cluster creation. | `false`
 `controller.ingressClass` | Ingress class, which controller handles. This is a replacement for deprecated `configmap.ingress-class` configuration property; if both are configured, `configmap.ingress-class` has precedence. | `nginx`
+`controller.lifecycle` | Configures NGINX controller container lifecycle hooks. | By default configured to run `/wait-shutdown` as controller container preStop hook.
+`controller.maxUnavailable` | Configures maximum unavailable replicas count for NGINX controller Deployment rolling upgrade strategy. | `1`
 `controller.metrics.enabled` | If true, create metrics Service for prometheus-operator support. | `false`
 `controller.metrics.port` | Configures container metrics port to be exposed. | `10254`
 `controller.metrics.service.servicePort` | Configures metrics Service port. | `9913`
-`controller.replicaCount` | Number of initial NGINX Ingress Controller Deployment replicas. | `1`
+`controller.minReadySeconds` | Configures minimum amount of time that NGINX IC Deployment replica has to be ready before rolling upgrade can proceed with the next replica. | `0`
+`controller.replicaCount` | Number of initial NGINX IC Deployment replicas. | `1`
 `controller.service.enabled` | If true, create NodePort Service. Dynamically calculated during cluster creation. | `false`
 `controller.service.type` | Applies only to `provider=aws` (`external`/`internal`) | `external`
+`controller.terminationGracePeriodSeconds` | Maximum amount of time NGINX Deployment replica is given to gracefully terminate. This should not be lower than `configmap.worker-shutdown-timeout`. | 300
 `provider` | Provider identifier (`aws`/`azure`/`kvm`) | `kvm`

helm/nginx-ingress-controller-app/templates/controller-deployment.yaml

@@ -20,6 +20,7 @@ spec:
     rollingUpdate:
       maxSurge: 1
       maxUnavailable: {{ .Values.controller.maxUnavailable }}
+  minReadySeconds: {{ .Values.controller.minReadySeconds }}
   template:
     metadata:
       labels:
@@ -45,6 +46,7 @@ spec:
               topologyKey: kubernetes.io/hostname
       serviceAccountName: {{ .Values.controller.name }}
       priorityClassName: system-cluster-critical
+      terminationGracePeriodSeconds: {{ .Values.controller.terminationGracePeriodSeconds }}
       containers:
       - name: {{ .Values.controller.name }}
         image: "{{ .Values.image.registry }}/{{ .Values.controller.image.repository }}:{{ .Values.controller.image.tag }}"
@@ -112,12 +114,7 @@ spec:
           initialDelaySeconds: 10
           timeoutSeconds: 1
         lifecycle:
-          # Enable graceful shutdowns and rolling updates with zero-downtime
-          preStop:
-            exec:
-              command:
-              - sleep
-              - "60"
+{{ toYaml .Values.controller.lifecycle | indent 10 }}
         ports:
         - name: http
           containerPort: 80

helm/nginx-ingress-controller-app/values.yaml

@@ -31,6 +31,8 @@ configmap:
   server-tokens: "false"
   worker-processes: "1"
   use-forwarded-headers: "true"
+  # value of worker-shutdown-timeout should not be higher than configured `controller.terminationGracePeriodSeconds`
+  worker-shutdown-timeout: "240s"
 
 controller:
   name: nginx-ingress-controller
@@ -39,6 +41,8 @@ controller:
   replicaCount: 1
 
   maxUnavailable: 1
+  # minReadySeconds to avoid killing pods before we are ready
+  minReadySeconds: 0
 
   configmap:
     name: ingress-nginx
@@ -89,6 +93,18 @@ controller:
           cpu: 500m
           memory: 600Mi
 
+  # allow the draining of connections up to five minutes
+  # this should not be lower than configmap.worker-shutdown-timeout
+  # for more info see https://github.com/kubernetes/ingress-nginx/pull/4487#issuecomment-525588554
+  # and important note in https://github.com/kubernetes/ingress-nginx/releases/tag/nginx-0.26.0
+  terminationGracePeriodSeconds: 300
+  lifecycle:
+    # Enable graceful shutdowns and rolling updates with zero-downtime
+    preStop:
+      exec:
+        command:
+        - /wait-shutdown
+
   # optional hpa settings
   autoscaling:
     enabled: true