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

Use NodeWrapper to directly initialize nodes with labels #92514

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/scheduler/testing/BUILD
Expand Up @@ -19,6 +19,7 @@ go_library(
"//pkg/scheduler/framework/v1alpha1:go_default_library",
"//pkg/scheduler/util:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
Expand Down
31 changes: 29 additions & 2 deletions pkg/scheduler/testing/wrappers.go
Expand Up @@ -19,7 +19,8 @@ package testing
import (
"fmt"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)
Expand Down Expand Up @@ -362,7 +363,8 @@ type NodeWrapper struct{ v1.Node }

// MakeNode creates a Node wrapper.
func MakeNode() *NodeWrapper {
return &NodeWrapper{v1.Node{}}
w := &NodeWrapper{v1.Node{}}
return w.Capacity(nil)
}

// Obj returns the inner Node.
Expand Down Expand Up @@ -390,3 +392,28 @@ func (n *NodeWrapper) Label(k, v string) *NodeWrapper {
n.Labels[k] = v
return n
}

// Capacity sets the capacity and the allocatable resources of the inner node.
// Each entry in `resources` corresponds to a resource name and its quantity.
Copy link
Member

Choose a reason for hiding this comment

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

Indicate that sets 32 pods limit by default

// By default, the capacity and allocatable number of pods are set to 32.
func (n *NodeWrapper) Capacity(resources map[v1.ResourceName]string) *NodeWrapper {
res := v1.ResourceList{
v1.ResourcePods: resource.MustParse("32"),
}
for name, value := range resources {
res[name] = resource.MustParse(value)
}
n.Status.Capacity, n.Status.Allocatable = res, res
Copy link
Member

@Huang-Wei Huang-Wei Jun 29, 2020

Choose a reason for hiding this comment

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

would be good to ensure v1.ResourcePods is always present, otherwise some pod scheduling may fail due to "no Pods quota".

FYI: I put it in https://github.com/kubernetes/kubernetes/pull/92571/files?file-filters%5B%5D=.go#diff-1cea28cd0be3cdbab57f5dc287dc98c0R427-R438

return n
}

// Images sets the images of the inner node. Each entry in `images` corresponds
// to an image name and its size in bytes.
func (n *NodeWrapper) Images(images map[string]int64) *NodeWrapper {
var containerImages []v1.ContainerImage
for name, size := range images {
containerImages = append(containerImages, v1.ContainerImage{Names: []string{name}, SizeBytes: size})
}
n.Status.Images = containerImages
return n
}
2 changes: 1 addition & 1 deletion test/integration/scheduler/BUILD
Expand Up @@ -86,10 +86,10 @@ go_library(
"//pkg/api/v1/pod:go_default_library",
"//pkg/controller/disruption:go_default_library",
"//pkg/scheduler:go_default_library",
"//pkg/scheduler/testing:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
Expand Down
17 changes: 9 additions & 8 deletions test/integration/scheduler/framework_test.go
Expand Up @@ -22,7 +22,7 @@ import (
"testing"
"time"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -33,6 +33,7 @@ import (
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
frameworkruntime "k8s.io/kubernetes/pkg/scheduler/framework/runtime"
framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1"
st "k8s.io/kubernetes/pkg/scheduler/testing"
testutils "k8s.io/kubernetes/test/integration/util"
)

Expand Down Expand Up @@ -1130,7 +1131,7 @@ func TestBindPlugin(t *testing.T) {
defer testutils.CleanupTest(t, testCtx)

// Add a few nodes.
_, err := createNodes(testCtx.ClientSet, "test-node", nil, 2)
_, err := createNodes(testCtx.ClientSet, "test-node", st.MakeNode(), 2)
if err != nil {
t.Fatalf("Cannot create nodes: %v", err)
}
Expand Down Expand Up @@ -1776,12 +1777,12 @@ func TestPreemptWithPermitPlugin(t *testing.T) {
defer testutils.CleanupTest(t, testCtx)

// Add one node.
nodeRes := &v1.ResourceList{
v1.ResourcePods: *resource.NewQuantity(32, resource.DecimalSI),
v1.ResourceCPU: *resource.NewMilliQuantity(500, resource.DecimalSI),
v1.ResourceMemory: *resource.NewQuantity(500, resource.DecimalSI),
nodeRes := map[v1.ResourceName]string{
v1.ResourcePods: "32",
v1.ResourceCPU: "500m",
v1.ResourceMemory: "500",
}
_, err := createNodes(testCtx.ClientSet, "test-node", nodeRes, 1)
_, err := createNodes(testCtx.ClientSet, "test-node", st.MakeNode().Capacity(nodeRes), 1)
if err != nil {
t.Fatalf("Cannot create nodes: %v", err)
}
Expand Down Expand Up @@ -1841,7 +1842,7 @@ func initTestSchedulerForFrameworkTest(t *testing.T, testCtx *testutils.TestCont
go testCtx.Scheduler.Run(testCtx.Ctx)

if nodeCount > 0 {
_, err := createNodes(testCtx.ClientSet, "test-node", nil, nodeCount)
_, err := createNodes(testCtx.ClientSet, "test-node", st.MakeNode(), nodeCount)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't MakeNode call Capacity(nil) or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally! My bad, let me fix it.

if err != nil {
t.Fatalf("Cannot create nodes: %v", err)
}
Expand Down
45 changes: 10 additions & 35 deletions test/integration/scheduler/predicates_test.go
Expand Up @@ -24,7 +24,6 @@ import (

v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
Expand All @@ -43,25 +42,12 @@ const pollInterval = 100 * time.Millisecond
func TestInterPodAffinity(t *testing.T) {
testCtx := initTest(t, "inter-pod-affinity")
defer testutils.CleanupTest(t, testCtx)
// Add a few nodes.
nodes, err := createNodes(testCtx.ClientSet, "testnode", nil, 2)

// Add a few nodes with labels
nodes, err := createNodes(testCtx.ClientSet, "testnode", st.MakeNode().Label("region", "r1").Label("zone", "z11"), 2)
if err != nil {
t.Fatalf("Cannot create nodes: %v", err)
}
// Add labels to the nodes.
labels1 := map[string]string{
"region": "r1",
"zone": "z11",
}
for _, node := range nodes {
// TODO(nodo): Use PodWrapper to directly initialize node with labels.
if err = utils.AddLabelsToNode(testCtx.ClientSet, node.Name, labels1); err != nil {
t.Fatalf("Cannot add labels to node: %v", err)
}
if err = waitForNodeLabels(testCtx.ClientSet, node.Name, labels1); err != nil {
t.Fatalf("Adding labels to node didn't succeed: %v", err)
}
}

cs := testCtx.ClientSet
podLabel := map[string]string{"service": "securityscan"}
Expand Down Expand Up @@ -886,7 +872,7 @@ func TestEvenPodsSpreadPredicate(t *testing.T) {
ns := testCtx.NS.Name
defer testutils.CleanupTest(t, testCtx)
// Add 4 nodes.
nodes, err := createNodes(cs, "node", nil, 4)
nodes, err := createNodes(cs, "node", st.MakeNode(), 4)
if err != nil {
t.Fatalf("Cannot create nodes: %v", err)
}
Expand All @@ -896,7 +882,6 @@ func TestEvenPodsSpreadPredicate(t *testing.T) {
"zone": fmt.Sprintf("zone-%d", i/2),
"node": node.Name,
}
// TODO(nodo): Use PodWrapper to directly initialize node with labels.
if err = utils.AddLabelsToNode(cs, node.Name, labels); err != nil {
t.Fatalf("Cannot add labels to node: %v", err)
}
Expand Down Expand Up @@ -1051,7 +1036,7 @@ func TestUnschedulablePodBecomesSchedulable(t *testing.T) {
Name: "pod-1",
},
update: func(cs kubernetes.Interface, _ string) error {
_, err := createNode(cs, "node-added", nil)
_, err := createNode(cs, st.MakeNode().Name("node-added").Obj())
if err != nil {
return fmt.Errorf("cannot create node: %v", err)
}
Expand All @@ -1061,7 +1046,7 @@ func TestUnschedulablePodBecomesSchedulable(t *testing.T) {
{
name: "node gets taint removed",
init: func(cs kubernetes.Interface, _ string) error {
node, err := createNode(cs, "node-tainted", nil)
node, err := createNode(cs, st.MakeNode().Name("node-tainted").Obj())
if err != nil {
return fmt.Errorf("cannot create node: %v", err)
}
Expand All @@ -1085,10 +1070,8 @@ func TestUnschedulablePodBecomesSchedulable(t *testing.T) {
{
name: "other pod gets deleted",
init: func(cs kubernetes.Interface, ns string) error {
nodeResources := &v1.ResourceList{
v1.ResourcePods: *resource.NewQuantity(1, resource.DecimalSI),
}
_, err := createNode(cs, "node-scheduler-integration-test", nodeResources)
nodeObject := st.MakeNode().Name("node-scheduler-integration-test").Capacity(map[v1.ResourceName]string{v1.ResourcePods: "1"}).Obj()
_, err := createNode(cs, nodeObject)
if err != nil {
return fmt.Errorf("cannot create node: %v", err)
}
Expand All @@ -1111,14 +1094,10 @@ func TestUnschedulablePodBecomesSchedulable(t *testing.T) {
{
name: "pod with pod-affinity gets added",
init: func(cs kubernetes.Interface, _ string) error {
node, err := createNode(cs, "node-1", nil)
_, err := createNode(cs, st.MakeNode().Name("node-1").Label("region", "test").Obj())
if err != nil {
return fmt.Errorf("cannot create node: %v", err)
}
// TODO(nodo): Use PodWrapper to directly initialize node with labels.
if err := utils.AddLabelsToNode(cs, node.Name, map[string]string{"region": "test"}); err != nil {
return fmt.Errorf("cannot add labels to node: %v", err)
}
return nil
},
pod: &pausePodConfig{
Expand Down Expand Up @@ -1155,14 +1134,10 @@ func TestUnschedulablePodBecomesSchedulable(t *testing.T) {
{
name: "scheduled pod gets updated to match affinity",
init: func(cs kubernetes.Interface, ns string) error {
node, err := createNode(cs, "node-1", nil)
_, err := createNode(cs, st.MakeNode().Name("node-1").Label("region", "test").Obj())
if err != nil {
return fmt.Errorf("cannot create node: %v", err)
}
// TODO(nodo): Use PodWrapper to directly initialize node with labels.
if err := utils.AddLabelsToNode(cs, node.Name, map[string]string{"region": "test"}); err != nil {
return fmt.Errorf("cannot add labels to node: %v", err)
}
if _, err := createPausePod(cs, initPausePod(&pausePodConfig{Name: "pod-to-be-updated", Namespace: ns})); err != nil {
return fmt.Errorf("cannot create pod: %v", err)
}
Expand Down