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

add pod info when failing to add pod to unschedulableQ #85470

Merged
merged 1 commit into from Dec 4, 2019

Conversation

@cwdsuzhou
Copy link
Member

cwdsuzhou commented Nov 20, 2019

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

add pod info when failing to add pod to unschedulableQ

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


Copy link
Member

wgliang left a comment

/ok-to-test
/lgtm

@cwdsuzhou

This comment has been minimized.

Copy link
Member Author

cwdsuzhou commented Nov 20, 2019

/retest

@alculquicondor

This comment has been minimized.

Copy link
Member

alculquicondor commented Nov 20, 2019

@@ -329,7 +329,7 @@ func (p *PriorityQueue) AddUnschedulableIfNotPresent(pInfo *framework.PodInfo, p
defer p.lock.Unlock()
pod := pInfo.Pod
if p.unschedulableQ.get(pod) != nil {
return fmt.Errorf("pod is already present in unschedulableQ")
return fmt.Errorf("pod: %v/%v is already present in unschedulableQ", pod.Namespace, pod.Name)

This comment has been minimized.

Copy link
@liu-cong

liu-cong Nov 20, 2019

Contributor

Can you change unschedulableQ to unschedulable queue?
Can you also make the same change for lines(338 and 341) below for active and backoff queues?

Thanks

This comment has been minimized.

Copy link
@cwdsuzhou

cwdsuzhou Nov 21, 2019

Author Member

done

@cwdsuzhou cwdsuzhou force-pushed the cwdsuzhou:add_pod_info branch from fb97ad0 to c9786dd Nov 21, 2019
@k8s-ci-robot k8s-ci-robot added size/S and removed lgtm size/XS labels Nov 21, 2019
@cwdsuzhou

This comment has been minimized.

Copy link
Member Author

cwdsuzhou commented Nov 21, 2019

ping @wgliang for approval

@cwdsuzhou

This comment has been minimized.

Copy link
Member Author

cwdsuzhou commented Nov 21, 2019

/retest

}

// Refresh the timestamp since the pod is re-added.
pInfo.Timestamp = p.clock.Now()
if _, exists, _ := p.activeQ.Get(pInfo); exists {
return fmt.Errorf("pod is already present in the activeQ")
return fmt.Errorf("pod is already present in the active queue")

This comment has been minimized.

Copy link
@liu-cong

liu-cong Nov 21, 2019

Contributor

can you also print the pod namespace and name in the error message? Same for the backoff queue line below

This comment has been minimized.

Copy link
@cwdsuzhou

cwdsuzhou Nov 22, 2019

Author Member

seems my latest commit did not send successfully

Updated it.

@@ -329,16 +329,16 @@ func (p *PriorityQueue) AddUnschedulableIfNotPresent(pInfo *framework.PodInfo, p
defer p.lock.Unlock()
pod := pInfo.Pod
if p.unschedulableQ.get(pod) != nil {
return fmt.Errorf("pod is already present in unschedulableQ")
return fmt.Errorf("pod: %v/%v is already present in unschedulable queue", pod.Namespace, pod.Name)

This comment has been minimized.

Copy link
@liu-cong

liu-cong Nov 21, 2019

Contributor

nit: seems that you can use the nsNameForPod helper method.

This comment has been minimized.

Copy link
@cwdsuzhou

cwdsuzhou Nov 22, 2019

Author Member

done

@cwdsuzhou cwdsuzhou force-pushed the cwdsuzhou:add_pod_info branch from c9786dd to 09f3c2e Nov 22, 2019
@cwdsuzhou cwdsuzhou force-pushed the cwdsuzhou:add_pod_info branch from 09f3c2e to 81afa77 Nov 22, 2019
@liu-cong

This comment has been minimized.

Copy link
Contributor

liu-cong commented Nov 22, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Nov 22, 2019
@cwdsuzhou

This comment has been minimized.

Copy link
Member Author

cwdsuzhou commented Nov 22, 2019

/assign @bsalamat

ping @bsalamat for approval

Copy link
Member

bsalamat left a comment

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 4, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, cwdsuzhou

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit ca7ad98 into kubernetes:master Dec 4, 2019
15 checks passed
15 checks passed
cla/linuxfoundation cwdsuzhou authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.