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

docs(concepts): Clarify podAffinity/podAntiAffinity note #948

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

maxbrunet
Copy link
Contributor

@maxbrunet maxbrunet commented Dec 8, 2021

1. Issue, if available:

Related to #942

2. Description of changes:

Hello 👋

This notes has been confusing me since I read it. At first, I thought podAntiAffinity would cause scalability issues for the deployments using them, but reading further around topologySpreadConstraints (especially its KEP), it seems to be an issue for scalability of the Kubernetes Scheduler.

Do they plan to deprecate podAffinity/podAntiAffinity? It has been mentioned, but no concrete action toward it.
Or do they plan to improve them? They seem to be some activity in this direction (e.g. kubernetes/enhancements#2249), thus should Karpenter plan to actually support them? (And update the doc to something like "Karpenter doesn't support them for the moment (You can follow their consideration by subscribing to #942).")

TL;DR For this recommendation, imho the doc should have:

  • a short reason
  • its source (e.g. official Kubernetes documentation, KEP)

I may still be wrong about the why, but I hope to use this PR to get clarification for myself and everyone else.

3. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

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

@netlify
Copy link

netlify bot commented Dec 8, 2021

✔️ Deploy Preview for karpenter-docs-prod ready!

🔨 Explore the source changes: a7cc75c

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/61b119ad5547db000852a67c

😎 Browse the preview: https://deploy-preview-948--karpenter-docs-prod.netlify.app

@ellistarn
Copy link
Contributor

This looks like a bad link: kubernetes/enhancements#249. Can you update?

@maxbrunet
Copy link
Contributor Author

Oops I meant kubernetes/enhancements#2249

@ellistarn
Copy link
Contributor

We didn't initially implement affinity due to:

  • scalability recommendations
  • topology as an alternative
  • complexity/effort of implementation

I'm open minded to implementing pod antiaffinity (or at least a subset of the spec). However, pod affinity seems to be a bit of a nonstarter. When provisioning capacity, it's impossible to know whether or not all the pods that need to coschedule have been created yet. It's extremely easy to get wedged this way. This is more viable in static clusters, but still relatively challenging.

I think your current docs change is a good recommendation, and we can continue the discussion in #942. tl;dr, given the number of charts that use antiaffinity, I think we may be causing undue pain by not supporting it. It's probably worth the investment, at least insofar as antiaffinity is translatable to topology.

@maxbrunet
Copy link
Contributor Author

Thank you @ellistarn for the reply, I have updated the KEP link to point to the section describing the problems, please do not hesitate to suggest a better reference if you have one.

Copy link
Contributor

@ellistarn ellistarn left a comment

Choose a reason for hiding this comment

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

Thanks!

@ellistarn ellistarn merged commit 0e6633f into aws:main Dec 8, 2021
@maxbrunet
Copy link
Contributor Author

I have finally found the literal recommendation

Note: Inter-pod affinity and anti-affinity require substantial amount of processing which can slow down scheduling in large clusters significantly. We do not recommend using them in clusters larger than several hundred nodes.

https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#inter-pod-affinity-and-anti-affinity

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