Skip to content

Commit

Permalink
Merge pull request kubernetes#120933 from mengjiao-liu/contextual-log…
Browse files Browse the repository at this point in the history
…ging-scheduler-remaining-part

kube-scheduler: convert the remaining part to use contextual logging
  • Loading branch information
k8s-ci-robot authored Oct 27, 2023
2 parents c8125c4 + 2cea512 commit fd5c406
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 92 deletions.
4 changes: 1 addition & 3 deletions hack/logcheck.conf
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ structured k8s.io/apiserver/pkg/server/options/encryptionconfig/.*
# The following packages have been migrated to contextual logging.
# Packages matched here do not have to be listed above because
# "contextual" implies "structured".
# TODO next: contextual k8s.io/kubernetes/pkg/scheduler/.*
# A few files involved in startup migrated already to contextual
# We can't enable contextual logcheck until all are migrated
contextual k8s.io/client-go/tools/events/.*
contextual k8s.io/client-go/tools/record/.*
contextual k8s.io/dynamic-resource-allocation/.*
contextual k8s.io/kubernetes/cmd/kube-scheduler/.*
contextual k8s.io/kubernetes/pkg/controller/.*
contextual k8s.io/kubernetes/pkg/scheduler/.*
contextual k8s.io/kubernetes/test/e2e/dra/.*

# As long as contextual logging is alpha or beta, all WithName, WithValues,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (pl *DefaultPreemption) SelectVictimsOnNode(
logger := klog.FromContext(ctx)
var potentialVictims []*framework.PodInfo
removePod := func(rpi *framework.PodInfo) error {
if err := nodeInfo.RemovePod(rpi.Pod); err != nil {
if err := nodeInfo.RemovePod(logger, rpi.Pod); err != nil {
return err
}
status := pl.fh.RunPreFilterExtensionRemovePod(ctx, state, pod, rpi, nodeInfo)
Expand Down
4 changes: 2 additions & 2 deletions pkg/scheduler/framework/preemption/preemption.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (ev *Evaluator) prepareCandidate(ctx context.Context, c Candidate, pod *v1.
// Otherwise we should delete the victim.
if waitingPod := fh.GetWaitingPod(victim.UID); waitingPod != nil {
waitingPod.Reject(pluginName, "preempted")
klog.V(2).InfoS("Preemptor pod rejected a waiting pod", "preemptor", klog.KObj(pod), "waitingPod", klog.KObj(victim), "node", c.Name())
logger.V(2).Info("Preemptor pod rejected a waiting pod", "preemptor", klog.KObj(pod), "waitingPod", klog.KObj(victim), "node", c.Name())
} else {
if feature.DefaultFeatureGate.Enabled(features.PodDisruptionConditions) {
condition := &v1.PodCondition{
Expand All @@ -377,7 +377,7 @@ func (ev *Evaluator) prepareCandidate(ctx context.Context, c Candidate, pod *v1.
errCh.SendErrorWithCancel(err, cancel)
return
}
klog.V(2).InfoS("Preemptor Pod preempted victim Pod", "preemptor", klog.KObj(pod), "victim", klog.KObj(victim), "node", c.Name())
logger.V(2).Info("Preemptor Pod preempted victim Pod", "preemptor", klog.KObj(pod), "victim", klog.KObj(victim), "node", c.Name())
}

fh.EventRecorder().Eventf(victim, pod, v1.EventTypeNormal, "Preempted", "Preempting", "Preempted by pod %v on node %v", pod.UID, c.Name())
Expand Down
12 changes: 6 additions & 6 deletions pkg/scheduler/framework/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -748,12 +748,12 @@ func podWithRequiredAntiAffinity(p *v1.Pod) bool {
len(affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) != 0
}

func removeFromSlice(s []*PodInfo, k string) ([]*PodInfo, bool) {
func removeFromSlice(logger klog.Logger, s []*PodInfo, k string) ([]*PodInfo, bool) {
var removed bool
for i := range s {
tmpKey, err := GetPodKey(s[i].Pod)
if err != nil {
klog.ErrorS(err, "Cannot get pod key", "pod", klog.KObj(s[i].Pod))
logger.Error(err, "Cannot get pod key", "pod", klog.KObj(s[i].Pod))
continue
}
if k == tmpKey {
Expand All @@ -772,20 +772,20 @@ func removeFromSlice(s []*PodInfo, k string) ([]*PodInfo, bool) {
}

// RemovePod subtracts pod information from this NodeInfo.
func (n *NodeInfo) RemovePod(pod *v1.Pod) error {
func (n *NodeInfo) RemovePod(logger klog.Logger, pod *v1.Pod) error {
k, err := GetPodKey(pod)
if err != nil {
return err
}
if podWithAffinity(pod) {
n.PodsWithAffinity, _ = removeFromSlice(n.PodsWithAffinity, k)
n.PodsWithAffinity, _ = removeFromSlice(logger, n.PodsWithAffinity, k)
}
if podWithRequiredAntiAffinity(pod) {
n.PodsWithRequiredAntiAffinity, _ = removeFromSlice(n.PodsWithRequiredAntiAffinity, k)
n.PodsWithRequiredAntiAffinity, _ = removeFromSlice(logger, n.PodsWithRequiredAntiAffinity, k)
}

var removed bool
if n.Pods, removed = removeFromSlice(n.Pods, k); removed {
if n.Pods, removed = removeFromSlice(logger, n.Pods, k); removed {
n.update(pod, -1)
return nil
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/scheduler/framework/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
st "k8s.io/kubernetes/pkg/scheduler/testing"
"k8s.io/kubernetes/test/utils/ktesting"
)

func TestNewResource(t *testing.T) {
Expand Down Expand Up @@ -1083,10 +1084,11 @@ func TestNodeInfoRemovePod(t *testing.T) {

for i, test := range tests {
t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
logger, _ := ktesting.NewTestContext(t)
ni := fakeNodeInfo(pods...)

gen := ni.Generation
err := ni.RemovePod(test.pod)
err := ni.RemovePod(logger, test.pod)
if err != nil {
if test.errExpected {
expectedErrorMsg := fmt.Errorf("no corresponding pod %s in pods of node %s", test.pod.Name, ni.Node().Name)
Expand Down
2 changes: 1 addition & 1 deletion pkg/scheduler/internal/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ func (cache *cacheImpl) removePod(logger klog.Logger, pod *v1.Pod) error {
if !ok {
logger.Error(nil, "Node not found when trying to remove pod", "node", klog.KRef("", pod.Spec.NodeName), "podKey", key, "pod", klog.KObj(pod))
} else {
if err := n.info.RemovePod(pod); err != nil {
if err := n.info.RemovePod(logger, pod); err != nil {
return err
}
if len(n.info.Pods) == 0 && n.info.Node() == nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/scheduler/internal/queue/scheduling_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ type SchedulingQueue interface {
SchedulingCycle() int64
// Pop removes the head of the queue and returns it. It blocks if the
// queue is empty and waits until a new item is added to the queue.
Pop() (*framework.QueuedPodInfo, error)
Pop(logger klog.Logger) (*framework.QueuedPodInfo, error)
// Done must be called for pod returned by Pop. This allows the queue to
// keep track of which pods are currently being processed.
Done(types.UID)
Expand Down Expand Up @@ -859,15 +859,15 @@ func (p *PriorityQueue) flushUnschedulablePodsLeftover(logger klog.Logger) {
// Pop removes the head of the active queue and returns it. It blocks if the
// activeQ is empty and waits until a new item is added to the queue. It
// increments scheduling cycle when a pod is popped.
func (p *PriorityQueue) Pop() (*framework.QueuedPodInfo, error) {
func (p *PriorityQueue) Pop(logger klog.Logger) (*framework.QueuedPodInfo, error) {
p.lock.Lock()
defer p.lock.Unlock()
for p.activeQ.Len() == 0 {
// When the queue is empty, invocation of Pop() is blocked until new item is enqueued.
// When Close() is called, the p.closed is set and the condition is broadcast,
// which causes this loop to continue and return from the Pop().
if p.closed {
klog.V(2).InfoS("Scheduling queue is closed")
logger.V(2).Info("Scheduling queue is closed")
return nil, nil
}
p.cond.Wait()
Expand Down
Loading

0 comments on commit fd5c406

Please sign in to comment.