Merge pull request #35665 from m1093782566/m109-pet-test

Automatic merge from submit-queue

Add StatefulSet update pod unit test and set log level

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, read our contributor guidelines https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md and developer guide https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md
2. If you want *faster* PR reviews, read how: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md
3. Follow the instructions for writing a release note: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes
-->

**What**:
- Add petset controller update pet unit test
- set petset controller log level

**Why**
- #32482 fixed "losing pet updated information between update retries", as @bprashanth suggested, "there should be a UT to ensure we fix identity if something corrupts it". I implement the UT in this PR.
- set petset controller log level in order to avoid spamming.

@bprashanth

pkg/controller/petset/BUILD

@@ -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",
     ],
 )

pkg/controller/petset/pet.go

@@ -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
@@ -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
 }
 
@@ -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
@@ -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
 		}
 	}

pkg/controller/petset/pet_set.go

@@ -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)

pkg/controller/petset/pet_set_utils.go

@@ -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

pkg/controller/petset/pet_test.go

@@ -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()
+	}
+}
+
+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
+		}
+	}
+}