feat: improve pod disruption budget validation #98
Conversation
lib/web-service/web-service.ts
Outdated
| const { minAvailable, maxUnavailable } = | ||
| props.podDisruptionBudget as PodDisruptionBudgetProps; | ||
| podDisruptionBudget as PodDisruptionBudgetProps; |
There was a problem hiding this comment.
No need to force the type because it's already set in the function argument
| podDisruptionBudget as PodDisruptionBudgetProps; | |
| podDisruptionBudget; |
test/web-service/web-service.test.ts
Outdated
| [{ replicas: 2 }, { minAvailable: IntOrString.fromNumber(2) }], | ||
| [{ replicas: 2 }, { maxUnavailable: IntOrString.fromNumber(0) }], | ||
| [{ replicas: 3 }, { minAvailable: IntOrString.fromString("100%") }], | ||
| [{ replicas: 3 }, { maxUnavailable: IntOrString.fromString("0%") }], | ||
| [{ replicas: 10 }, { minAvailable: IntOrString.fromString("91%") }], | ||
| [{ replicas: 42 }, { maxUnavailable: IntOrString.fromString("0%") }], | ||
| [ | ||
| { minReplicas: 5, maxReplicas: 10 }, | ||
| { minAvailable: IntOrString.fromString("85%") }, | ||
| ], |
There was a problem hiding this comment.
Are these removed for a reason?
There was a problem hiding this comment.
Yes, these were expected to fail before. Now, after validation condition is changed, these 4 test cases are failing because replicas == minAvailable. That's why they are removed.
There was a problem hiding this comment.
These tests are checking cases when validation does fail. So, for example, case
[{ replicas: 2 }, { minAvailable: IntOrString.fromNumber(2) }],
should still fail, because if we only have 2 pods, and PDB enforces that there must be 2, we would get eviction errors.
So I think the change to if (min > minReplicas) check is wrong, it should be >= (error if PDB's minAvailable is equal to minReplicas).
There was a problem hiding this comment.
That reverts it to the initial condition.
nemanja-kovacevic-thinkit
force-pushed
the
PLT-108-improve-pdb-validation
branch
from
78d48fb
to
c7330db
Compare
nemanja-kovacevic-thinkit
force-pushed
the
PLT-108-improve-pdb-validation
branch
from
c7330db
to
64c81f9
Compare
https://techfromsage.atlassian.net/browse/PLT-1083 Problem: PR #98 is merged. After merging we noticed that we are unable to get a green build against main branch which means that there is no release. Problem caused one e2e test which expectation is invalid. Green build on the branch happened because tests are flaky. Fix: Update e2e test expectation
|
🎉 This issue has been resolved in version 5.3.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
https://techfromsage.atlassian.net/browse/PLT-1083
Problem: While upgrading EKS to Kubernetes 1.30 and 1.31 we ran into issues with Node Group upgrades as they couldn’t evict some Pods. We found the reason to be PodDisruptionBudget violations where PDB is configured with minAvailable: 1 while there is only 1 replica.
Fix: WebService construct validates that when PodDisruptionBudget is enabled, then Deployment’s min replicas is greater (but not equal) than PDB’s minAvailable.