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

re-enable the PDB eviction test #1334

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

tzneal
Copy link
Contributor

@tzneal tzneal commented Feb 14, 2022

1. Issue, if available:
#638

2. Description of changes:

  • the pod must be in a non-pending state for eviction to consider the PDB
  • the Status.ObservedGeneration must be >= the pdb.Generation or the eviction results in a 429 retry with a 10 second delay

3. How was this change tested?

Unit testing

4. 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 Feb 14, 2022

✔️ Deploy Preview for karpenter-docs-prod canceled.

🔨 Explore the source changes: 1f52116

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

// kube-controller-manager normally does this, but we don't have one. If this isn't modified
// the eviction controller assumes that the PDB hasn't been processed by the disruption controller
// yet and adds a 10 second retry to our evict() call
pdb.Status.ObservedGeneration = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we include this in the test.PodDisruptionBudget library? We will then need to use ExpectCreatedWithStatus on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, done, the update call required the ResourceVersion to match, I thought that was with everything so I'm not sure why it only showed up for the PDB.

@tzneal tzneal force-pushed the fix-638-reenable-pdb-eviction-test branch from 3ace514 to ee5c7aa Compare February 14, 2022 18:23
pkg/test/pods.go Outdated Show resolved Hide resolved
ellistarn
ellistarn previously approved these changes Feb 14, 2022
- the pod must be in a non-pending state for eviction
  to consider the PDB
- the Status.ObservedGeneration must be >= the pdb.Generation
  or the eviction results in a 429 retry with a 10 second delay

Fixes aws#638
@tzneal tzneal force-pushed the fix-638-reenable-pdb-eviction-test branch from ee5c7aa to 1f52116 Compare February 14, 2022 18:57
@ellistarn ellistarn merged commit 3bdbe13 into aws:main Feb 14, 2022
@tzneal tzneal deleted the fix-638-reenable-pdb-eviction-test branch February 15, 2022 20:53
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