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

[Mitigate KCM CrashLooping] Add unittests for controllers' Init func #73337

Merged
merged 1 commit into from
Feb 4, 2019
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
5 changes: 5 additions & 0 deletions cmd/kube-controller-manager/app/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
cacheddiscovery "k8s.io/client-go/discovery/cached"
"k8s.io/client-go/dynamic"
clientset "k8s.io/client-go/kubernetes"
restclient "k8s.io/client-go/rest"
csiclientset "k8s.io/csi-api/pkg/client/clientset/versioned"
"k8s.io/kubernetes/pkg/controller"
cloudcontroller "k8s.io/kubernetes/pkg/controller/cloud"
Expand Down Expand Up @@ -331,6 +332,10 @@ func startNamespaceController(ctx ControllerContext) (http.Handler, bool, error)
nsKubeconfig.QPS *= 20
nsKubeconfig.Burst *= 100
namespaceKubeClient := clientset.NewForConfigOrDie(nsKubeconfig)
return startModifiedNamespaceController(ctx, namespaceKubeClient, nsKubeconfig)
}

func startModifiedNamespaceController(ctx ControllerContext, namespaceKubeClient clientset.Interface, nsKubeconfig *restclient.Config) (http.Handler, bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This name isn't obvious to me, but if Walter liked it I won't nit it to death. Next time something more obvious? If you removed a context dependency for instance it may make it more obvious. Based on where you split it, I'm guessing you didn't want to deal with the client built from config because you can't mock that.


dynamicClient, err := dynamic.NewForConfig(nsKubeconfig)
if err != nil {
Expand Down
68 changes: 47 additions & 21 deletions cmd/kube-controller-manager/app/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package app

import (
"net/http"
"testing"
"time"

Expand All @@ -34,8 +35,11 @@ type TestClientBuilder struct {
clientset clientset.Interface
}

func (TestClientBuilder) Config(name string) (*restclient.Config, error) { return nil, nil }
func (TestClientBuilder) ConfigOrDie(name string) *restclient.Config { return nil }
func (TestClientBuilder) Config(name string) (*restclient.Config, error) { return nil, nil }
func (TestClientBuilder) ConfigOrDie(name string) *restclient.Config {
return &restclient.Config{}
}

func (TestClientBuilder) Client(name string) (clientset.Interface, error) { return nil, nil }
func (m TestClientBuilder) ClientOrDie(name string) clientset.Interface {
return m.clientset
Expand All @@ -62,40 +66,55 @@ func (c *FakeClientSet) Discovery() discovery.DiscoveryInterface {
return c.DiscoveryObj
}

func (c *FakeClientSet) GetPossibleResources() []*metav1.APIResourceList {
return c.DiscoveryObj.PossibleResources
}

// Create a fake Clientset with its Discovery method overridden.
func NewFakeClientset(fakeDiscovery FakeDiscoveryWithError) *FakeClientSet {
cs := &FakeClientSet{}
cs.DiscoveryObj = &fakeDiscovery
return cs
}

func TestStartResourceQuotaController_DiscoveryError(t *testing.T) {
func possibleDiscoveryResource() []*metav1.APIResourceList {
return []*metav1.APIResourceList{
{
GroupVersion: "create/v1",
APIResources: []metav1.APIResource{
{
Name: "jobs",
Verbs: []string{"create", "list", "watch", "delete"},
ShortNames: []string{"jz"},
Categories: []string{"all"},
},
},
},
}
}

type controllerInitFunc func(ControllerContext) (http.Handler, bool, error)

func TestController_DiscoveryError(t *testing.T) {
controllerInitFuncMap := map[string]controllerInitFunc{
"ResourceQuotaController": startResourceQuotaController,
"GarbageCollectorController": startGarbageCollectorController,
}

tcs := map[string]struct {
discoveryError error
expectedErr bool
possibleResources []*metav1.APIResourceList
}{
"No Discovery Error": {
discoveryError: nil,
possibleResources: nil,
possibleResources: possibleDiscoveryResource(),
expectedErr: false,
},
"Discovery Calls Partially Failed": {
discoveryError: new(discovery.ErrGroupDiscoveryFailed),
possibleResources: []*metav1.APIResourceList{
{
GroupVersion: "create/v1",
APIResources: []metav1.APIResource{
{
Name: "jobs",
Verbs: []string{"create", "list", "watch", "delete"},
ShortNames: []string{"jz"},
Categories: []string{"all"},
},
},
},
},
expectedErr: false,
discoveryError: new(discovery.ErrGroupDiscoveryFailed),
possibleResources: possibleDiscoveryResource(),
expectedErr: false,
},
}
for name, test := range tcs {
Expand All @@ -107,9 +126,16 @@ func TestStartResourceQuotaController_DiscoveryError(t *testing.T) {
InformerFactory: informers.NewSharedInformerFactoryWithOptions(testClientset, time.Duration(1)),
InformersStarted: make(chan struct{}),
}
_, _, err := startResourceQuotaController(ctx)
for funcName, controllerInit := range controllerInitFuncMap {
_, _, err := controllerInit(ctx)
if test.expectedErr != (err != nil) {
t.Errorf("%v test failed for use case: %v", funcName, name)
}
}
_, _, err := startModifiedNamespaceController(
ctx, testClientset, testClientBuilder.ConfigOrDie("namespace-controller"))
if test.expectedErr != (err != nil) {
t.Errorf("test failed for use case: %v", name)
t.Errorf("Namespace Controller test failed for use case: %v", name)
}
}
}