Merge pull request #15966 from jim-minter/template-test-fixes

Automatic merge from submit-queue

Template test fixes

- add all template tests to Conformance
- don't use persistent templates for templateservicebroker tests
- additional template instance controller readiness checking testing
- increase tsb loglevel in extended tests

fixes #15912
fixes #15961

Author: openshift-merge-robot

examples/templateservicebroker/templateservicebroker-template.yaml

@@ -55,6 +55,11 @@ objects:
             name: serving-cert
           - mountPath: /var/apiserver-config
             name: apiserver-config
+          readinessProbe:
+            httpGet:
+              path: /healthz
+              port: 8443
+              scheme: HTTPS
         volumes:
         - name: serving-cert
           secret:

hack/update-generated-bindata.sh

@@ -59,7 +59,7 @@ pushd "${OS_ROOT}" > /dev/null
 popd > /dev/null
 
 # If you hit this, please reduce other tests instead of importing more
-if [[ "$( cat "${OUTPUT_PARENT}/test/extended/testdata/bindata.go" | wc -c )" -gt 920000 ]]; then
+if [[ "$( cat "${OUTPUT_PARENT}/test/extended/testdata/bindata.go" | wc -c )" -gt 950000 ]]; then
     echo "error: extended bindata is $( cat "${OUTPUT_PARENT}/test/extended/testdata/bindata.go" | wc -c ) bytes, reduce the size of the import" 1>&2
     exit 1
 fi

pkg/oc/bootstrap/bindata.go

@@ -155,7 +155,7 @@ func (c *TemplateInstanceController) sync(key string) error {
 	}
 
 	if !templateInstance.HasCondition(templateapi.TemplateInstanceInstantiateFailure, kapi.ConditionTrue) {
-		ready, err := c.checkReadiness(templateInstance)
+		ready, err := c.checkReadiness(templateInstance, time.Now())
 		if err != nil {
 			glog.V(4).Infof("TemplateInstance controller: checkReadiness %s returned %v", key, err)
 
@@ -205,8 +205,8 @@ func (c *TemplateInstanceController) sync(key string) error {
 	return nil
 }
 
-func (c *TemplateInstanceController) checkReadiness(templateInstance *templateapi.TemplateInstance) (bool, error) {
-	if time.Now().After(templateInstance.CreationTimestamp.Add(readinessTimeout)) {
+func (c *TemplateInstanceController) checkReadiness(templateInstance *templateapi.TemplateInstance, now time.Time) (bool, error) {
+	if now.After(templateInstance.CreationTimestamp.Add(readinessTimeout)) {
 		return false, fmt.Errorf("Timeout")
 	}
 

pkg/template/controller/templateinstance_controller.go

@@ -0,0 +1,121 @@
+package controller
+
+import (
+	"bytes"
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/rest"
+	clientgotesting "k8s.io/client-go/testing"
+	kapi "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/authorization"
+	batchv1 "k8s.io/kubernetes/pkg/apis/batch/v1"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
+
+	"github.com/openshift/origin/pkg/client"
+	templateapi "github.com/openshift/origin/pkg/template/apis/template"
+)
+
+type roundtripper func(*http.Request) (*http.Response, error)
+
+func (rt roundtripper) RoundTrip(r *http.Request) (*http.Response, error) {
+	return rt(r)
+}
+
+// TestControllerCheckReadiness verifies the basic behaviour of
+// TemplateInstanceController.checkReadiness(): that it can return ready, not
+// ready and timed out correctly.
+func TestControllerCheckReadiness(t *testing.T) {
+	job := batchv1.Job{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "batch/v1",
+			Kind:       "Job",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations: map[string]string{
+				templateapi.WaitForReadyAnnotation: "true",
+			},
+		},
+	}
+
+	// fake generic API server, responds to any HTTP req with the above job
+	// object
+	fakerestconfig := &rest.Config{
+		WrapTransport: func(http.RoundTripper) http.RoundTripper {
+			return roundtripper(func(req *http.Request) (*http.Response, error) {
+				b, err := json.Marshal(job)
+				if err != nil {
+					panic(err)
+				}
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Body:       ioutil.NopCloser(bytes.NewBuffer(b)),
+				}, nil
+			})
+		},
+	}
+
+	// fakeclient, respond "allowed" to any subjectaccessreview
+	fakeclientset := &fake.Clientset{}
+	c := &TemplateInstanceController{
+		restmapper: client.DefaultMultiRESTMapper(),
+		kc:         fakeclientset,
+		config:     fakerestconfig,
+	}
+	fakeclientset.AddReactor("create", "subjectaccessreviews", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
+		return true, &authorization.SubjectAccessReview{Status: authorization.SubjectAccessReviewStatus{Allowed: true}}, nil
+	})
+
+	now := time.Now()
+	templateInstance := &templateapi.TemplateInstance{
+		ObjectMeta: metav1.ObjectMeta{
+			CreationTimestamp: metav1.Time{Time: now},
+		},
+		Spec: templateapi.TemplateInstanceSpec{
+			Requester: &templateapi.TemplateInstanceRequester{},
+		},
+		Status: templateapi.TemplateInstanceStatus{
+			Objects: []templateapi.TemplateInstanceObject{
+				{
+					Ref: kapi.ObjectReference{
+						APIVersion: "batch/v1",
+						Kind:       "Job",
+						Namespace:  "namespace",
+						Name:       "name",
+					},
+				},
+			},
+		},
+	}
+
+	// should report not ready yet
+	ready, err := c.checkReadiness(templateInstance, now)
+	if ready || err != nil {
+		t.Error(ready, err)
+	}
+
+	// should report timed out
+	ready, err = c.checkReadiness(templateInstance, now.Add(readinessTimeout+1))
+	if ready || err == nil || err.Error() != "Timeout" {
+		t.Error(ready, err)
+	}
+
+	// should report ready
+	job.Status.CompletionTime = &metav1.Time{Time: now}
+	ready, err = c.checkReadiness(templateInstance, now)
+	if !ready || err != nil {
+		t.Error(ready, err)
+	}
+
+	// should report failed
+	job.Status.Failed = int32(1)
+	ready, err = c.checkReadiness(templateInstance, now)
+	if ready || err == nil || err.Error() != "Readiness failed on Job namespace/name" {
+		t.Error(ready, err)
+	}
+}

pkg/template/controller/templateinstance_controller_test.go

@@ -12,15 +12,15 @@ From the top-level origin directory, run
 
 Where \<some_script\>.sh is one of the bucket scripts such as "core.sh".
 
-You can further narrow the set of tests being run by passing
-`--ginkgo.focus='regex'` where 'regex' is a regular expression matching the
+You can further narrow the set of tests being run by setting the environment
+variable `FOCUS='regex'` where 'regex' is a regular expression matching the
 description of the test you want to run.  For example one of the s2i tests
 (s2i_incremental.go) defines:
 
 	var _ = g.Describe("[builds][Slow] incremental s2i build", func() {
 
-So you can write a focus regex that includes this test by passing
-`--ginkgo.focus='\[builds\]'` or `--ginkgo.focus='incremental s2i'`.
+So you can write a focus regex that includes this test by setting
+`FOCUS='\[builds\]'` or `FOCUS='incremental s2i'`.
 
 Prerequisites
 -------------
@@ -53,18 +53,18 @@ $ export KUBECONFIG=${KUBECONFIG-$HOME/.kube/config}
 Then, for example:
 ```console
 $ make build-extended-test
-$ TEST_ONLY=1 test/extended/core.sh --ginkgo.focus='\[builds\]'
+$ FOCUS='\[builds\]' TEST_ONLY=1 test/extended/core.sh
 ```
 
 By default the Kubernetes test framework will remove the project associated with
 your test spec when it completes, regardless of whether it fails or not.
-Origin's wrapper scripts may also do clean-up.  This can be inconvenient when
-debugging.  To stop this behaviour, set the `SKIP_TEARDOWN` environment variable
-and add the argument `--delete-namespace=false`:
+Origin's wrapper scripts may also do clean-up.  Running tests in parallel can
+also hinder debugging.  To stop these behaviours, set the `SKIP_TEARDOWN`
+environment variable, set `DELETE_NAMESPACE=false`, and set `PARALLEL_NODES=1`:
 
 ```console
 $ make build-extended-test
-$ SKIP_TEARDOWN=1 TEST_ONLY=1 test/extended/core.sh --delete-namespace=false --ginkgo.focus='\[builds\]'
+$ FOCUS='\[builds\]' TEST_ONLY=1 SKIP_TEARDOWN=1 DELETE_NAMESPACE=false PARALLEL_NODES=1 test/extended/core.sh
 ```
 
 Test labels