Integration tests for Tekton #528

mgencur · 2019-11-29T11:10:08Z

Fixes #471

Proposed Changes

install Tekton as part of the CI run
run integration tests using Tekton Pipeline and PipelineRun which pulls source for an application from github, builds it, pushes the resulting image to GCR registry and then runs kn task to create a Knative Service
there's a separate tekton-tests.sh which runs the tests but doesn't run as part of normal CI, we should create a nightly job that will use this script

knative-prow-robot

@mgencur: 0 warnings.

In response to this:

Fixes #

Proposed Changes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

* also put back the e2e-tests into its original format where it doesn't run the tekton tests

mgencur · 2019-12-02T09:06:05Z

The "build-tests" check is failing but I'm not sure why. They error message is ERROR: /home/prow/go/src/knative.dev/client is out of date. Please run ./hack/build.sh -c and commit. but I've already rebased and run ./hack/build.sh -c and committed. I'd appreciate some help on this bit.
The last commit on this PR returns the e2e-tests.sh script into the right format which means the tests for Tekton are not run in this CI checks. Results for Tekton can be seen in the previous run (before the last commit was added): https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/528/pull-knative-client-integration-tests/1201417741060280321

mgencur · 2019-12-02T09:10:10Z

The only change I did not commit after running ./hack/build.sh was go 1.13 in go.mod, but I'm not sure it should cause this trouble.

navidshaikh

Thanks a lot @mgencur for putting this together, I've added few comments rest lgtm. We should reference the used tasks from catalog.

test/e2e/tekton_test.go

navidshaikh · 2019-12-02T08:59:33Z

test/e2e/tekton_test.go

+	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/buildah-task.yaml"}, runOpts{})
+	assert.NilError(t, err)
+
+	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/kn-task.yaml"}, runOpts{})


same as for buildah task, lets grab them from catalog

Suggested change

_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/kn-task.yaml"}, runOpts{})

_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", "https://raw.githubusercontent.com/tektoncd/catalog/master/kn/kn.yaml"}, runOpts{})

test/test_images/helloworld/Dockerfile

test/resources/tekton/buildah-task.yaml

test/resources/tekton/kn-pipeline.yaml

navidshaikh · 2019-12-02T10:06:51Z

The "build-tests" check is failing but I'm not sure why.

I think it's because of the golang version used, if you retry(./hack/build.sh) with go 1.12.x you'd see the difference.

test/test_images/helloworld/Dockerfile

test/resources/tekton/kn-pipeline.yaml

test/e2e/tekton_test.go

navidshaikh · 2019-12-02T11:15:51Z

test/e2e/tekton_test.go

+	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/buildah-task.yaml"}, runOpts{})
+	assert.NilError(t, err)
+
+	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/kn-task.yaml"}, runOpts{})


Suggested change

_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/kn-task.yaml"}, runOpts{})

_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", "https://raw.githubusercontent.com/tektoncd/catalog/master/kn/kn.yaml"}, runOpts{})

This reverts commit 2ab272f.

test/tekton-tests.sh

maximilien · 2019-12-04T03:18:38Z

test/resources/tekton/kn-pipeline-run.yaml

+# 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.
+apiVersion: tekton.dev/v1alpha1


Should the version be parametrized?

If the tests fail with a newer version of Tekton then these files will need to be updated at that point and committed. The tests are there for testing regressions with future versions of Tekton / kn. They're not there to test compatibility of multiple versions of Tekton with different apiVersion versions; so IMO adding this as a parameter would not bring any benefits.

This reverts commit 3800adb.

mgencur · 2019-12-04T12:42:21Z

This run shows latest changes: https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/528/pull-knative-client-integration-tests/1202199261303803905 , tests passed. I'll remove the last commit to get it ready for integration.

This reverts commit 71a3d33.

mgencur · 2019-12-04T12:44:14Z

Hey! Thanks for reviews. All comments addressed. Please review the latest changes.

adrcunha

/lgtm

test/e2e/tekton_test.go

test/tekton-tests.sh

test/e2e-common.sh

adrcunha · 2019-12-05T16:26:59Z

test/e2e-smoke-tests.sh

@@ -25,15 +25,7 @@
 # the cluster.

 source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh


Suggested change

source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh

adrcunha · 2019-12-05T16:27:30Z

test/e2e-tests.sh

@@ -29,29 +29,10 @@
 # of this specified version will be installed in the Kubernetes cluster, and
 # all the tests will run against Knative serving of this specific version.
 source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh


Suggested change

source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh

test/e2e-common.sh

adrcunha

/lgtm

navidshaikh

/lgtm
/approve

Thanks @mgencur !

Logged knative/test-infra#1575 to configure running these e2e as nightly and pre-release Job.

knative-prow-robot · 2019-12-06T13:02:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgencur, navidshaikh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [navidshaikh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Addresses knative#527. So it's easy to check them when running E2E tests locally. `e2e-tests.sh` now displays where the artifacts were written once it finishes. For local runs, this is a new temporary directory and will contain both the JUnit XML file and the full go test log.

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 29, 2019

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 29, 2019

knative-prow-robot reviewed Nov 29, 2019

View reviewed changes

knative-prow-robot requested review from cppforlife and mattmoor November 29, 2019 11:10

mgencur force-pushed the tekton_tests_cli branch from 0cb353f to 14640f8 Compare November 29, 2019 12:59

mattmoor removed their request for review December 1, 2019 15:28

Integration tests for Tekton

2d4095f

mgencur force-pushed the tekton_tests_cli branch from f30ae82 to 2d4095f Compare December 2, 2019 08:27

Use knative/client instead of fork

2ab272f

* also put back the e2e-tests into its original format where it doesn't run the tekton tests

mgencur changed the title ~~[WIP] Integration tests for Tekton~~ Integration tests for Tekton Dec 2, 2019

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2019

mgencur mentioned this pull request Dec 2, 2019

Create integration tests for kn task in tektoncd catalog #471

Closed

navidshaikh reviewed Dec 2, 2019

View reviewed changes

Run ./hack/build.sh with Go 1.12

61dc2b6

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 2, 2019

navidshaikh reviewed Dec 2, 2019

View reviewed changes

mgencur added 2 commits December 2, 2019 12:20

Pull buildah and kn tasks directly from catalog

0976ec0

Revert "Use knative/client instead of fork"

f14c210

This reverts commit 2ab272f.

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 2, 2019

duglin reviewed Dec 3, 2019

View reviewed changes

test/tekton-tests.sh Outdated Show resolved Hide resolved

maximilien reviewed Dec 4, 2019

View reviewed changes

Run against arbitrary Docker registry

8a84331

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 4, 2019

mgencur added 2 commits December 4, 2019 13:12

Conditionally install Tekton

86a50d7

Use knative/client instead of fork

71a3d33

This reverts commit 3800adb.

mgencur force-pushed the tekton_tests_cli branch from 84b7a63 to 71a3d33 Compare December 4, 2019 12:13

mgencur added 2 commits December 4, 2019 13:42

Revert "Use knative/client instead of fork"

ecdae2a

This reverts commit 71a3d33.

Simplify passing the flag to test

c998bfa

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 4, 2019

adrcunha reviewed Dec 4, 2019

View reviewed changes

test/e2e/tekton_test.go Outdated Show resolved Hide resolved

test/tekton-tests.sh Outdated Show resolved Hide resolved

test/tekton-tests.sh Show resolved Hide resolved

knative-prow-robot assigned adrcunha Dec 4, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2019

duglin reviewed Dec 4, 2019

View reviewed changes

test/e2e-common.sh Show resolved Hide resolved

Simplify imports

07bede7

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2019

adrcunha reviewed Dec 5, 2019

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2019

navidshaikh approved these changes Dec 6, 2019

View reviewed changes

knative-prow-robot assigned navidshaikh Dec 6, 2019

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2019

knative-prow-robot merged commit 60567a9 into knative:master Dec 6, 2019

creydr pushed a commit to creydr/knative-client that referenced this pull request Nov 25, 2024

[release-v1.15] Update func to latest midstream (knative#528)

e36694b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration tests for Tekton #528

Integration tests for Tekton #528

mgencur commented Nov 29, 2019 •

edited

Loading

knative-prow-robot left a comment

mgencur commented Dec 2, 2019

mgencur commented Dec 2, 2019

navidshaikh left a comment

navidshaikh Dec 2, 2019

navidshaikh Dec 2, 2019

navidshaikh commented Dec 2, 2019

navidshaikh Dec 2, 2019

maximilien Dec 4, 2019

mgencur Dec 4, 2019

mgencur commented Dec 4, 2019

mgencur commented Dec 4, 2019

adrcunha left a comment

adrcunha Dec 5, 2019

adrcunha Dec 5, 2019

adrcunha left a comment

navidshaikh left a comment

knative-prow-robot commented Dec 6, 2019

	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", basedir + "/kn-task.yaml"}, runOpts{})
	_, err = kubectl.RunWithOpts([]string{"apply", "-n", test.env.Namespace, "-f", "https://raw.githubusercontent.com/tektoncd/catalog/master/kn/kn.yaml"}, runOpts{})

		@@ -25,15 +25,7 @@
		# the cluster.

		source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh

Integration tests for Tekton #528

Integration tests for Tekton #528

Conversation

mgencur commented Nov 29, 2019 • edited Loading

Proposed Changes

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

mgencur commented Dec 2, 2019

mgencur commented Dec 2, 2019

navidshaikh left a comment

Choose a reason for hiding this comment

navidshaikh Dec 2, 2019

Choose a reason for hiding this comment

navidshaikh Dec 2, 2019

Choose a reason for hiding this comment

navidshaikh commented Dec 2, 2019

navidshaikh Dec 2, 2019

Choose a reason for hiding this comment

maximilien Dec 4, 2019

Choose a reason for hiding this comment

mgencur Dec 4, 2019

Choose a reason for hiding this comment

mgencur commented Dec 4, 2019

mgencur commented Dec 4, 2019

adrcunha left a comment

Choose a reason for hiding this comment

adrcunha Dec 5, 2019

Choose a reason for hiding this comment

adrcunha Dec 5, 2019

Choose a reason for hiding this comment

adrcunha left a comment

Choose a reason for hiding this comment

navidshaikh left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Dec 6, 2019

mgencur commented Nov 29, 2019 •

edited

Loading