Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/nginx-ingress] Fix for "spec.clusterIP: Invalid value" error on upgrade #13646

Merged
merged 3 commits into from
May 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stable/nginx-ingress/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v1
name: nginx-ingress
version: 1.6.7
version: 1.6.8
appVersion: 0.24.1
home: https://github.com/kubernetes/ingress-nginx
description: An nginx Ingress controller that uses ConfigMap to store the nginx configuration.
Expand Down
14 changes: 14 additions & 0 deletions stable/nginx-ingress/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ Parameter | Description | Default
`controller.publishService.enabled` | if true, the controller will set the endpoint records on the ingress objects to reflect those on the service | `false`
`controller.publishService.pathOverride` | override of the default publish-service name | `""`
`controller.service.clusterIP` | internal controller cluster service IP | `""`
`controller.service.omitClusterIP` | To omit the `clusterIP` from the controller service | `false`
`controller.service.externalIPs` | controller service external IP addresses. Do not set this when `controller.hostNetwork` is set to `true` and `kube-proxy` is used as there will be a port-conflict for port `80` | `[]`
`controller.service.externalTrafficPolicy` | If `controller.service.type` is `NodePort` or `LoadBalancer`, set this to `Local` to enable [source IP preservation](https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-typenodeport) | `"Cluster"`
`controller.service.healthCheckNodePort` | If `controller.service.type` is `NodePort` or `LoadBalancer` and `controller.service.externalTrafficPolicy` is set to `Local`, set this to [the managed health-check port the kube-proxy will expose](https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-typenodeport). If blank, a random port in the `NodePort` range will be assigned | `""`
Expand Down Expand Up @@ -114,13 +115,15 @@ Parameter | Description | Default
`controller.stats.enabled` | if `true`, enable "vts-status" page | `false`
`controller.stats.service.annotations` | annotations for controller stats service | `{}`
`controller.stats.service.clusterIP` | internal controller stats cluster service IP | `""`
`controller.stats.service.omitClusterIP` | To omit the `clusterIP` from the stats service | `false`
`controller.stats.service.externalIPs` | controller service stats external IP addresses | `[]`
`controller.stats.service.loadBalancerIP` | IP address to assign to load balancer (if supported) | `""`
`controller.stats.service.loadBalancerSourceRanges` | list of IP CIDRs allowed access to load balancer (if supported) | `[]`
`controller.stats.service.type` | type of controller stats service to create | `ClusterIP`
`controller.metrics.enabled` | if `true`, enable Prometheus metrics (`controller.stats.enabled` must be `true` as well) | `false`
`controller.metrics.service.annotations` | annotations for Prometheus metrics service | `{}`
`controller.metrics.service.clusterIP` | cluster IP address to assign to service | `""`
`controller.metrics.service.omitClusterIP` | To omit the `clusterIP` from the metrics service | `false`
`controller.metrics.service.externalIPs` | Prometheus metrics service external IP addresses | `[]`
`controller.metrics.service.labels` | labels for metrics service | `{}`
`controller.metrics.service.loadBalancerIP` | IP address to assign to load balancer (if supported) | `""`
Expand Down Expand Up @@ -153,6 +156,7 @@ Parameter | Description | Default
`defaultBackend.priorityClassName` | default backend priorityClassName | `nil`
`defaultBackend.service.annotations` | annotations for default backend service | `{}`
`defaultBackend.service.clusterIP` | internal default backend cluster service IP | `""`
`defaultBackend.service.omitClusterIP` | To omit the `clusterIP` from the default backend service | `false`
`defaultBackend.service.externalIPs` | default backend service external IP addresses | `[]`
`defaultBackend.service.loadBalancerIP` | IP address to assign to load balancer (if supported) | `""`
`defaultBackend.service.loadBalancerSourceRanges` | list of IP CIDRs allowed access to load balancer (if supported) | `[]`
Expand Down Expand Up @@ -241,3 +245,13 @@ controller:
annotations:
domainName: "kubernetes-example.com"
```

## Helm error when upgrading: spec.clusterIP: Invalid value: ""

If you are upgrading this chart from a version between 0.31.0 and 1.2.2 then you may get an error like this:

```
Error: UPGRADE FAILED: Service "?????-controller" is invalid: spec.clusterIP: Invalid value: "": field is immutable
```

Detail of how and why are in [this issue](https://github.com/helm/charts/pull/13646) but to resolve this you can set `xxxx.service.omitClusterIP` to `true` where `xxxx` is the service referenced in the error.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ metadata:
release: {{ .Release.Name }}
name: {{ template "nginx-ingress.controller.fullname" . }}-metrics
spec:
{{- if not .Values.controller.metrics.service.omitClusterIP }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a new value that can be set lets have these check if the current value of clusterIP contains anything.

{{- if .Values.controller.metrics.service.clusterIP }}

https://github.com/helm/helm/blob/master/docs/chart_template_guide/control_structures.md#ifelse

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ChiefAlexander
Of course that's the obvious solution but it's not usable here. This is all covered in #9227

This was tried in #7933 and reverted in #10993

All users who installed the chart between version 0.31.0 and 1.2.2 cannot upgrade.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole thing is a disaster. Add a section in the readme to document this and how users need to use it to upgrade from those versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK done

clusterIP: "{{ .Values.controller.metrics.service.clusterIP }}"
{{- end }}
{{- if .Values.controller.metrics.service.externalIPs }}
externalIPs:
{{ toYaml .Values.controller.metrics.service.externalIPs | indent 4 }}
Expand Down
2 changes: 2 additions & 0 deletions stable/nginx-ingress/templates/controller-service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ metadata:
release: {{ .Release.Name }}
name: {{ template "nginx-ingress.controller.fullname" . }}
spec:
{{- if not .Values.controller.service.omitClusterIP }}
clusterIP: "{{ .Values.controller.service.clusterIP }}"
{{- end }}
{{- if .Values.controller.service.externalIPs }}
externalIPs:
{{ toYaml .Values.controller.service.externalIPs | indent 4 }}
Expand Down
2 changes: 2 additions & 0 deletions stable/nginx-ingress/templates/controller-stats-service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ metadata:
release: {{ .Release.Name }}
name: {{ template "nginx-ingress.controller.fullname" . }}-stats
spec:
{{- if not .Values.controller.stats.service.omitClusterIP }}
clusterIP: "{{ .Values.controller.stats.service.clusterIP }}"
{{- end }}
{{- if .Values.controller.stats.service.externalIPs }}
externalIPs:
{{ toYaml .Values.controller.stats.service.externalIPs | indent 4 }}
Expand Down
2 changes: 2 additions & 0 deletions stable/nginx-ingress/templates/default-backend-service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ metadata:
release: {{ .Release.Name }}
name: {{ template "nginx-ingress.defaultBackend.fullname" . }}
spec:
{{- if not .Values.defaultBackend.service.omitClusterIP }}
clusterIP: "{{ .Values.defaultBackend.service.clusterIP }}"
{{- end }}
{{- if .Values.defaultBackend.service.externalIPs }}
externalIPs:
{{ toYaml .Values.defaultBackend.service.externalIPs | indent 4 }}
Expand Down
4 changes: 4 additions & 0 deletions stable/nginx-ingress/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ controller:
service:
annotations: {}
labels: {}
omitClusterIP: false
clusterIP: ""

## List of IP addresses at which the controller services are available
Expand Down Expand Up @@ -247,6 +248,7 @@ controller:

service:
annotations: {}
omitClusterIP: false
clusterIP: ""

## List of IP addresses at which the stats service is available
Expand All @@ -269,6 +271,7 @@ controller:
# prometheus.io/scrape: "true"
# prometheus.io/port: "10254"

omitClusterIP: false
clusterIP: ""

## List of IP addresses at which the stats-exporter service is available
Expand Down Expand Up @@ -351,6 +354,7 @@ defaultBackend:

service:
annotations: {}
omitClusterIP: false
clusterIP: ""

## List of IP addresses at which the default backend service is available
Expand Down