fix bunch of issues in prow test #866
Conversation
| @@ -29,6 +29,3 @@ remote_code_archive_uri="${code_archive_prefix}_${PULL_BASE_SHA}_${date_string}. | |||
|
|
|||
| tar -czf "$local_code_archive_file" . | |||
| gsutil cp "$local_code_archive_file" "$remote_code_archive_uri" | |||
|
|
|||
There was a problem hiding this comment.
Any reason why we move these two variables out of the test-prep script
There was a problem hiding this comment.
i was having a hard time trace those environment variable as they are spread across multiple files. It's better to locate them in the same top level script.
There was a problem hiding this comment.
In fact, that is why I tried to move all shared env variable in the test_prep script, which also reduced code duplication.
There was a problem hiding this comment.
Shouldn't the environment variable be determined by the caller? how is it different from other env variables in the top level script?
There was a problem hiding this comment.
For env variables, yes. I mean, local variables such as PROJECT and GCR_IMAGE_BASE_DIR are not from the caller. TEST_CLUSTER_PREFIX and TEST_CLUSTER are also local variables that are used by these test scripts.
Above all, there are env variables that are from the caller and there are local variables that presubmit and postsubmit creates. My point was to move these shared local variables in the same script for better reference.
There was a problem hiding this comment.
I moved them to deploy-kubeflow.sh since it's only used there.
The variable passing in these scripts start to become a bit casual. Given the complexity of the test script I think we might also consider switching to go/python based binary.
There was a problem hiding this comment.
SG. Let's create an issue to keep track: #875
|
/test kubeflow-pipeline-e2e-test |
1 similar comment
|
/test kubeflow-pipeline-e2e-test |
|
fix #865 |
IronPan
changed the title
|
/assign @hongye-sun @gaoning777 |
|
/test kubeflow-pipeline-e2e-test |
|
The tests failed due to legit issue building e2e test image |
|
/test kubeflow-pipeline-e2e-test |
|
/test kubeflow-pipeline-sample-test |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan 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:
Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan 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:
Approvers can indicate their approval by writing |
* update tests * explicit return successful * fix * move variable * Update deploy-kubeflow.sh
…#866) * Add tekton tasks for eks-cluster-create/teardown and kaniko * * increase default k8s version of eks-cluster-create task * add sample file for aws-credentials secret * rerun make optional-generate and make optional-test
* Fix Tekton conditional dependency on sub-dag * fix tekton conditional dependency on group ops
set -ex to avoid swallowing the errors silently
This change is