New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 601022 link

Starred by 1 user
Status: Fixed
Owner:
no...@chromium.org
Closed: Jun 2016
Cc:
█ phajdan.jr@chromium.org
vadimsh@chromium.org
mar...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 600876
issue 601017
issue 601018
issue 601020
Blocking:
issue 613424



Sign in to add a comment

swarming: add cipd support

Project Member Reported by no...@chromium.org, Apr 6 2016 
Extend Task Properties in API, datastore and bot manifest with 

repeated CipdPackage packages;

message CipdPackage {
  string name = 1;
  string version = 2;
  // where to install
  string path = 3;
}

the user is responsible to make sure that task.idempotent is not set to true if any cipd package version is not pinned.

--

On the bot, install cipd client and the packages if manifest specifies packages.

Comment 1 by no...@chromium.org, Apr 6 2016

Blockedon: 600876 601017 601018
Cc: phajdan.jr@chromium.org

Comment 2 by mar...@chromium.org, Apr 13 2016 

> the user is responsible to make sure that task.idempotent is not set to true if any cipd package version is not pinned.

This can be enforced server side.

Comment 3 by no...@chromium.org, Apr 13 2016 

I wrote that because swarming trusts the value of "idempotent" today, so it can keep on trusting the user value without complicating the implementation.

Comment 4 by no...@chromium.org, Apr 13 2016 

Chatted with maruel. I'll add a check that if idempotent is set to true and a package version is not specified or does not look like a hash or tag, then it is an error.

Comment 5 by estaab@google.com, Apr 14 2016 

I thought when I spoke with vadimsh about this the idea was the resolve any refs to an immutable version identifier before storing cipd package information with the task so that the task can still be reproduced (and could still be idempotent).

Comment 6 by no...@chromium.org, Apr 14 2016 

Yeah, that's possible too. It will somewhat increase swarming latency but that's probably ok.

So I assume this must happen only if idempotent=true?

Comment 7 by mar...@chromium.org, Apr 14 2016 

Then this should be done by the client, not the server. The server could enforce that only digests are allowed as cipd packages, which doesn't require talking to an external service.

Comment 8 by no...@chromium.org, Apr 14 2016 

Chatted with maruel regarding filename clashes when both isolate and cipd are used together (we want to support this case).

We considered adding "path" parameter to isolate to specify where to extract an isolate, but maruel raised a concern that it may complicate reproducing a task locally when both isolate and packages are used.

We agreed on a different scheme: extract cipd packages to some undefined dir, but guarantee that the installed binaries are be available in $PATH, so binaries can be referred in scripts just by their base name. It is easier to implement and more convenient for users.

Comment 9 by estaab@google.com, Apr 14 2016 

Ahh, I really don't like that. If you're going to modify the environment can you use a new variable? What's wrong with a path with a root that can be overridden locally but gets set to some random temp dir on swarming?

What if the cipd package is just data and isn't intended to be executed?

(sorry these thoughts are out of order, I don't have much time)

Comment 10 by mar...@chromium.org, Apr 14 2016 

isolate was designed to be able to run something locally the same way it's run on the bot. And vice-versa.

With cipd, it's akin having the programs installed. This is *effectively* the programs being in the path.

I think it's very natural.

I don't know about CIPD data-only packages, that's an excellent question. What use cases do you see?

Comment 11 by no...@chromium.org, Apr 14 2016 

OK, what about
- pick some dir, install cipd packages there and set $CIPD_PATH to it
- prepend $CIPD_PATH to $PATH

It is same as #8, but we provide $CIPD_PATH

Comment 12 by estaab@chromium.org, Apr 18 2016 

That's probably good enough, I don't want to hold this up anymore so go ahead with that.

Comment 13 by aga...@chromium.org, Apr 27 2016

Components: Infra>Platform
Labels: -Infra-Platform
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/1c6c3d329a42a889001c88214fc7082e0f60c1d9

commit 1c6c3d329a42a889001c88214fc7082e0f60c1d9
Author: nodir <nodir@chromium.org>
Date: Wed Apr 27 19:31:50 2016

swarming: add support for cipd on the server side

- add CipdPackage ndb and rpc types
- add "packages" field to TaskProperties in ndb and rpc
- validate packages before putting task properties
- if idempotent=true, ensure that package versions are hashes
- include packages in bot manifest

R=maruel@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/1910713002

[add] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/cipd.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/handlers_bot_test.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/handlers_endpoints_test.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/message_conversion.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/server/task_request.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/swarming_rpcs.py
[modify] https://crrev.com/1c6c3d329a42a889001c88214fc7082e0f60c1d9/appengine/swarming/test_env_handlers.py

Comment 15 by no...@chromium.org, Apr 29 2016 

I've moved the design discussion to go/swarming-cipd
Project Member

Comment 17 by bugdroid1@chromium.org, May 3 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/ff263d1538f3434f1d92ee14c22de87beab596e4

commit ff263d1538f3434f1d92ee14c22de87beab596e4
Author: nodir <nodir@chromium.org>
Date: Tue May 03 16:39:57 2016

run_isolated: support non-isolated commands

Make --isolated option optional:
* if not specified, treat args as a command, so run_isolated.py becomes a
  wrapper
* --isolate-server option is required only if --isolated is specified or an
  arg contains ${ISOLATED_OUTDIR}
* even if input isolated hash is not provided, we still upload output files
  if any, so name "run_isolated" still makes sense

R=maruel@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/1932143003

[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/isolate.py
[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/isolateserver.py
[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/run_isolated.py
[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/swarming.py
[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/tests/run_isolated_test.py
[modify] https://crrev.com/ff263d1538f3434f1d92ee14c22de87beab596e4/client/tools/parallel_execution.py
Project Member

Comment 18 by bugdroid1@chromium.org, May 3 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/0a8232ec95d18fda544896c0b7f348f09d7db4a8

commit 0a8232ec95d18fda544896c0b7f348f09d7db4a8
Author: nodir <nodir@chromium.org>
Date: Tue May 03 17:20:32 2016

run_isolated: close storage

Forgot to close storage on exit in a prev CL

R=maruel@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/1942403002

[modify] https://crrev.com/0a8232ec95d18fda544896c0b7f348f09d7db4a8/client/run_isolated.py
Project Member

Comment 19 by bugdroid1@chromium.org, May 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/abdaf29b948fa7467b5f1fdaed3bfbc41a692145

commit abdaf29b948fa7467b5f1fdaed3bfbc41a692145
Author: nodir <nodir@chromium.org>
Date: Thu May 12 23:08:56 2016

swarming: change meaning of inputs_ref

Currently inputs_ref implicitly specifies isolated server and namespace
for isolated outputs. The problem is that a user may need to be able to specify
them for non-isolated command.

Change the meaning of inputs_ref from "a reference to input file tree"
to "isolate server/namespace and hash of input file tree".
Now (inputs_ref is not None) does not mean there is isolated input,
but (inputs_ref and inputs_ref.isolated) does.

In bot manifest, change "inputs_ref" to "isolated" that contains
input/output settings.

Add a TODO to refactor the code and update datastore schema.

Update tests.

R=maruel@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/1939343002

[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/handlers_bot_test.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/server/task_request.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/swarming_bot/bot_code/task_runner.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/swarming_bot/bot_code/task_runner_test.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/appengine/swarming/templates/user_task.html
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/client/swarming.py
[modify] https://crrev.com/abdaf29b948fa7467b5f1fdaed3bfbc41a692145/client/tests/swarming_test.py
Project Member

Comment 21 by bugdroid1@chromium.org, May 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/a93d4080fc3c1ef9873174631559979c5e4d446d

commit a93d4080fc3c1ef9873174631559979c5e4d446d
Author: nodir <nodir@chromium.org>
Date: Sat May 14 01:56:16 2016

swarming: refactor cipd input

Add cipd server settings that specify cipd server host and cipd client package.

Refactor "packages" field in task properties and manifest to "cipd_input"
message that contains the packages and cipd settings.

Allow parameters in CipdPackage.package_name. A bot will provide their values.

This CL changes TaskProperties schema in a backward incompatible way, but we never used "packages" datastore
field so far, so it is not a problem.

R=maruel@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/1946253003

[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/components/components/config/validation.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/cipd.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/handlers_bot_test.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/handlers_endpoints.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/handlers_endpoints_test.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/message_conversion.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/proto/config.proto
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/proto/config_pb2.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/server/config.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/server/config_test.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/server/task_request.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/swarming_rpcs.py
[modify] https://crrev.com/a93d4080fc3c1ef9873174631559979c5e4d446d/appengine/swarming/test_env_handlers.py
Project Member

Comment 22 by bugdroid1@chromium.org, May 14 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config.git/+/1d4ad2612f1e49afa8b8f926d3a62d278e15da62

commit 1d4ad2612f1e49afa8b8f926d3a62d278e15da62
Author: Nodir Turakulov <nodir@google.com>
Date: Sat May 14 02:28:03 2016
Project Member

Comment 23 by bugdroid1@chromium.org, May 19 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config.git/+/f05bfca3cbb894a70ea5e7bd6c9e34998b471f68

commit f05bfca3cbb894a70ea5e7bd6c9e34998b471f68
Author: Nodir Turakulov <nodir@google.com>
Date: Thu May 19 18:50:02 2016
Project Member

Comment 24 by bugdroid1@chromium.org, May 19 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal.git/+/c8093c21b1a96d2ab18671a93b058790beba463a

commit c8093c21b1a96d2ab18671a93b058790beba463a
Author: nodir <nodir@google.com>
Date: Thu May 19 19:49:28 2016

Comment 25 by no...@chromium.org, May 20 2016

Components: -Infra>Platform Infra>Platform>Swarming
Labels: M-53

Comment 26 by no...@chromium.org, May 20 2016

Blocking: 613424

Comment 27 by no...@chromium.org, May 20 2016

Labels: luci
Project Member

Comment 29 by bugdroid1@chromium.org, Jun 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/45eab10a17a55f1978f3bfa02c35457dee1c64e4

commit 45eab10a17a55f1978f3bfa02c35457dee1c64e4
Author: nodir <nodir@chromium.org>
Date: Thu Jun 09 22:51:51 2016

run_isolated.py: install CIPD packages

add cipd options to run_isolated.py:
--cipd-package: package name and version, both required. Can be specified multiple times
--cipd-server: URL of CIPD server
--cipd-client-package: CIPD client package to install
--cipd-cache: cache dir for CIPD clients, client versions and packages

expand ${platform} and ${os_ver} parameters in package name with values
read from current machine.

expand ${CIPD_PACKAGE} and ${EXECUTABLE_SUFFIX} parameters on command line with
path to CIPD package installation dir and (.exe|) suffix.

cipd client installation:
- lookup client version (typically a git_revision tag) in version cache
  (typically DiskCache)
- on cache miss, resolve version to instance id using CIPD backend API
- lookup client instance id in client cache (DiskCache)
- on cache miss, fetch it from cipd backend / GS, put to disk cache and
  make executable

package installation:
- put list of packages to a file
- use cipd-ensure to install packages
- pass -verbose and log all cipd-ensure output: stdout->INFO,
  stderr->DEBUGpass -verbose and log all cipd-ensure output:
  stdout->INFO, stderr->DEBUG. Typically there is only stderr
- set -cache-dir flag to "<cipd-cache flag value>/packages"

R=maruel@chromium.org, vadimsh@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/2037253002

[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/appengine/swarming/cipd.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/appengine/swarming/server/bot_archive.py
[add] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/cipd.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/isolateserver.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/run_isolated.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/swarming.py
[add] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/cipdserver_mock.py
[add] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/httpserver_mock.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/isolateserver_mock.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/run_isolated_test.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/subprocess42_test.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/tests/swarming_test.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/utils/file_path.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/utils/subprocess42.py
[modify] https://crrev.com/45eab10a17a55f1978f3bfa02c35457dee1c64e4/client/utils/tools.py
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/67813a2b5c1569fca9644e02680c984c90d1ccb0

commit 67813a2b5c1569fca9644e02680c984c90d1ccb0
Author: nodir <nodir@chromium.org>
Date: Wed Jun 15 03:14:08 2016

swarming-cipd: fix for Windows

Fix cipd.py for Windows: it cannot delete cipd.exe because the file is
readonly. DiskCache insists on files be read only, so make it writable
right before deletion

R=maruel@chromium.org, vadimsh@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/2066913002

[modify] https://crrev.com/67813a2b5c1569fca9644e02680c984c90d1ccb0/client/cipd.py
[modify] https://crrev.com/67813a2b5c1569fca9644e02680c984c90d1ccb0/client/utils/file_path.py
Project Member

Comment 32 by bugdroid1@chromium.org, Jun 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-py.git/+/296ce9e86230c641edf33cb6792a8d4648dfeee1

commit 296ce9e86230c641edf33cb6792a8d4648dfeee1
Author: nodir <nodir@chromium.org>
Date: Wed Jun 15 20:35:21 2016

swarming-cipd: custom installation paths

run_isolated.py: replace --cipd-package option with --cipd-package-list
which is a path to a JSON file that contains list of packages and their paths.
Create a site_root per unique package path relative to run dir. Remove
${CIPD_PATH} parameter because packages are installed always under run
dir at deterministic location.

isolatedserver.py: before writing a file on disk, check if it exists; if
does, raise AlreadyExists.  This is consistent with current behavior:
CMDdownload verifies that destination directory does not exist.

run_isolated.py: split --cipd-client-package to --cipd-client-package and --cipd-client-version.

Add "path" to CipdPackage ndb model and RPC entity. Validate it.
Propagate it to run_isolated.py.

Update user_task.html to group packages by installation path.

R=maruel@chromium.org, vadimsh@chromium.org
BUG= 601022 

Review-Url: https://codereview.chromium.org/2069903003

[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/handlers_bot_test.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/handlers_endpoints_test.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/server/task_request.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/server/task_request_test.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/swarming_bot/bot_code/task_runner.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/swarming_bot/bot_code/task_runner_test.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/swarming_rpcs.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/templates/user_task.html
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/appengine/swarming/test_env_handlers.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/client/cipd.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/client/isolateserver.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/client/run_isolated.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/client/swarming.py
[modify] https://crrev.com/296ce9e86230c641edf33cb6792a8d4648dfeee1/client/tests/run_isolated_test.py

Comment 33 by no...@chromium.org, Jun 15 2016

Status: Fixed (was: Started)
►

Sign in to add a comment