Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ipFamilyPolicy on services #375

Merged
merged 2 commits into from
Mar 29, 2023
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
5 changes: 5 additions & 0 deletions charts/opensearch-dashboards/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
### Security
---
## [2.9.2]
### Added
- Support setting ipFamilyPolicy on Service
- Support setting ipFamilies on Service
---
## [2.9.1]
### Added
### Changed
Expand Down
2 changes: 1 addition & 1 deletion charts/opensearch-dashboards/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 2.9.1
version: 2.9.2

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
2 changes: 2 additions & 0 deletions charts/opensearch-dashboards/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
| `service.nodePort` | Custom [nodePort][] port that can be set if you are using `service.type: nodePort` | `""` |
| `service.transportPortName` | The name of the transport port within the service | `transport` |
| `service.type` | OpenSearch [Service Types][] | `ClusterIP` |
| `service.ipFamilyPolicy` | This sets the preferred ip addresses in case of a dual-stack server, there are three options [PreferDualStack, SingleStack, RequireDualStack], [more information on dual stack](https://kubernetes.io/docs/concepts/services-networking/dual-stack/) | `""` |
| `service.ipFamilies` | Sets the preferred IP variants and in which order they are preferred, the first family you list is used for the legacy .spec.ClusterIP field, [more information on dual stack](https://kubernetes.io/docs/concepts/services-networking/dual-stack/) | `""` |
| `tolerations` | Configurable [tolerations][] | `[]` |
| `updateStrategy` | The [updateStrategy][] for the StatefulSet. By default Kubernetes will wait for the cluster to be green after upgrading each pod. Setting this to `OnDelete` will allow you to manually delete each pod during upgrades | `RollingUpdate` |
| `extraObjects` | Array of extra K8s manifests to deploy | list `[]` |
Expand Down
8 changes: 8 additions & 0 deletions charts/opensearch-dashboards/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ metadata:
{{- end }}
spec:
type: {{ .Values.service.type }}
{{- if (semverCompare ">= 1.23-0" .Capabilities.KubeVersion.Version) }}
{{- if .Values.service.ipFamilyPolicy }}
ipFamilyPolicy: {{ .Values.service.ipFamilyPolicy }}
Copy link
Member

Choose a reason for hiding this comment

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

Hey @UXabre thanks for PR, this is good change to have, can you please let us know what would be the default value, is this a breaking change if a chart is updated with helm upgrade ?
Thank you

Copy link
Contributor Author

@UXabre UXabre Mar 2, 2023

Choose a reason for hiding this comment

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

Hi, I actually set the default value to SingleStack to be more compatible (as many people don't use dual stack yet). However, now I'm thinking about it, I think it's safer to just check if the variable is set and thus fallback to the default that the CNI provides (at least, to my understanding the CNI would define the implicit default).
Because if the singlestack default and the assumption not many people are running dual stack clusters, I don't expect it to be a breaking change but it would be safer to not alter it if not set explicitely

Also, another change, to be more versatile would be to also provide a config for the IPFamilies as well, this way, one could set it to singlestack and ipv6 (if they are brave :p).

Curious to hear what you think of this proposal.

{{- end }}
{{- if .Values.service.ipFamilies }}
ipFamilies: {{ .Values.service.ipFamilies }}
{{- end }}
{{- end }}
{{- if .Values.service.loadBalancerIP }}
loadBalancerIP: {{ .Values.service.loadBalancerIP }}
{{- end }}
Expand Down
7 changes: 7 additions & 0 deletions charts/opensearch-dashboards/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ serverHost: "0.0.0.0"

service:
type: ClusterIP
# The IP family and IP families options are to set the behaviour in a dual-stack environment
# Omitting these values will let the service fall back to whatever the CNI dictates the defaults
# should be
#
# ipFamilyPolicy: SingleStack
# ipFamilies:
# - IPv4
port: 5601
loadBalancerIP: ""
nodePort: ""
Expand Down
5 changes: 5 additions & 0 deletions charts/opensearch/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
### Security
---
## [2.11.3]
### Added
- Support setting ipFamilyPolicy on Service
- Support setting ipFamilies on Service
---
## [2.11.2]
### Added
- Service ports for performance analyzer
Expand Down
2 changes: 1 addition & 1 deletion charts/opensearch/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 2.11.2
version: 2.11.3

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
2 changes: 2 additions & 0 deletions charts/opensearch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ helm uninstall my-release
| `service.nodePort` | Custom [nodePort][] port that can be set if you are using `service.type: nodePort` | `""` |
| `service.transportPortName` | The name of the transport port within the service | `transport` |
| `service.type` | OpenSearch [Service Types][] | `ClusterIP` |
| `service.ipFamilyPolicy` | This sets the preferred ip addresses in case of a dual-stack server, there are three options [PreferDualStack, SingleStack, RequireDualStack], [more information on dual stack](https://kubernetes.io/docs/concepts/services-networking/dual-stack/) | `""` |
| `service.ipFamilies` | Sets the preferred IP variants and in which order they are preferred, the first family you list is used for the legacy .spec.ClusterIP field, [more information on dual stack](https://kubernetes.io/docs/concepts/services-networking/dual-stack/) | `""` |
| `sidecarResources` | Allows you to set the [resources][] for the sidecar containers in the StatefulSet | {} |
| `sysctlInit` | Allows you to enable the `sysctlInit` to set sysctl vm.max_map_count through privileged `initContainer`. | `enabled: false` |
| `sysctlVmMaxMapCount` | Sets the [vm.max_map_count][] needed for OpenSearch | `262144` |
Expand Down
8 changes: 8 additions & 0 deletions charts/opensearch/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ metadata:
{{ toYaml .Values.service.annotations | indent 4 }}
spec:
type: {{ .Values.service.type }}
{{- if (semverCompare ">= 1.23-0" .Capabilities.KubeVersion.Version) }}
{{- if .Values.service.ipFamilyPolicy }}
ipFamilyPolicy: {{ .Values.service.ipFamilyPolicy }}
{{- end }}
{{- if .Values.service.ipFamilies }}
ipFamilies: {{ .Values.service.ipFamilies }}
{{- end }}
{{- end }}
selector:
{{- include "opensearch.selectorLabels" . | nindent 4 }}
ports:
Expand Down
7 changes: 7 additions & 0 deletions charts/opensearch/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,13 @@ service:
headless:
annotations: {}
type: ClusterIP
# The IP family and IP families options are to set the behaviour in a dual-stack environment
# Omitting these values will let the service fall back to whatever the CNI dictates the defaults
# should be
#
# ipFamilyPolicy: SingleStack
# ipFamilies:
# - IPv4
nodePort: ""
annotations: {}
httpPortName: http
Expand Down