-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Replicas on same node #149
Conversation
Question: should this check also be done for StatefulSets? |
In my opinion, it makes sense to include statefulsets |
@marians @pipo02mix Thank you both for this report! Yes probably more importantly so with sts imho. |
@derailed Just added support for StatefulSets. I'd welcome a review. |
// checkSameNode verifies if all replicas of the StatefulSet are running on | ||
// the same node. | ||
func (s *StatefulSet) checkSameNode(ctx context.Context, st *appsv1.StatefulSet) { | ||
if *st.Spec.Replicas <= 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marians Check nil?
return | ||
} | ||
|
||
nodeMap := make(map[string]int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think we need to count here ie if the node is already in the map the pods are collocated
s.ListPodsBySelector(st.Namespace, st.Spec.Selector
make(map[string]struct{}, len(oo))
if _, ok := nodeMap[pod.Spec.NodeName]; ok {
s.AddCode(...)
return
That said I don't think these are valid general checks ie ES multiple shards can land on the same node? Thus that would raise a false positive??
Closing, as I'm not working on this any more. |
Closes #143
This PR implements a check that issues a warning if all replicas of a deployment are scheduled to the same host.
Preview: