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

sync podstatus from leaf to root when root node from ready to notready #770

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gao12312
Copy link
Contributor

What type of PR is this?

/kind feature

What does this PR do?

sync podstatus from leaf to root when root node from ready to notready

Which issue(s) does this PR fix?

Fixes #705

Special notes for your reviewer:

Does this PR introduce a user-facing change?


@gao12312 gao12312 force-pushed the pod_snyc branch 2 times, most recently from 7e73738 to ca3f31b Compare November 25, 2024 03:33
@gao12312 gao12312 closed this Nov 25, 2024
@gao12312 gao12312 reopened this Nov 25, 2024
@gao12312 gao12312 closed this Nov 25, 2024
@gao12312 gao12312 reopened this Nov 25, 2024
@gao12312 gao12312 force-pushed the pod_snyc branch 2 times, most recently from f2db91b to 6428c2e Compare November 25, 2024 07:13
@gao12312 gao12312 closed this Nov 25, 2024
@gao12312 gao12312 reopened this Nov 25, 2024
@gao12312 gao12312 closed this Nov 25, 2024
@gao12312 gao12312 reopened this Nov 25, 2024
@gao12312 gao12312 closed this Nov 25, 2024
@gao12312 gao12312 reopened this Nov 25, 2024
@gao12312 gao12312 force-pushed the pod_snyc branch 2 times, most recently from a7c3205 to 0467e7f Compare December 5, 2024 09:24
@gao12312 gao12312 force-pushed the pod_snyc branch 7 times, most recently from 5c86aee to 1a2ede0 Compare December 10, 2024 07:33
@gao12312 gao12312 closed this Dec 10, 2024
@gao12312 gao12312 reopened this Dec 10, 2024
@gao12312 gao12312 closed this Dec 10, 2024
@gao12312 gao12312 reopened this Dec 10, 2024
@gao12312 gao12312 closed this Dec 10, 2024
@gao12312 gao12312 reopened this Dec 10, 2024
@gao12312 gao12312 force-pushed the pod_snyc branch 2 times, most recently from a032fb4 to 21c2c70 Compare December 11, 2024 08:12
@duanmengkk
Copy link
Contributor

add unit test

},
UpdateFunc: func(updateEvent event.UpdateEvent) bool {
pod1 := updateEvent.ObjectOld.(*corev1.Pod)
pod2 := updateEvent.ObjectNew.(*corev1.Pod)
Copy link
Member

Choose a reason for hiding this comment

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

pod1 and pod2 is strange

if !skipFunc(updateEvent.ObjectNew) {
return false
}
return !reflect.DeepEqual(pod1.Status, pod2.Status)
Copy link
Member

Choose a reason for hiding this comment

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

skipFunc and reflect.DeepEqual can be written together

return reconcile.Result{}, nil
}

if !c.GlobalLeafManager.HasNode(rootpod.Spec.NodeName) {
Copy link
Member

Choose a reason for hiding this comment

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

In one2cluster mode, the nodename of the rootpod is not in the subset cluster

if !skipFunc(updateEvent.ObjectNew) {
return false
}
return !reflect.DeepEqual(pod1.Status, pod2.Status)
Copy link
Member

Choose a reason for hiding this comment

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

You should only judge the conditions in status, not all the contents of status

return reconcile.Result{RequeueAfter: utils.DefaultRequeueTime}, nil
}

if !podutils.IsKosmosPod(&rootpod) {
Copy link
Member

Choose a reason for hiding this comment

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

It's the same as skipFunc

return reconcile.Result{}, nil
}

if podutils.IsKosmosPod(leafPod) && !reflect.DeepEqual(rootpod.Status, leafPod.Status) {
Copy link
Member

Choose a reason for hiding this comment

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

status.condition?

podutils.FitObjectMeta(&rPodCopy.ObjectMeta)
if err := c.RootClient.Status().Update(ctx, rPodCopy); err != nil && !apierrors.IsNotFound(err) {
klog.Errorf("error while updating rootpod status in kubernetes, %s", err)
return reconcile.Result{}, nil
Copy link
Member

Choose a reason for hiding this comment

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

requeue?

rPodCopy := rootpod.DeepCopy()
rPodCopy.Status = leafPod.Status
podutils.FitObjectMeta(&rPodCopy.ObjectMeta)
if err := c.RootClient.Status().Update(ctx, rPodCopy); err != nil && !apierrors.IsNotFound(err) {
Copy link
Member

Choose a reason for hiding this comment

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

apierrors.IsNotFound is redundant

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.

update pod status from leaf to root
3 participants