Tests - Use base image for frontend tests #190

Ark-kun · 2018-11-10T01:58:14Z

This change moves the unchanging parts of the image to a base image so that they're not rebuilt every time. This improves performance and reliability.

The Dockerfile is now just

FROM gcr.io/ml-pipeline-test/selenium-standalone-chrome-gcloud-nodejs
COPY --chown=seluser . /src
WORKDIR /src
ENTRYPOINT [ "./run_test.sh" ]

This change is

coveralls · 2018-11-10T02:02:43Z

Pull Request Test Coverage Report for Build 207

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 67.592%

Totals
Change from base Build 185:	0.0%
Covered Lines:	1535
Relevant Lines:	2179

💛 - Coveralls

coveralls · 2018-11-10T02:02:43Z

Pull Request Test Coverage Report for Build 290

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 67.592%

Totals
Change from base Build 185:	0.0%
Covered Lines:	1535
Relevant Lines:	2179

💛 - Coveralls

IronPan · 2018-11-10T06:41:42Z

/test presubmit-e2e-test

yebrahim · 2018-11-13T03:47:08Z

How much time does this actually save? Looks to me like too many lines of code if they just save us a few seconds of test time.

Ark-kun · 2018-11-16T20:16:55Z

How much time does this actually save? Looks to me like too many lines of code if they just save us a few seconds of test time.

This WIP, so there are some extra lines of code that should be gone before review.
This should save 90% of image building time
This should reduce test flakiness by not downloading from Internet during build.
This would allow us to use local image building tools like Bazel. These tools support adding files and packages to the image, but not RUNning programs.

Ark-kun · 2018-11-26T01:26:18Z

How much time does this actually save? Looks to me like too many lines of code if they just save us a few seconds of test time.

This is just basic prudence. Do not download/clone when you do not need to. Apart from big decrease in image build times, this reduces the failures due to temporary network problems and also saves us from completely broken check-in process.

Here is how the complete test failure looks like:

build-frontend-image:	Step 12/20 : FROM node:9.4.0-alpine
build-frontend-image:	9.4.0-alpine: Pulling from library/node
build-frontend-image:	error parsing HTTP 404 response body: invalid character 'p' after top-level value: "404 page not found\n"

So at this moment no code can be checked in due to failing test.
@yebrahim Would you like to fix that?

Ark-kun · 2018-11-26T01:37:32Z

/test presubmit-e2e-test

yebrahim · 2018-11-26T09:21:19Z

Doing docker pull node:9.4.0-alpine works for me; the image is still out there. We need to update it to use the lts version, but it's not breaking tests yet as far as I can see.

rileyjbauer · 2019-06-05T05:26:19Z

/lgtm

yebrahim

LGTM, just two minor comments. Approving in case you want to merge.
/approve

test/frontend-integration-test/selenium-standalone-chrome-gcloud-nodejs.Docker/Dockerfile

k8s-ci-robot · 2019-06-05T19:57:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yebrahim

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:

~~test/frontend-integration-test/OWNERS~~ [yebrahim]

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

k8s-ci-robot · 2019-06-18T13:40:42Z

@Ark-kun: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
presubmit-e2e-test	`a8317f5`	link	`/test presubmit-e2e-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

The ml-pipeline-staging bucker is inaccessible by the image-builder container.

See https://github.com/GoogleCloudPlatform/cloud-sdk-docker/blob/master/debian_slim/Dockerfile

For some reason the root, not the active user owns the data copied using COPY

Cleaning files before committing the layer. Installing pip using apt-get (it's no longer in python-setuptools)

Bobgy · 2019-10-15T03:47:38Z

/area front-end

Bobgy · 2019-10-15T03:50:00Z

@Ark-kun what's your plan about this PR?

Ark-kun · 2019-10-22T23:58:07Z

@Ark-kun what's your plan about this PR?

As the Frontend area lead, you can decide whether to close it or merge.

This change was designed to improve the build speed and stability. The build speed is now pretty good even without this PR. The stability aspect remains valid.

Bobgy · 2019-10-23T02:11:35Z

/retest
/lgtm

Bobgy · 2019-10-23T02:12:31Z

I think this makes code base cleaner in separating env setup with actual test steps. Also great work automating image building. Let's merge this as long as tests are still passing.

- Add installation from PyPi to sdk/README.md - Modify setup.py to read long_description from sdk/README.md - Replace relative links with absolute links to kfp-tekton GitHub repo - Add make distribution target for upload to testpypi Resolves kubeflow#185

k8s-ci-robot added do-not-merge/work-in-progress labels Nov 10, 2018

k8s-ci-robot requested review from rileyjbauer and yebrahim November 10, 2018 01:58

k8s-ci-robot added size/M labels Nov 10, 2018

k8s-ci-robot added size/L and removed size/M labels Nov 15, 2018

Ark-kun force-pushed the avolkov/Tests---Use-base-image-for-frontend-tests branch 3 times, most recently from 5929dea to 0309015 Compare December 1, 2018 07:10

k8s-ci-robot added size/M and removed size/L labels Dec 1, 2018

Ark-kun force-pushed the avolkov/Tests---Use-base-image-for-frontend-tests branch 2 times, most recently from a009496 to 8bf6df8 Compare December 1, 2018 23:06

k8s-ci-robot added size/L and removed size/M labels Dec 1, 2018

k8s-ci-robot assigned rileyjbauer Jun 5, 2019

k8s-ci-robot added the lgtm label Jun 5, 2019

yebrahim reviewed Jun 5, 2019

View reviewed changes

test/frontend-integration-test/selenium-standalone-chrome-gcloud-nodejs.Docker/Dockerfile Outdated Show resolved Hide resolved

test/frontend-integration-test/selenium-standalone-chrome-gcloud-nodejs.Docker/Dockerfile Outdated Show resolved Hide resolved

k8s-ci-robot added the approved label Jun 5, 2019

Ark-kun added 10 commits June 18, 2019 18:38

Tests - Use base image for frontend tests

bf343fd

Moved the base image to ml-pipeline-test bucket.

5108a2e

The ml-pipeline-staging bucker is inaccessible by the image-builder container.

Moved the gcloud installation to the base image

b616b04

Setting correct user for /src

7d8d73a

Trying to chown the directory using a different technique.

d2cc06d

GCP SDK is now in /tools/

eb42c21

Using the official gcloud install method

eb36096

See https://github.com/GoogleCloudPlatform/cloud-sdk-docker/blob/master/debian_slim/Dockerfile

Running gcloud only under the seluser user

0f4a05f

COPY --chown=seluser

2d40ae6

For some reason the root, not the active user owns the data copied using COPY

No need to run apt-get update again.

84e6a89

Cleaning files before committing the layer. Installing pip using apt-get (it's no longer in python-setuptools)

Ark-kun force-pushed the avolkov/Tests---Use-base-image-for-frontend-tests branch from 5abe22c to 84e6a89 Compare June 19, 2019 02:00

k8s-ci-robot removed the lgtm label Jun 19, 2019

k8s-ci-robot added the area/frontend label Oct 15, 2019

k8s-ci-robot assigned Bobgy Oct 23, 2019

k8s-ci-robot added the lgtm label Oct 23, 2019

k8s-ci-robot merged commit 1c566f8 into master Oct 23, 2019

IronPan deleted the avolkov/Tests---Use-base-image-for-frontend-tests branch October 30, 2019 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests - Use base image for frontend tests #190

Tests - Use base image for frontend tests #190

Ark-kun commented Nov 10, 2018 •

edited

Loading

coveralls commented Nov 10, 2018 •

edited

Loading

coveralls commented Nov 10, 2018

IronPan commented Nov 10, 2018

yebrahim commented Nov 13, 2018

Ark-kun commented Nov 16, 2018

Ark-kun commented Nov 26, 2018

Ark-kun commented Nov 26, 2018

yebrahim commented Nov 26, 2018

rileyjbauer commented Jun 5, 2019

yebrahim left a comment

k8s-ci-robot commented Jun 5, 2019

k8s-ci-robot commented Jun 18, 2019 •

edited

Loading

Bobgy commented Oct 15, 2019

Bobgy commented Oct 15, 2019

Ark-kun commented Oct 22, 2019 •

edited

Loading

Bobgy commented Oct 23, 2019

Bobgy commented Oct 23, 2019

Tests - Use base image for frontend tests #190

Tests - Use base image for frontend tests #190

Conversation

Ark-kun commented Nov 10, 2018 • edited Loading

coveralls commented Nov 10, 2018 • edited Loading

Pull Request Test Coverage Report for Build 207

💛 - Coveralls

coveralls commented Nov 10, 2018

Pull Request Test Coverage Report for Build 290

💛 - Coveralls

IronPan commented Nov 10, 2018

yebrahim commented Nov 13, 2018

Ark-kun commented Nov 16, 2018

Ark-kun commented Nov 26, 2018

Ark-kun commented Nov 26, 2018

yebrahim commented Nov 26, 2018

rileyjbauer commented Jun 5, 2019

yebrahim left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 5, 2019

k8s-ci-robot commented Jun 18, 2019 • edited Loading

Bobgy commented Oct 15, 2019

Bobgy commented Oct 15, 2019

Ark-kun commented Oct 22, 2019 • edited Loading

Bobgy commented Oct 23, 2019

Bobgy commented Oct 23, 2019

Ark-kun commented Nov 10, 2018 •

edited

Loading

coveralls commented Nov 10, 2018 •

edited

Loading

k8s-ci-robot commented Jun 18, 2019 •

edited

Loading

Ark-kun commented Oct 22, 2019 •

edited

Loading