Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tests - Fixed many test issues (external PRs, code de-sync) by getting rid of git clone #315

Merged
merged 4 commits into from
Nov 26, 2018
Merged

Tests - Fixed many test issues (external PRs, code de-sync) by getting rid of git clone #315

merged 4 commits into from
Nov 26, 2018

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Nov 19, 2018 •
edited
Loading

This change fixes many test related problems we have now i.e. tests for external PRs and test flakyness due to code versions de-sync.

Fixes #163 #198 #200 #368 #196 #215

  • run_test.sh scripts no longer clones the repo code, because the code is now correctly baked in during the image build. This saves us 11 clones
  • all images are now build from archived version of code the source code prepared by Prow. This saves 25 more clones and improves test reliability

P.S. There still can be some code de-sync due to build-images and tests running on different code. The only way to fully avoid this is to build images as part of each separate test suite.

This change is Reviewable

@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch 2 times, most recently from cbd1cd4 to f3c6b74 Compare November 19, 2018 22:12
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 19, 2018

/test build-image

@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch 4 times, most recently from 6e1cce4 to 1822bcd Compare November 19, 2018 23:36
@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch from 1822bcd to 9d7b084 Compare November 20, 2018 01:18
@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch 4 times, most recently from f68e232 to 65728d0 Compare November 20, 2018 02:50
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 20, 2018

/test presubmit-e2e-test

@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch 6 times, most recently from 8a98c62 to b7c8af0 Compare November 21, 2018 04:04
@Ark-kun
Build all images from source code prepared by Prow. Got rid of git pulls
6e44c03 
All images are now built from archived version of code the source code prepared by Prow.
This saves 25 more pulls and improves test reliability.
The archived source code location is passed through image-build-context-gcs-uri parameter.
@Ark-kun Ark-kun force-pushed the avolkov/Tests---test-source-code-archive-upload-download-extract branch from a13408c to 6e44c03 Compare November 23, 2018 07:17
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 23, 2018

/test presubmit-e2e-test

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 23, 2018

All tests passed including sample-test.

@Ark-kun Ark-kun changed the title [WIP] Tests - Fixed many test issues by getting rid of git clone Tests - Fixed many test issues by getting rid of git clone Nov 23, 2018
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 23, 2018 •
edited
Loading

@IronPan Can you please check this PR. It's pretty easy if you check the three individual commits here: https://github.com/kubeflow/pipelines/pull/315/commits

@Ark-kun Ark-kun changed the title Tests - Fixed many test issues by getting rid of git clone Tests - Fixed many test issues (external PRs, code de-sync) by getting rid of git clone Nov 23, 2018
IronPan
IronPan reviewed Nov 23, 2018
View reviewed changes
test/api-integration-test/run_test.sh Outdated Show resolved Hide resolved
test/imagebuilder/build.sh Show resolved Hide resolved
test/presubmit-tests.sh Show resolved Hide resolved
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 25, 2018

/retest

1 similar comment
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 25, 2018

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 26, 2018

/test build-image

@IronPan
Copy link
Member

IronPan commented Nov 26, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@IronPan
Copy link
Member

IronPan commented Nov 26, 2018

pls check the test failures

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Nov 26, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit bcbb2c5 into master Nov 26, 2018
@Ark-kun Ark-kun deleted the avolkov/Tests---test-source-code-archive-upload-download-extract branch November 28, 2018 17:36
This was referenced Mar 20, 2019
Closed
Closed
Closed
Linchin pushed a commit to Linchin/pipelines that referenced this pull request Apr 11, 2023
@jlewi @k8s-ci-robot
Fix repo spec for 0.4 auto-deploy cron job. (kubeflow#327)
4ff2a77 
* The v0.4-branch should be specified on the kubeflow/kubeflow repo
  not the testing repo.

Related to kubeflow#315
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
@rakelkar @k8s-ci-robot
Remove default azure secret name (kubeflow#315)
f31141e 
* add readme for azure

* fallback if the well known az secret is not found

* remove azure config name

* remove azure config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IronPan IronPan IronPan left review comments

@vicaire vicaire Awaiting requested review from vicaire

Assignees

@IronPan IronPan

Labels
approved lgtm priority/p0 size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Our test code and test images code is not always the same.
3 participants
@Ark-kun @IronPan @k8s-ci-robot