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

Refactor scheduler factory test #69412

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
2 changes: 1 addition & 1 deletion pkg/kubectl/polymorphichelpers/logsforobject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func getLogsAction(namespace string, opts *corev1.PodLogOptions) testclient.Acti
action.Verb = "get"
action.Namespace = namespace
action.Resource = podsResource
action.Subresource = "logs"
action.Subresource = "log"
action.Value = opts
return action
}
6 changes: 3 additions & 3 deletions pkg/scheduler/factory/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ go_test(
"//staging/src/k8s.io/api/core/v1: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/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/rest:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/core/v1/fake:go_default_library",
"//staging/src/k8s.io/client-go/testing:go_default_library",
"//staging/src/k8s.io/client-go/tools/cache:go_default_library",
"//staging/src/k8s.io/client-go/util/testing:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
],
)
Expand Down
150 changes: 56 additions & 94 deletions pkg/scheduler/factory/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,20 @@ package factory
import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"testing"
"time"

"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/informers"
clientset "k8s.io/client-go/kubernetes"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/kubernetes/fake"
fakeV1 "k8s.io/client-go/kubernetes/typed/core/v1/fake"
clienttesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache"
utiltesting "k8s.io/client-go/util/testing"
apitesting "k8s.io/kubernetes/pkg/api/testing"
"k8s.io/kubernetes/pkg/scheduler/algorithm"
schedulerapi "k8s.io/kubernetes/pkg/scheduler/api"
Expand All @@ -53,14 +51,7 @@ const (
)

func TestCreate(t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)
factory.Create()
}
Expand All @@ -71,14 +62,7 @@ func TestCreateFromConfig(t *testing.T) {
var configData []byte
var policy schedulerapi.Policy

handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)

// Pre-register some predicate and priority functions
Expand Down Expand Up @@ -116,14 +100,7 @@ func TestCreateFromConfigWithHardPodAffinitySymmetricWeight(t *testing.T) {
var configData []byte
var policy schedulerapi.Policy

handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)

// Pre-register some predicate and priority functions
Expand Down Expand Up @@ -162,14 +139,7 @@ func TestCreateFromEmptyConfig(t *testing.T) {
var configData []byte
var policy schedulerapi.Policy

handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)

configData = []byte(`{}`)
Expand All @@ -184,14 +154,7 @@ func TestCreateFromEmptyConfig(t *testing.T) {
// predicate/priority.
// The predicate/priority from DefaultProvider will be used.
func TestCreateFromConfigWithUnspecifiedPredicatesOrPriorities(t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)

RegisterFitPredicate("PredicateOne", PredicateOne)
Expand Down Expand Up @@ -224,14 +187,7 @@ func TestCreateFromConfigWithUnspecifiedPredicatesOrPriorities(t *testing.T) {
// predicate/priority.
// Empty predicate/priority sets will be used.
func TestCreateFromConfigWithEmptyPredicatesOrPriorities(t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)

RegisterFitPredicate("PredicateOne", PredicateOne)
Expand Down Expand Up @@ -283,24 +239,14 @@ func TestDefaultErrorFunc(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
Spec: apitesting.V1DeepEqualSafePodSpec(),
}
handler := utiltesting.FakeHandler{
StatusCode: 200,
ResponseBody: runtime.EncodeOrDie(schedulertesting.Test.Codec(), testPod),
T: t,
}
mux := http.NewServeMux()

// FakeHandler mustn't be sent requests other than the one you want to test.
mux.Handle(schedulertesting.Test.ResourcePath(string(v1.ResourcePods), "bar", "foo"), &handler)
server := httptest.NewServer(mux)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testPod}})
factory := newConfigFactory(client, v1.DefaultHardPodAffinitySymmetricWeight)
queue := &internalqueue.FIFO{FIFO: cache.NewFIFO(cache.MetaNamespaceKeyFunc)}
podBackoff := util.CreatePodBackoff(1*time.Millisecond, 1*time.Second)
errFunc := factory.MakeDefaultErrorFunc(podBackoff, queue)

errFunc(testPod, nil)

for {
// This is a terrible way to do this but I plan on replacing this
// whole error handling system in the future. The test will time
Expand All @@ -310,7 +256,27 @@ func TestDefaultErrorFunc(t *testing.T) {
if !exists {
continue
}
handler.ValidateRequest(t, schedulertesting.Test.ResourcePath(string(v1.ResourcePods), "bar", "foo"), "GET", nil)
requestReceived := false
actions := client.Actions()
for _, a := range actions {

Choose a reason for hiding this comment

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

Thanks for updating this. It is easier to read (to me) than the reactor stuff. I almost wonder if this test should assert len(actions)==1 since the test fails otherwise.

if a.GetVerb() == "get" {
getAction, ok := a.(clienttesting.GetAction)
if !ok {
t.Errorf("Can't cast action object to GetAction interface")
break
}
name := getAction.GetName()
ns := a.GetNamespace()
if name != "foo" || ns != "bar" {
t.Errorf("Expected name %s namespace %s, got %s %s",
"foo", "bar", name, ns)
}
requestReceived = true
}
}
if !requestReceived {
t.Errorf("Get pod request not received")
}
if e, a := testPod, got; !reflect.DeepEqual(e, a) {
t.Errorf("Expected %v, got %v", e, a)
}
Expand Down Expand Up @@ -371,35 +337,38 @@ func TestBind(t *testing.T) {
}

func testBind(binding *v1.Binding, t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 200,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
testPod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: binding.GetName(), Namespace: metav1.NamespaceDefault},
Spec: apitesting.V1DeepEqualSafePodSpec(),
}
client := fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testPod}})

b := binder{client}

if err := b.Bind(binding); err != nil {
t.Errorf("Unexpected error: %v", err)
return
}

pod := client.CoreV1().Pods(metav1.NamespaceDefault).(*fakeV1.FakePods)

bind, err := pod.GetBinding(binding.GetName())
if err != nil {
t.Fatalf("Unexpected error: %v", err)
return
}

expectedBody := runtime.EncodeOrDie(schedulertesting.Test.Codec(), binding)
handler.ValidateRequest(t,
schedulertesting.Test.SubResourcePath(string(v1.ResourcePods), metav1.NamespaceDefault, "foo", "binding"),
"POST", &expectedBody)
bind.APIVersion = ""
bind.Kind = ""
body := runtime.EncodeOrDie(schedulertesting.Test.Codec(), bind)
if expectedBody != body {
t.Errorf("Expected body %s, Got %s", expectedBody, body)
}
}

func TestInvalidHardPodAffinitySymmetricWeight(t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()
// factory of "default-scheduler"
factory := newConfigFactory(client, -1)
_, err := factory.Create()
Expand All @@ -409,14 +378,7 @@ func TestInvalidHardPodAffinitySymmetricWeight(t *testing.T) {
}

func TestInvalidFactoryArgs(t *testing.T) {
handler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "",
T: t,
}
server := httptest.NewServer(&handler)
defer server.Close()
client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}})
client := fake.NewSimpleClientset()

testCases := []struct {
name string
Expand Down Expand Up @@ -539,7 +501,7 @@ func TestSkipPodUpdate(t *testing.T) {
}
}

func newConfigFactory(client *clientset.Clientset, hardPodAffinitySymmetricWeight int32) Configurator {
func newConfigFactory(client clientset.Interface, hardPodAffinitySymmetricWeight int32) Configurator {
informerFactory := informers.NewSharedInformerFactory(client, 0)
return NewConfigFactory(&ConfigFactoryArgs{
v1.DefaultSchedulerName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,31 @@ import (
func (c *FakePods) Bind(binding *v1.Binding) error {
action := core.CreateActionImpl{}
action.Verb = "create"
action.Namespace = binding.Namespace
Copy link
Member

Choose a reason for hiding this comment

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

is name needed as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

@liggitt The action is create, so the name is not required in action and already in the binding object.

action.Resource = podsResource
action.Subresource = "bindings"
action.Subresource = "binding"
action.Object = binding

_, err := c.Fake.Invokes(action, binding)
return err
}

func (c *FakePods) GetBinding(name string) (result *v1.Binding, err error) {
obj, err := c.Fake.
Invokes(core.NewGetSubresourceAction(podsResource, c.ns, "binding", name), &v1.Binding{})

if obj == nil {
return nil, err
}
return obj.(*v1.Binding), err
}

func (c *FakePods) GetLogs(name string, opts *v1.PodLogOptions) *restclient.Request {
action := core.GenericActionImpl{}
action.Verb = "get"
action.Namespace = c.ns
action.Resource = podsResource
action.Subresource = "logs"
action.Subresource = "log"
action.Value = opts

_, _ = c.Fake.Invokes(action, &v1.Pod{})
Expand Down