Extend catalog test script to run tests for different platforms #797

barthy1 · 2021-08-04T16:13:07Z

Changes

PLATFORM env variable is introduced. This variable allows to run tests for specific platform (for instance, "linux/s390x" or "linux/ppc64le").
If the environment variable is specified:

some tests are updated with different image values, required for specific platform.
Image names should be specified in script file, separate for each platform. In the current code script file for linux/s390x is proposed.
only tasks, which have mention about corresponding platform support in the annotation, will be tested.
If PLATFORM env variable is not specified, the behaviour of test scripts will be similar to the one before the PR.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Follows the authoring recommendations
Includes docs (if user facing)
Includes tests (if functionality of task changed or new task added)
Commit messages follow commit message best practices
Complies with Catalog Organization TEP, see example. Note An issue has been filed to automate this validation
- File path follows <kind>/<name>/<version>/name.yaml
- Has README.md at <kind>/<name>/<version>/README.md
- Has mandatory metadata.labels - app.kubernetes.io/version the same as the <version> of the resource
- Has mandatory metadata.annotations tekton.dev/pipelines.minVersion
- mandatory spec.description follows the convention
```
  ```
  
    spec:
      description: >-
        one line summary of the resource
  
        Paragraph(s) to describe the resource.
  ```
```

See the contribution guide
for more details.

barthy1 · 2021-08-04T16:50:13Z

/test pull-tekton-catalog-integration-tests

barthy1 · 2021-08-05T07:06:30Z

/test pull-tekton-catalog-integration-tests

barthy1 · 2021-10-14T12:24:10Z

/test pull-tekton-catalog-integration-tests

chmouel · 2021-10-14T12:23:06Z

test/e2e-common.sh

+	set -o allexport
+	source $(dirname $0)/$(echo ${PLATFORM}| tr / -).envs
+	set +o allexport
+	# Install yq
+	echo "Getting yq"
+	wget https://github.com/mikefarah/yq/releases/download/v4.11.2/yq_linux_amd64
+	chmod +x yq_linux_amd64
+	mv yq_linux_amd64 /bin/yq
+	# Add MAVEN_IMAGE parameter to the appropriate maven tests
+	echo "Add extra MAVEN_IMAGE parameter"
+	find task/*maven*/*/tests/run.yaml | xargs -I{} yq eval '(..|select(.kind?=="Pipeline")|select(.metadata.name?=="jib-maven-test-pipeline"|"maven-test-pipeline")|.spec.tasks[1].params) |= . +{"name": "MAVEN_IMAGE","value": env(MAVEN_IMAGE)}' -i {}
+	# Replace registry image to platform specific
+	echo "Change registry image value"
+	find task -name *registry*.yaml | xargs -I{} yq eval '(..|select(.kind?=="Deployment")|select(.metadata.name?=="registry")|.spec.template.spec.containers[0].image)|= env(REGISTRY_IMAGE)' -i {}
+	# change GOARCH parameter value to the platform specific one for golang-* tasks
+	echo "Change GOARCH parameter value"
+	find task/golang*/*/tests/run.yaml | xargs -I{} yq eval '(..|select(.kind?=="Pipeline")|.spec.tasks[1].params) |= . +{"name": "GOARCH","value": env(GOARCH)}' -i {}


I like the idea of having platform specific env to being able to manipulate the task on demand but this seems a bit too specific and I am not sure if that would be scalable.

what about if those commands are just platform specifics, where basically moving all those lines to the .env file, so it only applies to the s390x platform ?

Hmm, interesting idea. I had in mind that the same modification will be required for other platforms (just because, I participated in test work also for Power, for example), but yeah, may be extra bash scripts, specific for each platform is even better and will provide much more flexibility. Thank you for idea, will work in it!
Just to make sure we are on the same page, you are ok regarding using yq tool for modifications?

yeah no worries! we just need to make sure it's in the base image!

(tbh: we should rework all of this e2e test suite in a python or golang but another fight for another day i guess)

we just need to make sure it's in the base image!

At this moment it is installed in the script ) I've just copied approach, which dashboard component has..
Let me check if I can add it to test-runner image

@chmouel yq is added to the test-runner image - tektoncd/plumbing#909
Could you please review this PR one more time?

chmouel

so far so good thanks, i have added some minor comments

chmouel · 2021-11-10T15:38:58Z

test/e2e-common.sh

+
+# Do the tasks modifications if special PLATFORM value is specified. By default nothing happens.
+if [[ -n ${PLATFORM} ]] && [[ -f "$(dirname $0)/$(echo ${PLATFORM}| tr / -).sh" ]]; then
+        # Load script specific to platform. File name should follow the pattern "os-arch.sh", for instance "linux-s390x.sh".


chmouel · 2021-11-10T15:40:07Z

test/e2e-common.sh

@@ -166,6 +175,10 @@ function test_yaml_can_install() {
        for ignore in ${TEST_YAML_IGNORES};do
            [[ ${ignore} == "${testname}" ]] && skipit=True
        done
+
+        # In case if PLATFORM env variable is specified, do the tests only for matching tasks
+        [[ -n ${PLATFORM} ]] && [[ $(grep "tekton.dev/platforms" ${runtest}) != *"${PLATFORM}"* ]]  && skipit=True


maybe redirect stderr to /dev/null since it may fails (but the case is handled in skipit)

@chmouel done

PLATFORM env variable is introduced. This variable allows to run tests for specific platform (for insance, "linux/s390x" or "linux/ppc64le"). If the environment variable is specified - some tests are updated with different image values/other parameter values, required for specific platform, using yq tool. Custom values should be specified in the script file, separate for each platform. In the current code script file for linux/s390x is proposed. - only tasks, which have mention about corresponding platform support in the annotation, will be tested. If PLATFORM env variale is no specified, the behaviour of test scripts will be similar to the one before the PR. Signed-off-by: Yulia Gaponenko <yulia.gaponenko1@de.ibm.com>

chmouel · 2021-11-11T15:18:38Z

/lgtm
thank you!

tekton-robot · 2021-11-12T04:37:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vinamra28

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~~ [vinamra28]

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

tekton-robot requested review from bobcatfish and dlorenc August 4, 2021 16:13

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 4, 2021

barthy1 force-pushed the platform_tests branch 2 times, most recently from fe91c3c to d3fb363 Compare October 14, 2021 12:10

chmouel reviewed Oct 14, 2021

View reviewed changes

barthy1 force-pushed the platform_tests branch from d3fb363 to 9624c2e Compare October 14, 2021 15:52

barthy1 force-pushed the platform_tests branch from 9624c2e to 47d92f2 Compare October 28, 2021 16:46

barthy1 force-pushed the platform_tests branch from 47d92f2 to b6617eb Compare November 10, 2021 10:09

chmouel reviewed Nov 10, 2021

View reviewed changes

barthy1 force-pushed the platform_tests branch from b6617eb to c648281 Compare November 11, 2021 13:29

tekton-robot assigned chmouel Nov 11, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2021

vinamra28 approved these changes Nov 12, 2021

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 12, 2021

tekton-robot merged commit cab0b76 into tektoncd:main Nov 12, 2021

svghadi mentioned this pull request Dec 30, 2021

Add test script to extend support for testing on ppc64le platform #886

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend catalog test script to run tests for different platforms #797

Extend catalog test script to run tests for different platforms #797

barthy1 commented Aug 4, 2021 •

edited

barthy1 commented Aug 4, 2021

barthy1 commented Aug 5, 2021

barthy1 commented Oct 14, 2021

chmouel Oct 14, 2021

barthy1 Oct 14, 2021

chmouel Oct 14, 2021

barthy1 Oct 14, 2021

barthy1 Oct 29, 2021

chmouel left a comment

chmouel Nov 10, 2021

chmouel Nov 10, 2021

barthy1 Nov 11, 2021

chmouel commented Nov 11, 2021

tekton-robot commented Nov 12, 2021

Extend catalog test script to run tests for different platforms #797

Extend catalog test script to run tests for different platforms #797

Conversation

barthy1 commented Aug 4, 2021 • edited

Changes

Submitter Checklist

barthy1 commented Aug 4, 2021

barthy1 commented Aug 5, 2021

barthy1 commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chmouel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chmouel commented Nov 11, 2021

tekton-robot commented Nov 12, 2021

barthy1 commented Aug 4, 2021 •

edited