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

scheduler: fix klog.KObjSlice when applied to []*NodeInfo #125699

Merged
merged 2 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/scheduler/framework/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,21 @@ type NodeInfo struct {
Generation int64
}

// NodeInfo implements KMetadata, so for example klog.KObjSlice(nodes) works
// when nodes is a []*NodeInfo.
var _ klog.KMetadata = &NodeInfo{}

func (n *NodeInfo) GetName() string {
if n == nil {
return "<nil>"
}
if n.node == nil {
return "<no node>"
}
return n.node.Name
}
func (n *NodeInfo) GetNamespace() string { return "" }

// nextGeneration: Let's make sure history never forgets the name...
// Increments the generation number monotonically ensuring that generation numbers never collide.
// Collision of the generation numbers would be particularly problematic if a node was deleted and
Expand Down
17 changes: 17 additions & 0 deletions pkg/scheduler/framework/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package framework
import (
"fmt"
"reflect"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -29,9 +30,11 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/features"
st "k8s.io/kubernetes/pkg/scheduler/testing"
"k8s.io/kubernetes/test/utils/ktesting"
"k8s.io/kubernetes/test/utils/ktesting/initoption"
)

func TestNewResource(t *testing.T) {
Expand Down Expand Up @@ -1657,3 +1660,17 @@ func TestCloudEvent_Match(t *testing.T) {
})
}
}

func TestNodeInfoKMetadata(t *testing.T) {
tCtx := ktesting.Init(t, initoption.BufferLogs(true))
logger := tCtx.Logger()
logger.Info("Some NodeInfo slice", "nodes", klog.KObjSlice([]*NodeInfo{nil, {}, {node: &v1.Node{}}, {node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "worker"}}}}))

output := logger.GetSink().(ktesting.Underlier).GetBuffer().String()

// The initial nil entry gets turned into empty ObjectRef by klog,
// which becomes an empty string during output formatting.
if !strings.Contains(output, `Some NodeInfo slice nodes=["","<no node>","","worker"]`) {
tCtx.Fatalf("unexpected output:\n%s", output)
}
}
10 changes: 10 additions & 0 deletions test/utils/ktesting/initoption/initoption.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ func PerTestOutput(enabled bool) InitOption {
c.PerTestOutput = enabled
}
}

// BufferLogs controls whether log entries are captured in memory in addition
// to being printed. Off by default. Unit tests that want to verify that
// log entries are emitted as expected can turn this on and then retrieve
// the captured log through the Underlier LogSink interface.
func BufferLogs(enabled bool) InitOption {
return func(c *internal.InitConfig) {
c.BufferLogs = enabled
}
}
1 change: 1 addition & 0 deletions test/utils/ktesting/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ package internal

type InitConfig struct {
PerTestOutput bool
BufferLogs bool
}
4 changes: 3 additions & 1 deletion test/utils/ktesting/tcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ import (
)

// Underlier is the additional interface implemented by the per-test LogSink
// behind [TContext.Logger].
// behind [TContext.Logger]. Together with [initoption.BufferLogs] it can be
// used to capture log output in memory to check it in tests.
type Underlier = ktesting.Underlier

// CleanupGracePeriod is the time that a [TContext] gets canceled before the
Expand Down Expand Up @@ -245,6 +246,7 @@ func Init(tb TB, opts ...InitOption) TContext {
}),
ktesting.VerbosityFlagName("v"),
ktesting.VModuleFlagName("vmodule"),
ktesting.BufferLogs(c.BufferLogs),
)

// Copy klog settings instead of making the ktesting logger
Expand Down