Skip to content

Commit

Permalink
Rewrite Configuration tests to call syncHandler directly (knative#401)
Browse files Browse the repository at this point in the history 
* Rewrite test to call syncHandler directly

This test is more reliable when the controller and informers are not
running, and the test calls syncHandler when necessary.

We still need hooks to test events since they're always delivered
asynchronously.

* Correct polarity of test conditions

* Convert remaining tests to use stopped controllers

Calling syncHandler directly is currently more reliable than a running
controller and informer.

* Add test of running controller logic

The file queueing_test.go is for testing running controllers, versus
controller_test.go which tests non-running controllers.

Because of the issues with watches in client-go 1.9, the running
controller tests must insert all objects into the client in advance to
avoid racy failures. When we upgrade to 1.10 these tests can more
naturally call Create() without races.

* Add missing comments explaining Indexer Add
  • Loading branch information
@grantr
grantr authored Mar 16, 2018
1 parent e695fa7 commit 4ae254d
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 117 deletions.
6 changes: 4 additions & 2 deletions pkg/controller/configuration/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ go_library(

go_test(
name = "go_default_test",
srcs = ["controller_test.go"],
srcs = [
"controller_test.go",
"queueing_test.go",
],
embed = [":go_default_library"],
importpath = "github.com/elafros/elafros/pkg/controller/configuration",
deps = [
Expand All @@ -43,7 +46,6 @@ go_test(
"//pkg/client/clientset/versioned/fake:go_default_library",
"//pkg/client/informers/externalversions:go_default_library",
"//pkg/controller/testing:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand Down
253 changes: 138 additions & 115 deletions pkg/controller/configuration/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@ package configuration
Congfiguration.
*/
import (
"reflect"
"regexp"
"strings"
"testing"
"time"

"github.com/golang/glog"
"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -112,7 +111,7 @@ func getTestRevision() *v1alpha1.Revision {
}
}

func newTestController(t *testing.T) (
func newTestController(t *testing.T, elaObjects ...runtime.Object) (
kubeClient *fakekubeclientset.Clientset,
elaClient *fakeclientset.Clientset,
controller *Controller,
Expand All @@ -121,7 +120,10 @@ func newTestController(t *testing.T) (

// Create fake clients
kubeClient = fakekubeclientset.NewSimpleClientset()
elaClient = fakeclientset.NewSimpleClientset()
// The ability to insert objects here is intended to work around the problem
// with watches not firing in client-go 1.9. When we update to client-go 1.10
// this can probably be removed.
elaClient = fakeclientset.NewSimpleClientset(elaObjects...)

// Create informer factories with fake clients. The second parameter sets the
// resync period to zero, disabling it.
Expand All @@ -139,15 +141,15 @@ func newTestController(t *testing.T) (
return
}

func newRunningTestController(t *testing.T) (
func newRunningTestController(t *testing.T, elaObjects ...runtime.Object) (
kubeClient *fakekubeclientset.Clientset,
elaClient *fakeclientset.Clientset,
controller *Controller,
kubeInformer kubeinformers.SharedInformerFactory,
elaInformer informers.SharedInformerFactory,
stopCh chan struct{}) {

kubeClient, elaClient, controller, kubeInformer, elaInformer = newTestController(t)
kubeClient, elaClient, controller, kubeInformer, elaInformer = newTestController(t, elaObjects...)

// Start the informers. This must happen after the call to NewController,
// otherwise there are no informers to be started.
Expand All @@ -174,30 +176,12 @@ func keyOrDie(obj interface{}) string {
}

func TestCreateConfigurationsCreatesRevision(t *testing.T) {
kubeClient, elaClient, _, _, _, stopCh := newRunningTestController(t)
defer close(stopCh)
kubeClient, elaClient, controller, _, elaInformer := newTestController(t)
config := getTestConfiguration()
h := hooks.NewHooks()

h.OnCreate(&elaClient.Fake, "revisions", func(obj runtime.Object) hooks.HookResult {
rev := obj.(*v1alpha1.Revision)
glog.Infof("checking revision %s", rev.Name)
if diff := cmp.Diff(config.Spec.Template.Spec, rev.Spec); diff != "" {
t.Errorf("rev spec != config template spec (-want +got): %v", diff)
}

if rev.Labels[ela.ConfigurationLabelKey] != config.Name {
t.Errorf("rev does not have label <%s:%s>", ela.ConfigurationLabelKey, config.Name)
}

if len(rev.OwnerReferences) != 1 || config.Name != rev.OwnerReferences[0].Name {
t.Errorf("expected owner references to have 1 ref with name %s", config.Name)
}

return hooks.HookComplete
})

// Look for the events.
// Look for the event. Events are delivered asynchronously so we need to use
// hooks here.
h.OnCreate(&kubeClient.Fake, "events", func(obj runtime.Object) hooks.HookResult {
event := obj.(*corev1.Event)
expectedMessages := "Created Revision "
Expand All @@ -211,18 +195,41 @@ func TestCreateConfigurationsCreatesRevision(t *testing.T) {
})

elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config)
// Since syncHandler looks in the lister, we need to add it to the informer
elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(config)
controller.syncHandler(keyOrDie(config))

list, err := elaClient.ElafrosV1alpha1().Revisions(testNamespace).List(metav1.ListOptions{})

if err != nil {
t.Fatalf("error listing revisions: %v", err)
}

if got, want := len(list.Items), 1; got != want {
t.Fatalf("expected %v revisions, got %v", want, got)
}

rev := list.Items[0]
if diff := cmp.Diff(config.Spec.Template.Spec, rev.Spec); diff != "" {
t.Errorf("rev spec != config template spec (-want +got): %v", diff)
}

if rev.Labels[ela.ConfigurationLabelKey] != config.Name {
t.Errorf("rev does not have label <%s:%s>", ela.ConfigurationLabelKey, config.Name)
}

if len(rev.OwnerReferences) != 1 || config.Name != rev.OwnerReferences[0].Name {
t.Errorf("expected owner references to have 1 ref with name %s", config.Name)
}

if err := h.WaitForHooks(time.Second * 3); err != nil {
t.Error(err)
}
}

func TestCreateConfigurationCreatesBuildAndPR(t *testing.T) {
_, elaClient, _, _, _, stopCh := newRunningTestController(t)
defer close(stopCh)
func TestCreateConfigurationCreatesBuildAndRevision(t *testing.T) {
_, elaClient, controller, _, elaInformer := newTestController(t)
config := getTestConfiguration()
h := hooks.NewHooks()

config.Spec.Build = &buildv1alpha1.BuildSpec{
Steps: []corev1.Container{{
Name: "nop",
Expand All @@ -232,112 +239,128 @@ func TestCreateConfigurationCreatesBuildAndPR(t *testing.T) {
}},
}

h.OnCreate(&elaClient.Fake, "builds", func(obj runtime.Object) hooks.HookResult {
b := obj.(*buildv1alpha1.Build)
glog.Infof("checking build %s", b.Name)
if config.Spec.Build.Steps[0].Name != b.Spec.Steps[0].Name {
t.Errorf("BuildSpec mismatch; want %v, got %v", config.Spec.Build.Steps[0], b.Spec.Steps[0])
}
return hooks.HookComplete
})
elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config)
// Since syncHandler looks in the lister, we need to add it to the informer
elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(config)
controller.syncHandler(keyOrDie(config))

h.OnCreate(&elaClient.Fake, "revisions", func(obj runtime.Object) hooks.HookResult {
rev := obj.(*v1alpha1.Revision)
glog.Infof("checking revision %s", rev.Name)
// TODO(mattmoor): The fake doesn't properly support GenerateName,
// so it never looks like the BuildName is populated.
// if rev.Spec.BuildName == "" {
// t.Error("Missing BuildName; want non-empty, but got empty string")
// }
return hooks.HookComplete
})
revList, err := elaClient.ElafrosV1alpha1().Revisions(testNamespace).List(metav1.ListOptions{})
if err != nil {
t.Fatalf("error listing revisions: %v", err)
}
if got, want := len(revList.Items), 1; got != want {
t.Fatalf("expected %v revisions, got %v", want, got)
}

elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config)
buildList, err := elaClient.BuildV1alpha1().Builds(testNamespace).List(metav1.ListOptions{})
if err != nil {
t.Fatalf("error listing builds: %v", err)
}
if got, want := len(buildList.Items), 1; got != want {
t.Fatalf("expected %v builds, got %v", want, got)
}

if err := h.WaitForHooks(time.Second * 3); err != nil {
t.Error(err)
b := buildList.Items[0]
if diff := cmp.Diff(config.Spec.Build.Steps, b.Spec.Steps); diff != "" {
t.Errorf("Unexpected build steps diff (-want +got): %v", diff)
}
}

func TestMarkConfigurationReadyWhenLatestRevisionReady(t *testing.T) {
kubeClient, elaClient, controller, _, _, stopCh := newRunningTestController(t)
defer close(stopCh)
kubeClient, elaClient, controller, _, elaInformer := newTestController(t)
configClient := elaClient.ElafrosV1alpha1().Configurations(testNamespace)

config := getTestConfiguration()
config.Status.LatestCreatedRevisionName = revName
h := hooks.NewHooks()

update := 0
h.OnUpdate(&elaClient.Fake, "configurations", func(obj runtime.Object) hooks.HookResult {
config := obj.(*v1alpha1.Configuration)
update = update + 1
switch update {
case 1:
// This update is triggered by the configuration creation.
if got, want := len(config.Status.Conditions), 0; !reflect.DeepEqual(got, want) {
t.Errorf("Conditions length diff; got %v, want %v", got, want)
}
if got, want := config.Status.LatestReadyRevisionName, ""; got != want {
t.Errorf("Latest in Status diff; got %v, want %v", got, want)
}
controllerRef := metav1.NewControllerRef(config, controllerKind)
revision := getTestRevision()
revision.OwnerReferences = append(revision.OwnerReferences, *controllerRef)
revision.Status = v1alpha1.RevisionStatus{
Conditions: []v1alpha1.RevisionCondition{{
Type: v1alpha1.RevisionConditionReady,
Status: corev1.ConditionTrue,
}},
}
// This call should update configration.
go controller.addRevisionEvent(revision)
// Events are delivered asynchronously so we need to use hooks here. Each hook
// tests for a specific event.
h := hooks.NewHooks()
h.OnCreate(&kubeClient.Fake, "events", func(obj runtime.Object) hooks.HookResult {
event := obj.(*corev1.Event)
want := "Configuration becomes ready"
if event.Message != want {
return hooks.HookIncomplete
case 2:
// The next update we receive should tell us that the revision is ready.
expectedConfigConditions := []v1alpha1.ConfigurationCondition{
v1alpha1.ConfigurationCondition{
Type: v1alpha1.ConfigurationConditionReady,
Status: corev1.ConditionTrue,
Reason: "LatestRevisionReady",
},
}
if got, want := config.Status.Conditions, expectedConfigConditions; !reflect.DeepEqual(got, want) {
t.Errorf("Conditions diff; got %v, want %v", got, want)
}
if got, want := config.Status.LatestReadyRevisionName, revName; got != want {
t.Errorf("Latest in Status diff; got %v, want %v", got, want)
}
}
t.Logf("Got an event matching %q", want)
if got, want := event.Type, corev1.EventTypeNormal; got != want {
t.Errorf("unexpected event Type: %q expected: %q", got, want)
}
return hooks.HookComplete
})

// Look for the event
expectedMessages := map[string]struct{}{
"Configuration becomes ready": struct{}{},
"LatestReadyRevisionName updated to \"test-rev\"": struct{}{},
}
eventNum := 0
h.OnCreate(&kubeClient.Fake, "events", func(obj runtime.Object) hooks.HookResult {
event := obj.(*corev1.Event)
eventNum = eventNum + 1
if strings.HasPrefix(event.Message, "Created Revision ") {
// The first revision created event.
want := regexp.MustCompile("LatestReadyRevisionName updated to .+")
if !want.MatchString(event.Message) {
return hooks.HookIncomplete
}
if _, ok := expectedMessages[event.Message]; !ok {
t.Errorf("unexpected Message: %q expected one of: %q", event.Message, expectedMessages)
}
if wanted, got := corev1.EventTypeNormal, event.Type; wanted != got {
t.Errorf("unexpected event Type: %q expected: %q", got, wanted)
}
// Expect 3 events.
if eventNum < 3 {
return hooks.HookIncomplete
t.Logf("Got an event matching %v", want)
if got, want := event.Type, corev1.EventTypeNormal; got != want {
t.Errorf("unexpected event Type: %q expected: %q", got, want)
}
return hooks.HookComplete
})

elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config)
configClient.Create(config)
// Since syncHandler looks in the lister, we need to add it to the informer
elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(config)
controller.syncHandler(keyOrDie(config))

reconciledConfig, err := configClient.Get(config.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Couldn't get config: %v", err)
}

// Config should not have any conditions after reconcile
if got, want := len(reconciledConfig.Status.Conditions), 0; got != want {
t.Errorf("Conditions length diff; got %v, want %v", got, want)
}
// Config should not have a latest ready revision
if got, want := reconciledConfig.Status.LatestReadyRevisionName, ""; got != want {
t.Errorf("Latest in Status diff; got %v, want %v", got, want)
}

// Get the revision created
revList, err := elaClient.ElafrosV1alpha1().Revisions(config.Namespace).List(metav1.ListOptions{})
if err != nil {
t.Fatalf("error listing revisions: %v", err)
}
if got, want := len(revList.Items), 1; got != want {
t.Fatalf("expected %d revisions, got %d", want, got)
}
revision := revList.Items[0]

// mark the revision as Ready
revision.Status = v1alpha1.RevisionStatus{
Conditions: []v1alpha1.RevisionCondition{{
Type: v1alpha1.RevisionConditionReady,
Status: corev1.ConditionTrue,
}},
}
// Since addRevisionEvent looks in the lister, we need to add it to the informer
elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(reconciledConfig)
controller.addRevisionEvent(&revision)

readyConfig, err := configClient.Get(config.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("Couldn't get config: %v", err)
}

expectedConfigConditions := []v1alpha1.ConfigurationCondition{
v1alpha1.ConfigurationCondition{
Type: v1alpha1.ConfigurationConditionReady,
Status: corev1.ConditionTrue,
Reason: "LatestRevisionReady",
},
}
if diff := cmp.Diff(expectedConfigConditions, readyConfig.Status.Conditions); diff != "" {
t.Errorf("Unexpected config conditions diff (-want +got): %v", diff)
}
if got, want := readyConfig.Status.LatestReadyRevisionName, revision.Name; got != want {
t.Errorf("Latest in Status diff; got %v, want %v", got, want)
}

// wait for events to be created
if err := h.WaitForHooks(time.Second * 3); err != nil {
t.Error(err)
}
Expand Down
Loading

0 comments on commit 4ae254d

Please sign in to comment.