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.

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

Remove unnecessary configuration in Queue Processor mode #745

Merged
merged 2 commits into from
Dec 22, 2022
Merged

Remove unnecessary configuration in Queue Processor mode #745

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11702e2
Remove unnecessary configuration in Queue P
johngmyers Dec 10, 2022
2071522
Address review comments
johngmyers Dec 15, 2022
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
10 changes: 5 additions & 5 deletions config/helm/aws-node-termination-handler/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ The configuration in this table applies to AWS Node Termination Handler in IMDS
| `daemonsetTolerations` | Tolerations for DaemonSet pod assignment. For backwards compatibility the `tolerations` has priority over this but shouldn't be used. | `[]` |
| `linuxTolerations` | Override `daemonsetTolerations` for the Linux DaemonSet. | `[]` |
| `windowsTolerations` | Override `daemonsetTolerations` for the Linux DaemonSet. | `[]` |
| `enableProbesServer` | If `true`, start an http server exposing `/healthz` endpoint for probes. | `false` |
| `enableProbesServer` | If `true`, start an http server exposing `/healthz` endpoint for probes. Only used in IMDS mode. | `false` |
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect. The probes server can be exposed whether NTH runs as a daemonset or as a deployment and is not related to IMDS mode. Our default configurations here only enable it for the daemonset, not the deployment, but that is not a technical limitation.

Indeed, our release artifacts explicitly include an aggregated YAML configuration with probes server enabled in Queue Processor mode as a deployment (see the artifact all-resources-queue-processor.yaml from any v1 release). Here's the code that generates that file:

aws-node-termination-handler/scripts/generate-k8s-yaml

Lines 61 to 66 in c3e411b

## Queue Processor Mode
$BUILD_DIR/helm template aws-node-termination-handler \
--namespace $NAMESPACE \
--set enableSqsTerminationDraining="true" \
--set enableProbesServer="true" \
$SCRIPTPATH/../config/helm/aws-node-termination-handler/ > $QP_AGG_RESOURCES_YAML

| `metadataTries` | The number of times to try requesting metadata. | `3` |
| `enableSpotInterruptionDraining` | If `true`, drain nodes when the spot interruption termination notice is received. | `true` |
| `enableScheduledEventDraining` | If `true`, drain nodes before the maintenance window starts for an EC2 instance scheduled event. | `true` |
| `enableRebalanceMonitoring` | If `true`, cordon nodes when the rebalance recommendation notice is received. If you'd like to drain the node in addition to cordoning, then also set `enableRebalanceDraining`. | `false` |
| `enableRebalanceDraining` | If `true`, drain nodes when the rebalance recommendation notice is received. | `false` |
| `enableSpotInterruptionDraining` | If `true`, drain nodes when the spot interruption termination notice is received. Only used in IMDS mode. | `true` |
| `enableScheduledEventDraining` | If `true`, drain nodes before the maintenance window starts for an EC2 instance scheduled event. Only used in IMDS mode. | `true` |
| `enableRebalanceMonitoring` | If `true`, cordon nodes when the rebalance recommendation notice is received. If you'd like to drain the node in addition to cordoning, then also set `enableRebalanceDraining`. Only used in IMDS mode. | `false` |
| `enableRebalanceDraining` | If `true`, drain nodes when the rebalance recommendation notice is received. Only used in IMDS mode. | `false` |

### Testing Configuration

Expand Down
8 changes: 0 additions & 8 deletions config/helm/aws-node-termination-handler/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,6 @@ spec:
- name: WEBHOOK_TEMPLATE
value: {{ .Values.webhookTemplate | quote }}
{{- end }}
- name: ENABLE_SPOT_INTERRUPTION_DRAINING
value: {{ .Values.enableSpotInterruptionDraining | quote }}
- name: ENABLE_SCHEDULED_EVENT_DRAINING
value: {{ .Values.enableScheduledEventDraining | quote }}
- name: ENABLE_REBALANCE_MONITORING
value: {{ .Values.enableRebalanceMonitoring | quote }}
- name: ENABLE_REBALANCE_DRAINING
value: {{ .Values.enableRebalanceDraining | quote }}
- name: ENABLE_SQS_TERMINATION_DRAINING
value: "true"
{{- with .Values.awsRegion }}
Expand Down
10 changes: 5 additions & 5 deletions config/helm/aws-node-termination-handler/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,22 +259,22 @@ daemonsetTolerations:
linuxTolerations: []
windowsTolerations: []

# If the probes server is running for the Daemonset
# If the probes server is running for the Daemonset. Only used in IMDS mode.
Copy link
Contributor

Choose a reason for hiding this comment

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

To be more correct, let's remove "for the Daemonset", since the probes server can be used in a deployment and is not related to IMDS mode.

Suggested change
# If the probes server is running for the Daemonset. Only used in IMDS mode.
# If the probes server is running

enableProbesServer: false

# Total number of times to try making the metadata request before failing.
metadataTries: 3

# enableSpotInterruptionDraining If false, do not drain nodes when the spot interruption termination notice is received
# enableSpotInterruptionDraining If false, do not drain nodes when the spot interruption termination notice is received. Only used in IMDS mode.
enableSpotInterruptionDraining: true

# enableScheduledEventDraining If false, do not drain nodes before the maintenance window starts for an EC2 instance scheduled event
# enableScheduledEventDraining If false, do not drain nodes before the maintenance window starts for an EC2 instance scheduled event. Only used in IMDS mode.
enableScheduledEventDraining: true

# enableRebalanceMonitoring If true, cordon nodes when the rebalance recommendation notice is received
# enableRebalanceMonitoring If true, cordon nodes when the rebalance recommendation notice is received. Only used in IMDS mode.
enableRebalanceMonitoring: false

# enableRebalanceDraining If true, drain nodes when the rebalance recommendation notice is received
# enableRebalanceDraining If true, drain nodes when the rebalance recommendation notice is received. Only used in IMDS mode.
enableRebalanceDraining: false

# ---------------------------------------------------------------------------------------------------------------------
Expand Down