From 75adeeff23d35e9f0fb7a6983f50e176f9194465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stevo=20Slavi=C4=87?= Date: Tue, 7 Apr 2020 23:59:41 +0200 Subject: [PATCH] Align graceful termination configuration with changes made in upstream ingress-nginx 0.26.0 (#46) --- CHANGELOG.md | 5 +++++ .../Configuration.md | 7 ++++++- .../templates/controller-deployment.yaml | 9 +++------ helm/nginx-ingress-controller-app/values.yaml | 16 ++++++++++++++++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 740c7856..45491f3b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/helm/nginx-ingress-controller-app/Configuration.md b/helm/nginx-ingress-controller-app/Configuration.md index 5e609fb8..691ae926 100644 --- a/helm/nginx-ingress-controller-app/Configuration.md +++ b/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` diff --git a/helm/nginx-ingress-controller-app/templates/controller-deployment.yaml b/helm/nginx-ingress-controller-app/templates/controller-deployment.yaml index 4a3f220b..584f55bb 100644 --- a/helm/nginx-ingress-controller-app/templates/controller-deployment.yaml +++ b/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 diff --git a/helm/nginx-ingress-controller-app/values.yaml b/helm/nginx-ingress-controller-app/values.yaml index 33873df2..767e7073 100644 --- a/helm/nginx-ingress-controller-app/values.yaml +++ b/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