Skip to content

Commit

Permalink
make testclient threadsafe by guarding internal state with accessors
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikedanese
mikedanese committed Jul 29, 2015
1 parent 59611d7 commit 1b84fb7
Show file tree
Hide file tree
Showing 28 changed files with 151 additions and 99 deletions.
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func (c *FakeEndpoints) Delete(name string) error {
}

func (c *FakeEndpoints) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-endpoints", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-endpoints", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}

func (c *FakeEndpoints) Update(endpoints *api.Endpoints) (*api.Endpoints, error) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/testclient/fake_events.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func (c *FakeEvents) Get(id string) (*api.Event, error) {

// Watch starts watching for events matching the given selectors.
func (c *FakeEvents) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-events", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-events", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}

// Search returns a list of events matching the specified object.
Expand All @@ -72,6 +72,6 @@ func (c *FakeEvents) Delete(name string) error {
}

func (c *FakeEvents) GetFieldSelector(involvedObjectName, involvedObjectNamespace, involvedObjectKind, involvedObjectUID *string) fields.Selector {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "get-field-selector"})
c.Fake.Invokes(FakeAction{Action: "get-field-selector"}, nil)
return fields.Everything()
}
2 changes: 1 addition & 1 deletion pkg/client/testclient/fake_limit_ranges.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func (c *FakeLimitRanges) Update(limitRange *api.LimitRange) (*api.LimitRange, e
}

func (c *FakeLimitRanges) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-limitRange", Value: resourceVersion})
c.Fake.Invokes(FakeAction{Action: "watch-limitRange", Value: resourceVersion}, nil)
return c.Fake.Watch, nil
}
6 changes: 3 additions & 3 deletions pkg/client/testclient/fake_namespaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ func (c *FakeNamespaces) Delete(name string) error {
}

func (c *FakeNamespaces) Create(namespace *api.Namespace) (*api.Namespace, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "create-namespace"})
return &api.Namespace{}, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "create-namespace"}, nil)
return &api.Namespace{}, c.Fake.Err()
}

func (c *FakeNamespaces) Update(namespace *api.Namespace) (*api.Namespace, error) {
Expand All @@ -55,7 +55,7 @@ func (c *FakeNamespaces) Update(namespace *api.Namespace) (*api.Namespace, error
}

func (c *FakeNamespaces) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-namespaces", Value: resourceVersion})
c.Fake.Invokes(FakeAction{Action: "watch-namespaces", Value: resourceVersion}, nil)
return c.Fake.Watch, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_nodes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,6 @@ func (c *FakeNodes) UpdateStatus(minion *api.Node) (*api.Node, error) {
}

func (c *FakeNodes) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-nodes", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-nodes", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_persistent_volume_claims.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ func (c *FakePersistentVolumeClaims) UpdateStatus(claim *api.PersistentVolumeCla
}

func (c *FakePersistentVolumeClaims) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-persistentVolumeClaims", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-persistentVolumeClaims", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_persistent_volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ func (c *FakePersistentVolumes) UpdateStatus(pv *api.PersistentVolume) (*api.Per
}

func (c *FakePersistentVolumes) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-persistentVolumes", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-persistentVolumes", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_pod_templates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func (c *FakePodTemplates) Update(pod *api.PodTemplate) (*api.PodTemplate, error
}

func (c *FakePodTemplates) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-podTemplates", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-podTemplates", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
6 changes: 3 additions & 3 deletions pkg/client/testclient/fake_pods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func (c *FakePods) Update(pod *api.Pod) (*api.Pod, error) {
}

func (c *FakePods) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-pods", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-pods", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}

func (c *FakePods) Bind(bind *api.Binding) error {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "bind-pod", Value: bind.Name})
c.Fake.Invokes(FakeAction{Action: "bind-pod", Value: bind.Name}, nil)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/client/testclient/fake_replication_controllers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,6 @@ func (c *FakeReplicationControllers) Delete(name string) error {
}

func (c *FakeReplicationControllers) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: WatchControllerAction, Value: resourceVersion})
c.Fake.Invokes(FakeAction{Action: WatchControllerAction, Value: resourceVersion}, nil)
return c.Fake.Watch, nil
}
2 changes: 1 addition & 1 deletion pkg/client/testclient/fake_resource_quotas.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ func (c *FakeResourceQuotas) UpdateStatus(resourceQuota *api.ResourceQuota) (*ap
}

func (c *FakeResourceQuotas) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-resourceQuota", Value: resourceVersion})
c.Fake.Invokes(FakeAction{Action: "watch-resourceQuota", Value: resourceVersion}, nil)
return c.Fake.Watch, nil
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func (c *FakeSecrets) Delete(name string) error {
}

func (c *FakeSecrets) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-secrets", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-secrets", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_service_accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func (c *FakeServiceAccounts) Delete(name string) error {
}

func (c *FakeServiceAccounts) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-serviceAccounts", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-serviceAccounts", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
4 changes: 2 additions & 2 deletions pkg/client/testclient/fake_services.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func (c *FakeServices) Delete(name string) error {
}

func (c *FakeServices) Watch(label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
c.Fake.Actions = append(c.Fake.Actions, FakeAction{Action: "watch-services", Value: resourceVersion})
return c.Fake.Watch, c.Fake.Err
c.Fake.Invokes(FakeAction{Action: "watch-services", Value: resourceVersion}, nil)
return c.Fake.Watch, c.Fake.Err()
}
53 changes: 46 additions & 7 deletions pkg/client/testclient/testclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package testclient

import (
"sync"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/registered"
Expand Down Expand Up @@ -48,10 +50,11 @@ type ReactionFunc func(FakeAction) (runtime.Object, error)
// Fake implements client.Interface. Meant to be embedded into a struct to get a default
// implementation. This makes faking out just the method you want to test easier.
type Fake struct {
Actions []FakeAction
Watch watch.Interface
Err error
sync.RWMutex
actions []FakeAction
err error

Watch watch.Interface
// ReactFn is an optional function that will be invoked with the provided action
// and return a response. It can implement scenario specific behavior. The type
// of object returned must match the expected type from the caller (even if nil).
Expand All @@ -61,11 +64,47 @@ type Fake struct {
// Invokes records the provided FakeAction and then invokes the ReactFn (if provided).
// obj is expected to be of the same type a normal call would return.
func (c *Fake) Invokes(action FakeAction, obj runtime.Object) (runtime.Object, error) {
c.Actions = append(c.Actions, action)
c.Lock()
defer c.Unlock()

c.actions = append(c.actions, action)
if c.ReactFn != nil {
return c.ReactFn(action)
}
return obj, c.Err
return obj, c.err
}

// ClearActions clears the history of actions called on the fake client
func (c *Fake) ClearActions() {
c.Lock()
c.Unlock()

c.actions = make([]FakeAction, 0)
}

// Actions returns a chronologically ordered slice fake actions called on the fake client
func (c *Fake) Actions() []FakeAction {
c.RLock()
defer c.RUnlock()
fa := make([]FakeAction, len(c.actions))
copy(fa, c.actions)
return fa
}

// SetErr sets the error to return for client calls
func (c *Fake) SetErr(err error) {
c.Lock()
defer c.Unlock()

c.err = err
}

// Err returns any a client error or nil
func (c *Fake) Err() error {
c.RLock()
c.RUnlock()

return c.err
}

func (c *Fake) LimitRanges(namespace string) client.LimitRangeInterface {
Expand Down Expand Up @@ -125,13 +164,13 @@ func (c *Fake) Namespaces() client.NamespaceInterface {
}

func (c *Fake) ServerVersion() (*version.Info, error) {
c.Actions = append(c.Actions, FakeAction{Action: "get-version", Value: nil})
c.Invokes(FakeAction{Action: "get-version", Value: nil}, nil)
versionInfo := version.Get()
return &versionInfo, nil
}

func (c *Fake) ServerAPIVersions() (*api.APIVersions, error) {
c.Actions = append(c.Actions, FakeAction{Action: "get-apiversions", Value: nil})
c.Invokes(FakeAction{Action: "get-apiversions", Value: nil}, nil)
return &api.APIVersions{Versions: registered.RegisteredVersions}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cloudprovider/nodecontroller/nodecontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ func TestMonitorNodeStatusEvictPods(t *testing.T) {

podEvictor.TryEvict(func(nodeName string) { nodeController.deletePods(nodeName) })
podEvicted := false
for _, action := range item.fakeNodeHandler.Actions {
for _, action := range item.fakeNodeHandler.Actions() {
if action.Action == "delete-pod" {
podEvicted = true
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/cloudprovider/servicecontroller/servicecontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,21 @@ func TestCreateExternalLoadBalancer(t *testing.T) {
client := &testclient.Fake{}
controller := New(cloud, client, "test-cluster")
controller.init()
cloud.Calls = nil // ignore any cloud calls made in init()
client.Actions = nil // ignore any client calls made in init()
cloud.Calls = nil // ignore any cloud calls made in init()
client.ClearActions() // ignore any client calls made in init()
err, _ := controller.createLoadBalancerIfNeeded(types.NamespacedName{"foo", "bar"}, item.service, nil)
if !item.expectErr && err != nil {
t.Errorf("unexpected error: %v", err)
} else if item.expectErr && err == nil {
t.Errorf("expected error creating %v, got nil", item.service)
}
actions := client.Actions()
if !item.expectCreateAttempt {
if len(cloud.Calls) > 0 {
t.Errorf("unexpected cloud provider calls: %v", cloud.Calls)
}
if len(client.Actions) > 0 {
t.Errorf("unexpected client actions: %v", client.Actions)
if len(actions) > 0 {
t.Errorf("unexpected client actions: %v", actions)
}
} else {
if len(cloud.Balancers) != 1 {
Expand All @@ -117,13 +118,13 @@ func TestCreateExternalLoadBalancer(t *testing.T) {
t.Errorf("created load balancer has incorrect parameters: %v", cloud.Balancers[0])
}
actionFound := false
for _, action := range client.Actions {
for _, action := range actions {
if action.Action == "update-service" {
actionFound = true
}
}
if !actionFound {
t.Errorf("expected updated service to be sent to client, got these actions instead: %v", client.Actions)
t.Errorf("expected updated service to be sent to client, got these actions instead: %v", actions)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/replication/replication_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ func TestControllerUpdateStatusWithFailure(t *testing.T) {
numReplicas := 10
updateReplicaCount(fakeRCClient, *rc, numReplicas)
updates, gets := 0, 0
for _, a := range fakeClient.Actions {
for _, a := range fakeClient.Actions() {
switch a.Action {
case testclient.GetControllerAction:
gets++
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubectl/rolling_updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@ func (c *fakeRc) Get(name string) (*api.ReplicationController, error) {
if len(c.responses) == 0 {
return nil, fmt.Errorf("Unexpected Action: %s", action)
}
c.Fake.Actions = append(c.Fake.Actions, action)
c.Fake.Invokes(action, nil)
result := c.responses[0]
c.responses = c.responses[1:]
return result.controller, result.err
}

func (c *fakeRc) Create(controller *api.ReplicationController) (*api.ReplicationController, error) {
c.Fake.Actions = append(c.Fake.Actions, testclient.FakeAction{Action: "create-controller", Value: controller.ObjectMeta.Name})
c.Fake.Invokes(testclient.FakeAction{Action: "create-controller", Value: controller.ObjectMeta.Name}, nil)
return controller, nil
}

func (c *fakeRc) Update(controller *api.ReplicationController) (*api.ReplicationController, error) {
c.Fake.Actions = append(c.Fake.Actions, testclient.FakeAction{Action: "update-controller", Value: controller.ObjectMeta.Name})
c.Fake.Invokes(testclient.FakeAction{Action: "update-controller", Value: controller.ObjectMeta.Name}, nil)
return controller, nil
}

Expand Down
22 changes: 12 additions & 10 deletions pkg/kubectl/scale_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,15 @@ func TestReplicationControllerScale(t *testing.T) {
name := "foo"
scaler.Scale("default", name, count, &preconditions, nil, nil)

if len(fake.Actions) != 2 {
t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", fake.Actions)
actions := fake.Actions()
if len(actions) != 2 {
t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", actions)
}
if fake.Actions[0].Action != "get-replicationController" || fake.Actions[0].Value != name {
t.Errorf("unexpected action: %v, expected get-replicationController %s", fake.Actions[0], name)
if actions[0].Action != "get-replicationController" || actions[0].Value != name {
t.Errorf("unexpected action: %v, expected get-replicationController %s", actions[0], name)
}
if fake.Actions[1].Action != "update-replicationController" || fake.Actions[1].Value.(*api.ReplicationController).Spec.Replicas != int(count) {
t.Errorf("unexpected action %v, expected update-replicationController with replicas = %d", fake.Actions[1], count)
if actions[1].Action != "update-replicationController" || actions[1].Value.(*api.ReplicationController).Spec.Replicas != int(count) {
t.Errorf("unexpected action %v, expected update-replicationController with replicas = %d", actions[1], count)
}
}

Expand All @@ -96,11 +97,12 @@ func TestReplicationControllerScaleFailsPreconditions(t *testing.T) {
name := "foo"
scaler.Scale("default", name, count, &preconditions, nil, nil)

if len(fake.Actions) != 1 {
t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", fake.Actions)
actions := fake.Actions()
if len(actions) != 1 {
t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", actions)
}
if fake.Actions[0].Action != "get-replicationController" || fake.Actions[0].Value != name {
t.Errorf("unexpected action: %v, expected get-replicationController %s", fake.Actions[0], name)
if actions[0].Action != "get-replicationController" || actions[0].Value != name {
t.Errorf("unexpected action: %v, expected get-replicationController %s", actions[0], name)
}
}

Expand Down
12 changes: 7 additions & 5 deletions pkg/kubectl/stop_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ func TestReplicationControllerStop(t *testing.T) {
if s != expected {
t.Errorf("expected %s, got %s", expected, s)
}
if len(fake.Actions) != 7 {
actions := fake.Actions()
if len(actions) != 7 {
t.Errorf("unexpected actions: %v, expected 6 actions (get, list, get, update, get, get, delete)", fake.Actions)
}
for i, action := range []string{"get", "list", "get", "update", "get", "get", "delete"} {
if fake.Actions[i].Action != action+"-replicationController" {
t.Errorf("unexpected action: %+v, expected %s-replicationController", fake.Actions[i], action)
if actions[i].Action != action+"-replicationController" {
t.Errorf("unexpected action: %+v, expected %s-replicationController", actions[i], action)
}
}
}
Expand Down Expand Up @@ -159,10 +160,11 @@ func TestSimpleStop(t *testing.T) {
t.Errorf("unexpected return: %s (%s)", s, test.test)
}
}
if len(test.actions) != len(fake.Actions) {
actions := fake.Actions()
if len(test.actions) != len(actions) {
t.Errorf("unexpected actions: %v; expected %v (%s)", fake.Actions, test.actions, test.test)
}
for i, action := range fake.Actions {
for i, action := range actions {
testAction := test.actions[i]
if action.Action != testAction {
t.Errorf("unexpected action: %v; expected %v (%s)", action, testAction, test.test)
Expand Down
Loading

0 comments on commit 1b84fb7

Please sign in to comment.