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

Optional removal of node taint on successful IP assignment #146

Merged
merged 8 commits into from
May 13, 2024

Conversation

RagnarHal
Copy link
Contributor

@RagnarHal RagnarHal commented May 7, 2024

This PR adds a feature that allows removal of a node taint upon a successful IP address assignment. Using this should avoid potential race conditions where workloads may be scheduled on newly provisioned nodes before KubeIP has completed the IP assignment. I have tested this on a GKE cluster, but not in AWS. There shouldn't be anything vendor-specific here.

The specific use case we have for this is that we run some jobs on elastic/autoscaling node pools, which call services with IP-whitelisting. We had observed that in rare cases, it would take a while (up to a minute) for the nodes to receive their IP addresses, and meanwhile workloads were free to be scheduled on the nodes. We wanted to ensure that calls to these services were not made before the node had received a static IP address.

This should be backwards-compatible with current deployments of KubeIP, and if the feature is not enabled then it should behave exactly as it does currently.

Using this feature requires an additional, potentially sensitive permission on the ClusterRole. This is explained in the readme, but not using the feature (i.e. not explicitly setting the taint-key configuration option) does not require any additional permissions, and it will still work with only the "get" permission.

Copy link
Collaborator

@alexei-led alexei-led left a comment

Choose a reason for hiding this comment

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

@RagnarHal LGTM thank you for this PR. Reasonable use case.

cmd/main.go Show resolved Hide resolved
@alexei-led alexei-led merged commit f24b10b into doitintl:master May 13, 2024
1 of 2 checks passed
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