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

Search all workloads if Pod selector doesn't match any workloads #541

Merged
merged 2 commits into from
Aug 30, 2019

Conversation

dturn
Copy link
Contributor

@dturn dturn commented Aug 28, 2019

What are you trying to accomplish with this PR?

Currently a service assumes that the pod selector will also match the workload. This isn't always true. Because it's possible for there to be no pods that match the service's pod-selector (when the workload is scaled to 0). We need to check every workload instead of assuming no such workload exists.

Closes: #483

How is this accomplished?
Search all workloads to find the correct set.

What could go wrong?

  • If the namespace has many StatefulSets and/or Deployments this could be a lot of data to fetch and check.

  • Should we always search everything? We could only search everything when there are no pods. And if there are pods walk the owner references back. However, walking the owner refs would be an extra API call though with less data...

@dturn dturn force-pushed the service-workload-pod-label-mismatched branch 2 times, most recently from e1f0141 to c12d28f Compare August 29, 2019 21:53
@timothysmith0609
Copy link
Contributor

If the namespace has many StatefulSets and/or Deployments this could be a lot of data to fetch and check.

I would take the hit and we can reconsider this if it becomes problematic

Should we always search everything? We could only search everything when there are no pods. And if there are pods walk the owner references back. However, walking the owner refs would be an extra API call though with less data...

I think we should grab everything we (could) need in sync; I don't really see a clean way around that. Trying to selectively grab resources only when there's no pods feels like it will open us up to strange edge cases.

Copy link
Contributor

@RyanBrushett RyanBrushett left a comment

Choose a reason for hiding this comment

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

I like it!

@dturn dturn changed the base branch from ss-service-0-pods to master August 30, 2019 21:28
@dturn dturn force-pushed the service-workload-pod-label-mismatched branch from c12d28f to 8ed7966 Compare August 30, 2019 21:30
@dturn dturn force-pushed the service-workload-pod-label-mismatched branch from 8ed7966 to 9267703 Compare August 30, 2019 21:32
@dturn dturn merged commit ede97b3 into master Aug 30, 2019
@dturn dturn deleted the service-workload-pod-label-mismatched branch August 30, 2019 21:43
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.

Service assumes pod selector is in sync with deployment's labels.
3 participants