Repository: knative/client
Pull request #895 (2020-06-19 11:32:47)

Refer broker from v1beta1 APIs and add missing e2e tests
## Description
- Broker is now moved to v1beta1 APIs, while we were referring it from `v1alpha1`
https://github.com/knative/client/blob/10b23b8802d59f0c0cecbe2d7f4ae8af944cbf68/pkg/kn/commands/flags/sink.go#L53
 this wasnt caught in e2e because we dont have an e2e for referring broker using `--sink`.
- This PR refers broker from v1beta1 and add respective e2e tests to verify.

## Changes
- Move broker injection namespace label, unlabel utilities to lib
- Refer broker from eventing v1beta1 APIs
- Add e2e test for refering default broker as sink

/lint

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/895#" title="Author self-approved">navidshaikh</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/master/OWNERS)~~ [navidshaikh]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/kn/commands/flags/sink.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/895/pull-knative-client-go-coverage/1273941868517789699/artifacts/line-cov.html#file0) | 57.6% | 45.5% | -12.1

knative-prow-robot: @navidshaikh: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-client-go-coverage | da8e46d481304dc583890aca3bef5ca9871ddc68 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/895/pull-knative-client-go-coverage/1273941868517789699) | `/test pull-knative-client-go-coverage`

[Full PR test history](https://gubernator.knative.dev/pr/knative_client/895). [Your PR dashboard](https://gubernator.knative.dev/pr/navidshaikh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @navidshaikh: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-client-go-coverage | da8e46d481304dc583890aca3bef5ca9871ddc68 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/895/pull-knative-client-go-coverage/1273941868517789699) | `/test pull-knative-client-go-coverage`
pull-knative-client-unit-tests | da8e46d481304dc583890aca3bef5ca9871ddc68 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/895/pull-knative-client-unit-tests/1273941868517789698) | `/test pull-knative-client-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_client/895). [Your PR dashboard](https://gubernator.knative.dev/pr/navidshaikh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/895#" title="Author self-approved">navidshaikh</a>*, *<a href="https://github.com/knative/client/pull/895#pullrequestreview-434015852" title="LGTM">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/master/OWNERS)~~ [navidshaikh,rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: New changes are detected. LGTM label has been removed.
knative-prow-robot: @navidshaikh: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-client-integration-tests-latest-release | 2aa63b779a10645ceed412ac0c19fd46d295cbce | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/895/pull-knative-client-integration-tests-latest-release/1273947530958213121) | `/test pull-knative-client-integration-tests-latest-release`

[Full PR test history](https://gubernator.knative.dev/pr/knative_client/895). [Your PR dashboard](https://gubernator.knative.dev/pr/navidshaikh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
navidshaikh: /test pull-knative-client-integration-tests-latest-release

    Error: timeout: service 'hello' not ready after 600 seconds

rhuss: /lgtm

Patch:
diff --git a/pkg/dynamic/fake/fake.go b/pkg/dynamic/fake/fake.go
index be89639c..c4c29568 100644
--- a/pkg/dynamic/fake/fake.go
+++ b/pkg/dynamic/fake/fake.go
@@ -29,7 +29,7 @@ import (
 func CreateFakeKnDynamicClient(testNamespace string, objects ...runtime.Object) dynamic.KnDynamicClient {
 	scheme := runtime.NewScheme()
 	scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "serving.knative.dev", Version: "v1", Kind: "Service"}, &servingv1.Service{})
-	scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "eventing.knative.dev", Version: "v1alpha1", Kind: "Broker"}, &eventingv1beta1.Broker{})
+	scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "eventing.knative.dev", Version: "v1beta1", Kind: "Broker"}, &eventingv1beta1.Broker{})
 	client := dynamicfake.NewSimpleDynamicClient(scheme, objects...)
 	return dynamic.NewKnDynamicClient(client, testNamespace)
 }
diff --git a/pkg/kn/commands/flags/sink.go b/pkg/kn/commands/flags/sink.go
index d8732b8b..0ef554ad 100644
--- a/pkg/kn/commands/flags/sink.go
+++ b/pkg/kn/commands/flags/sink.go
@@ -50,7 +50,7 @@ var sinkMappings = map[string]schema.GroupVersionResource{
 	"broker": {
 		Resource: "brokers",
 		Group:    "eventing.knative.dev",
-		Version:  "v1alpha1",
+		Version:  "v1beta1",
 	},
 	"service": {
 		Resource: "services",


Test patch:
diff --git a/lib/test/broker.go b/lib/test/broker.go
new file mode 100644
index 00000000..99dcb953
--- /dev/null
+++ b/lib/test/broker.go
@@ -0,0 +1,48 @@
+// Copyright 2020 The Knative 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 im
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package test
+
+import (
+	"strings"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+)
+
+// LabelNamespaceForDefaultBroker adds label 'knative-eventing-injection=enabled' to the configured namespace
+func LabelNamespaceForDefaultBroker(r *KnRunResultCollector) error {
+	_, err := Kubectl{}.Run("label", "namespace", r.KnTest().Kn().Namespace(), "knative-eventing-injection=enabled")
+
+	if err != nil {
+		r.T().Fatalf("Error executing 'kubectl label namespace %s knative-eventing-injection=enabled'. Error: %s", r.KnTest().Kn().Namespace(), err.Error())
+	}
+
+	return wait.PollImmediate(10*time.Second, 5*time.Minute, func() (bool, error) {
+		out, err := NewKubectl(r.KnTest().Kn().Namespace()).Run("get", "broker", "-o=jsonpath='{.items[0].status.conditions[?(@.type==\"Ready\")].status}'")
+		if err != nil {
+			return false, nil
+		}
+
+		return strings.Contains(out, "True"), nil
+	})
+}
+
+// UnlabelNamespaceForDefaultBroker removes label 'knative-eventing-injection=enabled' from the configured namespace
+func UnlabelNamespaceForDefaultBroker(r *KnRunResultCollector) {
+	_, err := Kubectl{}.Run("label", "namespace", r.KnTest().Kn().Namespace(), "knative-eventing-injection-")
+	if err != nil {
+		r.T().Fatalf("Error executing 'kubectl label namespace %s knative-eventing-injection-'. Error: %s", r.KnTest().Kn().Namespace(), err.Error())
+	}
+}
diff --git a/pkg/kn/commands/flags/sink_test.go b/pkg/kn/commands/flags/sink_test.go
index f3b72147..aa8d09ab 100644
--- a/pkg/kn/commands/flags/sink_test.go
+++ b/pkg/kn/commands/flags/sink_test.go
@@ -40,7 +40,7 @@ func TestResolve(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{Name: "mysvc", Namespace: "default"},
 	}
 	defaultBroker := &eventingv1beta1.Broker{
-		TypeMeta:   metav1.TypeMeta{Kind: "Broker", APIVersion: "eventing.knative.dev/v1alpha1"},
+		TypeMeta:   metav1.TypeMeta{Kind: "Broker", APIVersion: "eventing.knative.dev/v1beta1"},
 		ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"},
 	}
 
@@ -59,7 +59,7 @@ func TestResolve(t *testing.T) {
 		{"svc:absent", nil, "\"absent\" not found"},
 		{"broker:default", &duckv1.Destination{
 			Ref: &duckv1.KReference{Kind: "Broker",
-				APIVersion: "eventing.knative.dev/v1alpha1",
+				APIVersion: "eventing.knative.dev/v1beta1",
 				Namespace:  "default",
 				Name:       "default"}}, ""},
 		{"http://target.example.com", &duckv1.Destination{
diff --git a/test/e2e/sink_test.go b/test/e2e/sink_test.go
new file mode 100644
index 00000000..82b0227a
--- /dev/null
+++ b/test/e2e/sink_test.go
@@ -0,0 +1,54 @@
+// Copyright 2020 The Knative 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 im
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build e2e
+// +build !serving
+
+package e2e
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+
+	"knative.dev/client/lib/test"
+)
+
+func TestSink(t *testing.T) {
+	t.Parallel()
+	it, err := test.NewKnTest()
+	assert.NilError(t, err)
+	defer func() {
+		assert.NilError(t, it.Teardown())
+	}()
+
+	r := test.NewKnRunResultCollector(t, it)
+	defer r.DumpIfFailed()
+
+	// create broker
+	err = test.LabelNamespaceForDefaultBroker(r)
+	assert.NilError(t, err)
+	defer test.UnlabelNamespaceForDefaultBroker(r)
+
+	t.Log("Create Ping source with a sink to the default broker")
+	pingSourceCreate(r, "testpingsource0", "* * * * */1", "ping", "broker:default")
+
+	jpSinkRefNameInSpec := "jsonpath={.spec.sink.ref.name}"
+	out, err := test.GetResourceFieldsWithJSONPath(t, it, "pingsource", "testpingsource0", jpSinkRefNameInSpec)
+	assert.NilError(t, err)
+	assert.Equal(t, out, "default")
+
+	t.Log("delete Ping sources")
+	pingSourceDelete(r, "testpingsource0")
+}
diff --git a/test/e2e/trigger_test.go b/test/e2e/trigger_test.go
index 34757136..fde09f1b 100644
--- a/test/e2e/trigger_test.go
+++ b/test/e2e/trigger_test.go
@@ -18,12 +18,9 @@
 package e2e
 
 import (
-	"strings"
 	"testing"
-	"time"
 
 	"gotest.tools/assert"
-	"k8s.io/apimachinery/pkg/util/wait"
 
 	"knative.dev/client/lib/test"
 	"knative.dev/client/pkg/util"
@@ -40,9 +37,9 @@ func TestBrokerTrigger(t *testing.T) {
 	r := test.NewKnRunResultCollector(t, it)
 	defer r.DumpIfFailed()
 
-	err = lableNamespaceForDefaultBroker(t, it)
+	err = test.LabelNamespaceForDefaultBroker(r)
 	assert.NilError(t, err)
-	defer unlableNamespaceForDefaultBroker(t, it)
+	defer test.UnlabelNamespaceForDefaultBroker(r)
 
 	test.ServiceCreate(r, "sinksvc0")
 	test.ServiceCreate(r, "sinksvc1")
@@ -82,30 +79,6 @@ func TestBrokerTrigger(t *testing.T) {
 
 // Private functions
 
-func unlableNamespaceForDefaultBroker(t *testing.T, it *test.KnTest) {
-	_, err := test.Kubectl{}.Run("label", "namespace", it.Kn().Namespace(), "knative-eventing-injection-")
-	if err != nil {
-		t.Fatalf("Error executing 'kubectl label namespace %s knative-eventing-injection-'. Error: %s", it.Kn().Namespace(), err.Error())
-	}
-}
-
-func lableNamespaceForDefaultBroker(t *testing.T, it *test.KnTest) error {
-	_, err := test.Kubectl{}.Run("label", "namespace", it.Kn().Namespace(), "knative-eventing-injection=enabled")
-
-	if err != nil {
-		t.Fatalf("Error executing 'kubectl label namespace %s knative-eventing-injection=enabled'. Error: %s", it.Kn().Namespace(), err.Error())
-	}
-
-	return wait.PollImmediate(10*time.Second, 5*time.Minute, func() (bool, error) {
-		out, err := test.NewKubectl(it.Kn().Namespace()).Run("get", "broker", "-o=jsonpath='{.items[0].status.conditions[?(@.type==\"Ready\")].status}'")
-		if err != nil {
-			return false, nil
-		} else {
-			return strings.Contains(out, "True"), nil
-		}
-	})
-}
-
 func triggerCreate(r *test.KnRunResultCollector, name string, sinksvc string, filters []string) {
 	args := []string{"trigger", "create", name, "--broker", "default", "--sink", "svc:" + sinksvc}
 	if len(filters) > 0 {