Fix podgroup not created #3561
Fix podgroup not created #3561
Conversation
|
Welcome @liuyuanchun11! |
volcano-sh-bot
added
the
size/S
|
Is there a scenario that can be reproduced? |
There is a high probability that this issue occurs when multiple deployments are upgraded in rolling mode. The key point is that the add pod event is received before the replicaset event. |
| err := pg.vcClient.SchedulingV1beta1().PodGroups(rs.Namespace).Delete(context.TODO(), pgName, metav1.DeleteOptions{}) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| klog.Errorf("Failed to delete PodGroup <%s/%s>: %v", rs.Namespace, pgName, err) | ||
| } | ||
| } else if *rs.Spec.Replicas > 0 { |
There was a problem hiding this comment.
else is no need and we should add a comment to explain why this is added.
There was a problem hiding this comment.
Sorry, I don't understand what you mean by else is not needed. Are you saying that the webhook already guarantees that replicas are unsigned, so there's no need to write a > 0 check?
The comment I'll add later.
There was a problem hiding this comment.
Sorry for misunderstanding, I mean we can use if *rs.Spec.Replicas > 0 directly and no need to use else if.
There was a problem hiding this comment.
Modified based on review comments.
| if podList != nil { | ||
| for _, pod := range podList.Items { | ||
| klog.V(4).Infof("Try to create podgroup for pod %s/%s", pod.Namespace, pod.Name) | ||
| err := pg.createNormalPodPGIfNotExist(&pod) |
There was a problem hiding this comment.
How about just check whether rs associated pg exists first, only create pg when it doesn't exist to avoid unnecessary API request, cause it's a rare case here.
There was a problem hiding this comment.
createNormalPodPGIfNotExist will checks if the podgroup exists.
volcano-sh-bot
added
size/M
|
@liuyuanchun11 please log a issue and add the description for the bug and associate the pr with issue. |
liuyuanchun11
changed the title
liuyuanchun11
force-pushed
the
fix_pg_not_create
branch
from
cc4c00f
to
6e25fc5
Compare
volcano-sh-bot
added
size/S
done, pr has been associated with issue #3563 |
volcano-sh-bot
added
the
do-not-merge/invalid-commit-message
liuyuanchun11
force-pushed
the
fix_pg_not_create
branch
from
93384c0
to
df27590
Compare
volcano-sh-bot
added
retest-not-required-docs-only
approved
|
/reopen |
|
@liuyuanchun11: Reopened this PR. In 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/test-infra repository. |
volcano-sh-bot
added
do-not-merge/invalid-commit-message
volcano-sh-bot
added
size/S
Signed-off-by: liuyuanchun <[email protected]>
liuyuanchun11
force-pushed
the
fix_pg_not_create
branch
from
05c04ba
to
7c9bd38
Compare
volcano-sh-bot
removed
the
do-not-merge/invalid-commit-message
liuyuanchun11
changed the title
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: william-wang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
approved
During the rolling upgrade of replicaset, pg_controller occasionally receives the addPod event and creates a podgroup. Then, pg_controller receives the addReplicaSet (replicas = 0) event and deletes the corresponding podgroup (to solve the pg fc problem). The updateReplicaSet (replicas = 1) event is received but not processed. As a result, the pod group corresponding to the pod is not correctly created.
issue: #3563