Bug 230006 - [build.webkit.org][ews-build.webkit.org] Only try to download from S3 on the production server
Summary: [build.webkit.org][ews-build.webkit.org] Only try to download from S3 on the ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-07 09:39 PDT by Carlos Alberto Lopez Perez
Modified: 2021-09-13 04:10 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.55 KB, patch)
2021-09-07 09:56 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (4.72 KB, patch)
2021-09-07 10:11 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (5.15 KB, patch)
2021-09-07 10:20 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2021-09-07 09:39:54 PDT
Uploading the built product to S3 is only possible on the official deployment (since it has the keys, etc)
So on tests deployments we need to find a different way of passing the built product between workers.

r269261 implemented this for ews-build.webkit.org and then r269634 implemented it for build.webkit.org

After this patches on a test deployment the product is not uploaded to S3, is only uploaded from the worker to the main buildbot server (master)

However, the worker still tries to download the test product from S3 and only if fails it fall-backs to download it from the master.

This is an issue, because the URL identifiers on S3 are not random, so it can happen that the S3 download on a test deployment doesn't fail but instead downloads the built product from the official deployment

For build.webkit.org the only variable in the URL is the bot name, architecture and svn revision and for ews-build.webkit.org is the same but instead of the svn revision the patch number.

And why is a problem downloading the built product from the official deployment instead of the test one?

There are several problems with this. For example there is no guarantee that on a test deployment the built configurations are the same than on the official one.
So I think is safe to assume that the user would want to consume the built products from her own test deployment.

Also, even when testing the same build configurations, this can also causes issue on testers, see this example:

1. Official GTK build EWS builds patch 437292 on top of 91691c326150 (r282009) <https://ews-build.webkit.org/#/builders/36/builds/43955>
2. Uploads product it to https://s3-us-west-2.amazonaws.com/ews-archives.webkit.org/gtk-x86_64-release/437292.zip
3. Triggers GTK API test EWS
4. Official GTK API test EWS downloads patch 437292 for testing and correctly checkouts 91691c326150 (r282009) <https://ews-build.webkit.org/#/builders/34/builds/42792>

A. Unofficial GTK build EWS builds patch 437292 on top of 1ee2b9feb0a3 (r282020) <https://ews-build.webkit-uat.org/#/builders/35/builds/107607>
B. It don't uploads the product (transfer-to-s3 step is skipped)
C. Triggers GTK WK2 Layout tests EWS
D. Unofficial GTK WK2 Layout test EWS downloads patch 437292 for testing, but it downloads the patch from the official buildbot instead of the one from UAT (See 1.). That patch was built on top of r282009. However, this buildbot checkouts 1ee2b9feb0a3 (r282020) (which was the checkout used by the UAT buildbot GTK builder)
E. So we end with this GTK WK2 Layout tests worker checked out at r282009 testing a patch that was built on top of r282020 <https://ews-build.webkit-uat.org/#/builders/34/builds/41403>

What is the problem with this?

- r282020 removed several failures from the TestExpectations file because it fixed those tests
- But since the built-product we are testing was built on top of r282009 it still don't has the fixes implemented by r282020
- However, on our checkout the failures are removed from the TestExpectations file (because we are checking r282020 instead of r282009).
- This causes unexpected failures on this Unofficial GTK WK2 Layout test EWS run <https://ews-build.webkit-uat.org/#/builders/34/builds/41403>, the tests imported/w3c/web-platform-tests/css/css-color-adjust/rendering/dark-color-scheme/color-scheme-iframe-background-mismatch-opaque-cross-origin.sub.html and imported/w3c/web-platform-tests/css/css-color-adjust/rendering/dark-color-scheme/color-scheme-iframe-background-mismatch-opaque.html are failing because the built-product tested doesn't match the checked-out base version.
Comment 1 Carlos Alberto Lopez Perez 2021-09-07 09:56:10 PDT
Created attachment 437517 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2021-09-07 10:11:56 PDT
Created attachment 437521 [details]
Patch
Comment 3 Carlos Alberto Lopez Perez 2021-09-07 10:20:56 PDT
Created attachment 437525 [details]
Patch

v2: Add an unit test
Comment 4 Aakash Jain 2021-09-07 10:50:07 PDT
Comment on attachment 437525 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437525&action=review

> Tools/CISupport/build-webkit-org/steps.py:412
> +        self.results = SKIPPED

Ditto. This line isn't needed.

> Tools/CISupport/ews-build/steps.py:2806
> +        self.results = SKIPPED

This line isn't needed. Better to remove it. I tested in https://ews-build.webkit-uat.org/#/builders/34/builds/41512 that it works without this line.
Comment 5 Carlos Alberto Lopez Perez 2021-09-07 11:19:00 PDT
Committed r282100 (241398@main): <https://commits.webkit.org/241398@main>
Comment 6 Radar WebKit Bug Importer 2021-09-07 11:19:18 PDT
<rdar://problem/82831029>
Comment 7 Carlos Alberto Lopez Perez 2021-09-13 04:10:39 PDT
(In reply to Carlos Alberto Lopez Perez from comment #0)
> Uploading the built product to S3 is only possible on the official
> deployment (since it has the keys, etc)
> So on tests deployments we need to find a different way of passing the built
> product between workers.
> 
> r269261 implemented this for ews-build.webkit.org and then r269634
> implemented it for build.webkit.org

   ^^^  just a minor edit, above should say: "" r269248 implemented this for ews-build.webkit.org ""