fix: Do not create PodDisruptionBudget for single replica topology mode#1707
Conversation
kubevirt-bot
added
release-note-none
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new single-node PDB test uses
MatchError(errors.IsNotFound, "errors.IsNotFound"), buterrors.IsNotFoundreturns a bool, not an error; consider asserting withExpect(apierrors.IsNotFound(err)).To(BeTrue())or usingMatchErrorwith the actual error or its string instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new single-node PDB test uses `MatchError(errors.IsNotFound, "errors.IsNotFound")`, but `errors.IsNotFound` returns a bool, not an error; consider asserting with `Expect(apierrors.IsNotFound(err)).To(BeTrue())` or using `MatchError` with the actual error or its string instead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
/lgtm |
0xFelix
left a comment
0xFelix
left a comment
There was a problem hiding this comment.
In general I approve of this change, but the wording is a bit off. It is possible to have multiple replicas, even on a single node. This should say that it does not create PDBs for single replicas / when in single replica mode.
akrejcir
force-pushed
the
dont-deploy-pdb-on-sno
branch
from
b881488 to
a207e7c
Compare
akrejcir
changed the title
|
Good point. I've updated the commit message and PR description. |
| } | ||
| }) | ||
|
|
||
| It("should not create PodDisruptionBudget on single node clusters", func() { |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 0xFelix The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
The PodDisruptionBudget does not make sense in single replica topology mode. It would have "spec.minAvailable" set to 1, but there would always be one pod. It would cause a warning to be always present: "The pod disruption budget is at the minimum disruptions allowed level. The number of current healthy pods is equal to the desired healthy pods." Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>
akrejcir
force-pushed
the
dont-deploy-pdb-on-sno
branch
from
a207e7c to
a63407f
Compare
|
|
/lgtm |
|
/cherry-pick release-v0.25 |
|
@akrejcir: new pull request created: #1714 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@akrejcir: new pull request created: #1715 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
5 participants
What this PR does / why we need it:
The
PodDisruptionBudgetis not useful in single replica topology mode. It would havespec.minAvailableset to 1, but there would always be one pod. It would cause a warning to be shown:`The pod disruption budget is at the minimum disruptions allowed level. The number of current healthy pods is equal to the desired healthy pods."
Which issue(s) this PR fixes:
Jira: https://issues.redhat.com/browse/CNV-71871
Release note: