Merge pull request #34531 from mwielgus/checking-fun

Automatic merge from submit-queue

Checking function integrated with get from chanel in fed tests

Current federation controller tests assume that each actions results in only one etcd action. However, due to various timing issues, this may not necessary the truth and multiple updates can be generated. Thus tests should try to get all available messages from the debug channels instead failing on the first message, which may come from the previous test scenario. 

Applied to deployment tests. Other tests will be updated in the following PR(s).

cc: @quinton-hoole @wojtek-t

federation/pkg/federation-controller/deployment/deploymentcontroller_test.go

@@ -119,34 +119,34 @@ func TestDeploymentController(t *testing.T) {
 	// Create deployment. Expect to see it in cluster1.
 	dep1 := newDeploymentWithReplicas("depA", 6)
 	deploymentsWatch.Add(dep1)
-	createdDep1 := GetDeploymentFromChan(cluster1CreateChan)
-	assert.NotNil(t, createdDep1)
-	assert.Equal(t, dep1.Namespace, createdDep1.Namespace)
-	assert.Equal(t, dep1.Name, createdDep1.Name)
-	assert.Equal(t, dep1.Spec.Replicas, createdDep1.Spec.Replicas)
+	checkDeployment := func(base *extensionsv1.Deployment, replicas int32) CheckingFunction {
+		return func(obj runtime.Object) error {
+			if obj == nil {
+				return fmt.Errorf("Observed object is nil")
+			}
+			d := obj.(*extensionsv1.Deployment)
+			if err := CompareObjectMeta(base.ObjectMeta, d.ObjectMeta); err != nil {
+				return err
+			}
+			if replicas != *d.Spec.Replicas {
+				return fmt.Errorf("Replica count is different expected:%d observed:%d", replicas, *d.Spec.Replicas)
+			}
+			return nil
+		}
+	}
+	assert.NoError(t, CheckObjectFromChan(cluster1CreateChan, checkDeployment(dep1, *dep1.Spec.Replicas)))
 
 	// Increase replica count. Expect to see the update in cluster1.
 	newRep := int32(8)
 	dep1.Spec.Replicas = &newRep
 	deploymentsWatch.Modify(dep1)
-	updatedDep1 := GetDeploymentFromChan(cluster1UpdateChan)
-	assert.NotNil(t, updatedDep1)
-	assert.Equal(t, dep1.Namespace, updatedDep1.Namespace)
-	assert.Equal(t, dep1.Name, updatedDep1.Name)
-	assert.Equal(t, dep1.Spec.Replicas, updatedDep1.Spec.Replicas)
+	assert.NoError(t, CheckObjectFromChan(cluster1UpdateChan, checkDeployment(dep1, *dep1.Spec.Replicas)))
 
 	// Add new cluster. Although rebalance = false, no pods have been created yet so it should
 	// rebalance anyway.
 	clusterWatch.Add(cluster2)
-	updatedDep1 = GetDeploymentFromChan(cluster1UpdateChan)
-	createdDep2 := GetDeploymentFromChan(cluster2CreateChan)
-	assert.NotNil(t, updatedDep1)
-	assert.NotNil(t, createdDep2)
-
-	assert.Equal(t, dep1.Namespace, createdDep2.Namespace)
-	assert.Equal(t, dep1.Name, createdDep2.Name)
-	assert.Equal(t, *dep1.Spec.Replicas/2, *createdDep2.Spec.Replicas)
-	assert.Equal(t, *dep1.Spec.Replicas/2, *updatedDep1.Spec.Replicas)
+	assert.NoError(t, CheckObjectFromChan(cluster1UpdateChan, checkDeployment(dep1, *dep1.Spec.Replicas/2)))
+	assert.NoError(t, CheckObjectFromChan(cluster2CreateChan, checkDeployment(dep1, *dep1.Spec.Replicas/2)))
 }
 
 func GetDeploymentFromChan(c chan runtime.Object) *extensionsv1.Deployment {

federation/pkg/federation-controller/util/test/test_helper.go

@@ -17,7 +17,9 @@ limitations under the License.
 package testutil
 
 import (
+	"fmt"
 	"os"
+	"reflect"
 	"runtime/pprof"
 	"sync"
 	"time"
@@ -201,6 +203,49 @@ func GetObjectFromChan(c chan runtime.Object) runtime.Object {
 	}
 }
 
+type CheckingFunction func(runtime.Object) error
+
+// CheckObjectFromChan tries to get an object matching the given check function
+// within a reasonable time.
+func CheckObjectFromChan(c chan runtime.Object, checkFunction CheckingFunction) error {
+	delay := 20 * time.Second
+	var lastError error
+	for {
+		select {
+		case obj := <-c:
+			if lastError = checkFunction(obj); lastError == nil {
+				return nil
+			}
+			glog.Infof("Check function failed with %v", lastError)
+			delay = 5 * time.Second
+		case <-time.After(delay):
+			pprof.Lookup("goroutine").WriteTo(os.Stderr, 1)
+			if lastError == nil {
+				return fmt.Errorf("Failed to get an object from channel")
+			} else {
+				return lastError
+			}
+		}
+	}
+}
+
+// CompareObjectMeta returns an error when the given objects are not equivalent.
+func CompareObjectMeta(a, b api_v1.ObjectMeta) error {
+	if a.Namespace != b.Namespace {
+		return fmt.Errorf("Different namespace expected:%s observed:%s", a.Namespace, b.Namespace)
+	}
+	if a.Name != b.Name {
+		return fmt.Errorf("Different name expected:%s observed:%s", a.Namespace, b.Namespace)
+	}
+	if !reflect.DeepEqual(a.Annotations, b.Annotations) {
+		return fmt.Errorf("Annotations are different expected:%v observerd:%v", a.Annotations, b.Annotations)
+	}
+	if !reflect.DeepEqual(a.Labels, b.Labels) {
+		return fmt.Errorf("Annotations are different expected:%v observerd:%v", a.Labels, b.Labels)
+	}
+	return nil
+}
+
 func ToFederatedInformerForTestOnly(informer util.FederatedInformer) util.FederatedInformerForTestOnly {
 	inter := informer.(interface{})
 	return inter.(util.FederatedInformerForTestOnly)