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

Add nodeSelector option to daemonset-check podSpec #546

Closed
zeleena opened this issue Jul 2, 2020 · 2 comments
Closed

Add nodeSelector option to daemonset-check podSpec #546

zeleena opened this issue Jul 2, 2020 · 2 comments
Labels
feature request A request for a specific feature to be added to Kuberhealthy
Milestone

Comments

@zeleena
Copy link
Contributor

zeleena commented Jul 2, 2020

Add nodeSelector option to daemonset-check podSpec
Currently, the daemonset-check will fail in a cluster with both Linux nodes and Windows nodes. The daemonset-check creates a daemonset with pods that run PAUSE_CONTAINER_IMAGE, a Linux image. Since the Linux image PAUSE_CONTAINER_IMAGE will schedule on Windows nodes, no there is no nodeSelector to prevent this, it will fail since Windows nodes can't run Linux containers.

Insufficient workarounds
While it is true we could taint the Windows nodes and add a toleration, having the functionality to use the nodeSelector within the daemonset-check's podSpec would be highly preferable so that this node taint does not affect other deployments in the cluster.

Desired change
This change would include adding a field for NodeSelector under the daemonset's .spec.template.spec (underneath this podSpec block: https://github.com/Comcast/kuberhealthy/blob/v2.2.0/cmd/daemonset-check/main.go#L261 )

To grab the nodeSelector key(s) and value(s), it should be configurable in the values file. I was thinking it could either inherit the current nodeSelector configuration of the primary daemonset pod, or be set with extra environment variables via the extraEnvs key.

Why this is important
This is an issue because kuberhealthy will report "OK": false for the daemonset-check with error "waiting for all pods to come online" since Linux containers can't be ran on Windows nodes. Without the nodeSelector, I do not want to run the daemonset-check since it currently reports misleading information about my cluster.

@zeleena zeleena added the feature request A request for a specific feature to be added to Kuberhealthy label Jul 2, 2020
@integrii
Copy link
Collaborator

integrii commented Jul 6, 2020

Well described! I think this should be pretty easy to add. I think we should:

  • Add an environment variable to the helm chart for daemonset check (maybe NODE_SELECTOR)
  • Document the new environment variable in the README.md for the daemonset check
  • Read the environment variable into a global within the daemonset check
  • If the environment variable read is not blank (""), then set the node selector in the daemonset spec that the pod creates
  • Rev the daemonset check version in both the makefile and helm chart

@jonnydawg
Copy link
Collaborator

This has been addressed in a PR and is undergoing testing and debugging.

@integrii integrii added this to the 2.3.0 milestone Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A request for a specific feature to be added to Kuberhealthy
Projects
None yet
Development

No branches or pull requests

3 participants