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

Add locks in HPA test #24537

Merged
merged 1 commit into from
Apr 21, 2016
Merged
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
48 changes: 48 additions & 0 deletions pkg/controller/podautoscaler/horizontal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"fmt"
"io"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -66,10 +67,12 @@ type fakeResource struct {
}

type testCase struct {
sync.Mutex
minReplicas int
maxReplicas int
initialReplicas int
desiredReplicas int

// CPU target utilization as a percentage of the requested resources.
CPUTarget int
CPUCurrent int
Expand All @@ -88,6 +91,7 @@ type testCase struct {
resource *fakeResource
}

// Needs to be called under a lock.
func (tc *testCase) computeCPUCurrent() {
if len(tc.reportedLevels) != len(tc.reportedCPURequests) || len(tc.reportedLevels) == 0 {
return
Expand All @@ -111,6 +115,8 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
MatchLabels: map[string]string{"name": podNamePrefix},
}

tc.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

I hate githube. Please add a comment by computeCPUCurrent that it needs to be called under the lock.


tc.scaleUpdated = false
tc.statusUpdated = false
tc.eventCreated = false
Expand All @@ -126,9 +132,13 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
kind: "replicationcontrollers",
}
}
tc.Unlock()

fakeClient := &fake.Clientset{}
fakeClient.AddReactor("list", "horizontalpodautoscalers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := &extensions.HorizontalPodAutoscalerList{
Items: []extensions.HorizontalPodAutoscaler{
{
Expand All @@ -154,6 +164,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
},
},
}

if tc.CPUTarget > 0.0 {
obj.Items[0].Spec.CPUUtilization = &extensions.CPUTargetUtilization{TargetPercentage: tc.CPUTarget}
}
Expand All @@ -169,6 +180,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("get", "replicationcontrollers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := &extensions.Scale{
ObjectMeta: api.ObjectMeta{
Name: tc.resource.name,
Expand All @@ -186,6 +200,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("get", "deployments", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := &extensions.Scale{
ObjectMeta: api.ObjectMeta{
Name: tc.resource.name,
Expand All @@ -203,6 +220,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("get", "replicasets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := &extensions.Scale{
ObjectMeta: api.ObjectMeta{
Name: tc.resource.name,
Expand All @@ -220,6 +240,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("list", "pods", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := &api.PodList{}
for i := 0; i < len(tc.reportedCPURequests); i++ {
podName := fmt.Sprintf("%s-%d", podNamePrefix, i)
Expand Down Expand Up @@ -252,6 +275,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddProxyReactor("services", func(action core.Action) (handled bool, ret restclient.ResponseWrapper, err error) {
tc.Lock()
defer tc.Unlock()

timestamp := time.Now()
metrics := heapster.MetricResultList{}
for _, level := range tc.reportedLevels {
Expand All @@ -266,6 +292,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("update", "replicationcontrollers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := action.(testclient.UpdateAction).GetObject().(*extensions.Scale)
replicas := action.(testclient.UpdateAction).GetObject().(*extensions.Scale).Spec.Replicas
assert.Equal(t, tc.desiredReplicas, replicas)
Expand All @@ -274,6 +303,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("update", "deployments", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := action.(testclient.UpdateAction).GetObject().(*extensions.Scale)
replicas := action.(testclient.UpdateAction).GetObject().(*extensions.Scale).Spec.Replicas
assert.Equal(t, tc.desiredReplicas, replicas)
Expand All @@ -282,6 +314,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("update", "replicasets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := action.(testclient.UpdateAction).GetObject().(*extensions.Scale)
replicas := action.(testclient.UpdateAction).GetObject().(*extensions.Scale).Spec.Replicas
assert.Equal(t, tc.desiredReplicas, replicas)
Expand All @@ -290,6 +325,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("update", "horizontalpodautoscalers", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := action.(testclient.UpdateAction).GetObject().(*extensions.HorizontalPodAutoscaler)
assert.Equal(t, namespace, obj.Namespace)
assert.Equal(t, hpaName, obj.Name)
Expand All @@ -305,6 +343,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
})

fakeClient.AddReactor("*", "events", func(action core.Action) (handled bool, ret runtime.Object, err error) {
tc.Lock()
defer tc.Unlock()

obj := action.(testclient.CreateAction).GetObject().(*api.Event)
if tc.verifyEvents {
assert.Equal(t, "SuccessfulRescale", obj.Reason)
Expand All @@ -321,6 +362,9 @@ func (tc *testCase) prepareTestClient(t *testing.T) *fake.Clientset {
}

func (tc *testCase) verifyResults(t *testing.T) {
tc.Lock()
defer tc.Unlock()

assert.Equal(t, tc.initialReplicas != tc.desiredReplicas, tc.scaleUpdated)
assert.True(t, tc.statusUpdated)
if tc.verifyEvents {
Expand Down Expand Up @@ -351,9 +395,13 @@ func (tc *testCase) runTest(t *testing.T) {
defer close(stop)
go hpaController.Run(stop)

tc.Lock()
if tc.verifyEvents {
tc.Unlock()
// We need to wait for events to be broadcasted (sleep for longer than record.sleepDuration).
time.Sleep(2 * time.Second)
} else {
tc.Unlock()
}
// Wait for HPA to be processed.
<-tc.processed
Expand Down