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

[aws-node-termination-handler]: Use hostPort only with hostNetwork #527

Closed

Conversation

hwoarang
Copy link

Description of changes

There is no need to use the hostPort option if we are not directly
connected to the hostNetwork. Using the hostPort option makes that port
unavailable for binding to any other service on the node which is
not really desirable if the pod itself is not on the host network.

Checklist

  • Incremented the chart version in Chart.yaml for the modified chart(s)
  • Manually tested. Describe what testing was done in the testing section below
  • Make sure the title of the PR is a good description that can go into the release notes

Testing

No testing was done apart from checking with helmTemplate that no hostPort is
present in the rendered files when useHostNetwork is not enabled.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

There is no need to use the hostPort option if we are not directly
connected to the hostNetwork. Using the hostPort option makes that port
unavailable for binding to any other service on the node which is
not really desirable if the pod itself is not on the host network.
@hwoarang hwoarang requested a review from bwagner5 as a code owner May 24, 2021 08:48
@bwagner5
Copy link
Collaborator

bwagner5 commented Jun 8, 2021

Sorry for the delay! We only take PRs to NTH through the application's repo here https://github.com/aws/aws-node-termination-handler , this allows us to run our suite of e2e tests and then we sync over the chart to eks-charts when we release NTH . Would you mind opening this PR in that repo?

The changes look good here. When you do open the PR in the NTH repo, you can drop the chart version change since it will automatically be incremented when we sync the chart here. Thanks!

@bwagner5 bwagner5 closed this Jun 8, 2021
@hwoarang hwoarang deleted the termination-handler-no-hostPort branch June 9, 2021 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants