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 StatefulSet update pod unit test and set log level #35665

Merged
merged 1 commit into from
Nov 7, 2016
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
8 changes: 8 additions & 0 deletions pkg/controller/petset/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,27 @@ go_test(
"identity_mappers_test.go",
"iterator_test.go",
"pet_set_test.go",
"pet_test.go",
],
library = "go_default_library",
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/api/testapi:go_default_library",
"//pkg/apimachinery/registered:go_default_library",
"//pkg/apis/apps:go_default_library",
"//pkg/client/cache:go_default_library",
"//pkg/client/clientset_generated/internalclientset:go_default_library",
"//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/apps/internalversion:go_default_library",
"//pkg/client/clientset_generated/internalclientset/typed/apps/internalversion/fake:go_default_library",
"//pkg/client/restclient:go_default_library",
"//pkg/client/testing/core:go_default_library",
"//pkg/controller:go_default_library",
"//pkg/runtime:go_default_library",
"//pkg/util/errors:go_default_library",
"//pkg/util/sets:go_default_library",
"//pkg/util/testing:go_default_library",
],
)
16 changes: 8 additions & 8 deletions pkg/controller/petset/pet.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ func (p *petSyncer) Sync(pet *pcb) error {
}
} else if exists {
if !p.isHealthy(realPet.pod) {
glog.Infof("StatefulSet %v waiting on unhealthy pet %v", pet.parent.Name, realPet.pod.Name)
glog.V(4).Infof("StatefulSet %v waiting on unhealthy pet %v", pet.parent.Name, realPet.pod.Name)
}
return p.Update(realPet, pet)
}
if p.blockingPet != nil {
message := errUnhealthyPet(fmt.Sprintf("Create of %v in StatefulSet %v blocked by unhealthy pet %v", pet.pod.Name, pet.parent.Name, p.blockingPet.pod.Name))
glog.Info(message)
glog.V(4).Infof(message.Error())
return message
}
// This is counted as a create, even if it fails. We can't skip indices
Expand Down Expand Up @@ -149,17 +149,17 @@ func (p *petSyncer) Delete(pet *pcb) error {
return nil
}
if p.blockingPet != nil {
glog.Infof("Delete of %v in StatefulSet %v blocked by unhealthy pet %v", realPet.pod.Name, pet.parent.Name, p.blockingPet.pod.Name)
glog.V(4).Infof("Delete of %v in StatefulSet %v blocked by unhealthy pet %v", realPet.pod.Name, pet.parent.Name, p.blockingPet.pod.Name)
return nil
}
// This is counted as a delete, even if it fails.
// The returned error will force a requeue.
p.blockingPet = realPet
if !p.isDying(realPet.pod) {
glog.Infof("StatefulSet %v deleting pet %v", pet.parent.Name, pet.pod.Name)
glog.V(4).Infof("StatefulSet %v deleting pet %v", pet.parent.Name, pet.pod.Name)
return p.petClient.Delete(pet)
}
glog.Infof("StatefulSet %v waiting on pet %v to die in %v", pet.parent.Name, realPet.pod.Name, realPet.pod.DeletionTimestamp)
glog.V(4).Infof("StatefulSet %v waiting on pet %v to die in %v", pet.parent.Name, realPet.pod.Name, realPet.pod.DeletionTimestamp)
return nil
}

Expand Down Expand Up @@ -223,7 +223,7 @@ func (p *apiServerPetClient) Update(pet *pcb, expectedPet *pcb) (updateErr error
if err != nil || !needsUpdate {
return err
}
glog.Infof("Resetting pet %v/%v to match StatefulSet %v spec", pet.pod.Namespace, pet.pod.Name, pet.parent.Name)
glog.V(4).Infof("Resetting pet %v/%v to match StatefulSet %v spec", pet.pod.Namespace, pet.pod.Name, pet.parent.Name)
_, updateErr = pc.Update(&updatePod)
if updateErr == nil || i >= updateRetries {
return updateErr
Expand Down Expand Up @@ -309,9 +309,9 @@ func (d *defaultPetHealthChecker) isHealthy(pod *api.Pod) bool {
initialized, ok := pod.Annotations[StatefulSetInitAnnotation]
if ok {
if initAnnotation, err := strconv.ParseBool(initialized); err != nil {
glog.Infof("Failed to parse %v annotation on pod %v: %v", StatefulSetInitAnnotation, pod.Name, err)
glog.V(4).Infof("Failed to parse %v annotation on pod %v: %v", StatefulSetInitAnnotation, pod.Name, err)
} else if !initAnnotation {
glog.Infof("StatefulSet pod %v waiting on annotation %v", pod.Name, StatefulSetInitAnnotation)
glog.V(4).Infof("StatefulSet pod %v waiting on annotation %v", pod.Name, StatefulSetInitAnnotation)
podReady = initAnnotation
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/petset/pet_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (psc *StatefulSetController) Sync(key string) error {

// syncStatefulSet syncs a tuple of (statefulset, pets).
func (psc *StatefulSetController) syncStatefulSet(ps *apps.StatefulSet, pets []*api.Pod) (int, error) {
glog.Infof("Syncing StatefulSet %v/%v with %d pods", ps.Namespace, ps.Name, len(pets))
glog.V(2).Infof("Syncing StatefulSet %v/%v with %d pods", ps.Namespace, ps.Name, len(pets))

it := NewStatefulSetIterator(ps, pets)
blockingPet, err := psc.blockingPetStore.Get(ps, pets)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/petset/pet_set_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (u *unhealthyPetTracker) Get(ps *apps.StatefulSet, knownPets []*api.Pod) (*
glog.V(4).Infof("Ignoring healthy pod %v for StatefulSet %v", p.Name, ps.Name)
continue
}
glog.Infof("No recorded blocking pod, but found unhealthy pod %v for StatefulSet %v", p.Name, ps.Name)
glog.V(4).Infof("No recorded blocking pod, but found unhealthy pod %v for StatefulSet %v", p.Name, ps.Name)
return &pcb{pod: p, parent: ps}, nil
}
return nil, nil
Expand Down
177 changes: 177 additions & 0 deletions pkg/controller/petset/pet_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
Copyright 2016 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package petset

import (
"fmt"
"net/http/httptest"
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/testapi"
"k8s.io/kubernetes/pkg/apimachinery/registered"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/client/restclient"
"k8s.io/kubernetes/pkg/client/testing/core"
"k8s.io/kubernetes/pkg/runtime"
utiltesting "k8s.io/kubernetes/pkg/util/testing"
)

func newPetClient(client *clientset.Clientset) *apiServerPetClient {
return &apiServerPetClient{
c: client,
}
}

func makeTwoDifferntPCB() (pcb1, pcb2 *pcb) {
userAdded := api.Volume{
Name: "test",
VolumeSource: api.VolumeSource{
EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory},
},
}
ps := newStatefulSet(2)
pcb1, _ = newPCB("1", ps)
pcb2, _ = newPCB("2", ps)
pcb2.pod.Spec.Volumes = append(pcb2.pod.Spec.Volumes, userAdded)
return pcb1, pcb2
}

func TestUpdatePetWithoutRetry(t *testing.T) {
pcb1, pcb2 := makeTwoDifferntPCB()
// invalid pet with empty pod
invalidPcb := *pcb1
invalidPcb.pod = nil

testCases := []struct {
realPet *pcb
expectedPet *pcb
expectErr bool
requests int
}{
// case 0: error occurs, no need to update
{
realPet: pcb1,
expectedPet: &invalidPcb,
expectErr: true,
requests: 0,
},
// case 1: identical pet, no need to update
{
realPet: pcb1,
expectedPet: pcb1,
expectErr: false,
requests: 0,
},
// case 2: need to call update once
{
realPet: pcb1,
expectedPet: pcb2,
expectErr: false,
requests: 1,
},
}

for k, tc := range testCases {
body := runtime.EncodeOrDie(testapi.Default.Codec(), &api.Pod{ObjectMeta: api.ObjectMeta{Name: "empty_pod"}})
fakeHandler := utiltesting.FakeHandler{
StatusCode: 200,
ResponseBody: string(body),
}
testServer := httptest.NewServer(&fakeHandler)

client := clientset.NewForConfigOrDie(&restclient.Config{Host: testServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &registered.GroupOrDie(api.GroupName).GroupVersion}})
petClient := newPetClient(client)
err := petClient.Update(tc.realPet, tc.expectedPet)

if tc.expectErr != (err != nil) {
t.Errorf("case %d: expect error(%v), got err: %v", k, tc.expectErr, err)
}
fakeHandler.ValidateRequestCount(t, tc.requests)
testServer.Close()
}
Copy link
Member

Choose a reason for hiding this comment

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

also check fakeHandler.ValidateRequestCount(t, n) here, n be the number of expected calls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good advice but there is a limitation in FakeHandler that we can call ValidateRequestCount only one time for every FakeHandler instance, otherwise will cause panic. So, it's not easy to ValidateRequestCount for every case. Do you have any good ideas?

Copy link
Member

Choose a reason for hiding this comment

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

what if we move the whole fakehandler part into the for loop so that each ValidateRequestCount uses different fakehandler instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will do.

}

func TestUpdatePetWithFailure(t *testing.T) {
fakeHandler := utiltesting.FakeHandler{
StatusCode: 500,
ResponseBody: "{}",
}
testServer := httptest.NewServer(&fakeHandler)
defer testServer.Close()

client := clientset.NewForConfigOrDie(&restclient.Config{Host: testServer.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &registered.GroupOrDie(api.GroupName).GroupVersion}})
petClient := newPetClient(client)

pcb1, pcb2 := makeTwoDifferntPCB()

if err := petClient.Update(pcb1, pcb2); err == nil {
t.Errorf("expect error, got nil")
}
// 1 Update and 1 GET, both of which fail
fakeHandler.ValidateRequestCount(t, 2)
}

func TestUpdatePetRetrySucceed(t *testing.T) {
pcb1, pcb2 := makeTwoDifferntPCB()

fakeClient := &fake.Clientset{}
fakeClient.AddReactor("get", "pods", func(action core.Action) (bool, runtime.Object, error) {
return true, pcb2.pod, nil
})
fakeClient.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
return true, nil, fmt.Errorf("Fake error")
})
petClient := apiServerPetClient{
c: fakeClient,
}

if err := petClient.Update(pcb1, pcb2); err != nil {
t.Errorf("unexpected error: %v", err)
}

actions := fakeClient.Actions()
if len(actions) != 2 {
t.Errorf("Expect 2 actions, got %d actions", len(actions))
}
for i := 0; i < len(actions); i++ {
a := actions[i]
if a.GetResource().Resource != "pods" {
t.Errorf("Unexpected action %+v", a)
continue
}

switch action := a.(type) {
case core.GetAction:
if i%2 == 0 {
t.Errorf("Unexpected Get action")
}
// Make sure the get is for the right pod
if action.GetName() != pcb2.pod.Name {
t.Errorf("Expected get pod %v, got %q instead", pcb2.pod.Name, action.GetName())
}
case core.UpdateAction:
if i%2 == 1 {
t.Errorf("Unexpected Update action")
}
default:
t.Errorf("Unexpected action %+v", a)
break
}
}
}