Skip to content

add daemonset extrahostports #98

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.

Sign up for GitHub

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

Open
bapung wants to merge 3 commits into
base: master
Choose a base branch
from
Open

add daemonset extrahostports #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2934c6
add .Values.controller.daemonset.extraHostPorts
bapung Dec 22, 2025
6f16216
update haproxy-ingress/README.md to include new value
bapung Dec 22, 2025
f482c60
add kind==DaemonSet check and explicit protocol: TCP
bapung Jan 4, 2026
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
1 change: 1 addition & 0 deletions haproxy-ingress/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ Parameter | Description | Default
`controller.daemonset.hostPorts.http` | If `controller.daemonset.useHostPort` is `true` and this is non-empty sets the hostPort for http | `"80"`
`controller.daemonset.hostPorts.https` | If `controller.daemonset.useHostPort` is `true` and this is non-empty sets the hostPort for https | `"443"`
`controller.daemonset.hostPorts.tcp` | If `controller.daemonset.useHostPort` is `true` use hostport for these ports from `tcp` | `[]`
`controller.daemonset.extraHostPorts` | If `controller.daemonset.useHostPort` is `true` configure additional host ports mapping. Each entry should specify `port`, `containerPort`, and `hostPort` | `[]`
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The documentation mentions that each entry should specify port, containerPort, and hostPort, but the template only uses the port field for generating the port name. This creates confusion about the purpose of the port field. Consider either removing port from the documentation if it's only used for naming, or clarifying its purpose (e.g., "Each entry should specify port (used for naming), containerPort, and hostPort").

Suggested change
`controller.daemonset.extraHostPorts` | If `controller.daemonset.useHostPort` is `true` configure additional host ports mapping. Each entry should specify `port`, `containerPort`, and `hostPort` | `[]`
`controller.daemonset.extraHostPorts` | If `controller.daemonset.useHostPort` is `true` configure additional host ports mapping. Each entry should specify `port` (used for naming), `containerPort`, and `hostPort` | `[]`

Copilot uses AI. Check for mistakes.
`controller.updateStrategy` | the update strategy settings | _see defaults below_
`controller.updateStrategy.type` | the update strategy type to use | `RollingUpdate`
`controller.updateStrategy.rollingUpdate.maxUnavailable` | the max number of unavailable controllers when doing rolling updates | `1`
Expand Down
11 changes: 11 additions & 0 deletions haproxy-ingress/templates/_podtemplate.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,17 @@ spec:
{{- end }}
{{- end }}
{{- end }}

{{- if eq $.Values.controller.kind "DaemonSet" }}
{{- if $.Values.controller.daemonset.useHostPort }}
{{- range $row := .Values.controller.daemonset.extraHostPorts }}
- name: "extra-port-{{ $row.port }}"
containerPort: {{ $row.containerPort }}
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The extraHostPorts feature lacks support for the hostIP configuration that is available for other daemonset host ports. All other daemonset host port configurations in this template (see lines 315-316, 325-326, 353-354) include conditional hostIP support when controller.daemonset.hostIP is set. This should be added to allow users to bind extra ports to a specific host IP address.

Suggested change
containerPort: {{ $row.containerPort }}
containerPort: {{ $row.containerPort }}
{{- if $.Values.controller.daemonset.hostIP }}
hostIP: {{ $.Values.controller.daemonset.hostIP }}
{{- end }}

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The extraHostPorts implementation is missing a protocol specification. All other port definitions in this template include protocol: TCP (see lines 336, 344, 351). This field should be added to maintain consistency and ensure proper protocol configuration.

Suggested change
containerPort: {{ $row.containerPort }}
containerPort: {{ $row.containerPort }}
protocol: TCP

Copilot uses AI. Check for mistakes.
protocol: TCP
hostPort: {{ $row.hostPort }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}

{{- define "haproxy-ingress.controller.probes" }}
Expand Down