Closed Bug 1250904 Opened 9 years ago Closed 8 years ago

When using one-click loaner do not run the tests; only set them up

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: armenzg, Assigned: ahal)

References

(Blocks 1 open bug)

Details

(Whiteboard: [frontend])

Attachments

(4 files, 2 obsolete files)

We can just append --no-run-tests which gets passed down to Mozharness.
We can do this inside of bin/test.sh when TASKCLUSTER_INTERACTIVE=true is set.

jonasfj: how can I add messages when a developer connects to the shell like interface?
Flags: needinfo?(jopsen)
Create a file:
  /etc/taskcluster-motd
On the docker image.


It's a hack for now:
https://github.com/taskcluster/taskcluster-tools/blob/7ae2f24954e3cd58f945bdd0b8956d3b137c20e1/shell/app.jsx#L27

Maybe we'll formalize it later :)
Flags: needinfo?(jopsen)
Thanks jonasfj. I will look into it.
Assignee: nobody → armenzg
jonasfj: where can I change what command a one-click-loaner task executes?
Flags: needinfo?(jopsen)
I forgot. nvm.
Attached patch one_click_loaner.diff (obsolete) — Splinter Review
What do you think about this?

Perhaps I can wait for a file to be created insted of telling the developer to go and look at the output of the task being executed.

It would reduce the developer having know much about it.
I could also tell them where the Mozharness log is if they want to inspect what was accomplished during the setup time.
Attachment #8724259 - Flags: feedback?
FYI this still needs to be tested (still building) but I want your initial feedback.
Attachment #8724259 - Flags: feedback? → feedback?(jopsen)
Comment on attachment 8724259 [details] [diff] [review]
one_click_loaner.diff

Review of attachment 8724259 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like a really good start.

Question: Will this not download the binary that the task would otherwise download?
I think it should. Then there is no need to specify --binary-path, right?

---
Also with respect to the waiting thing I propose:
We add /etc/taskcluster-interactive-shell.sh as something like:
    #!/bin/bash
    while [ ! -f /bin/run-tests.sh ]; do echo "Waiting..."; sleep 1; done;
    echo "Ready, you can now run tests with: run-tests.sh"; 
    exec bash -li

Then not only is the "run-tests.sh" binary a simple command to run the tests.
But it is also the file we wait for in the interactive session.

Note: In the /etc/taskcluster-interactive-shell.sh script I intentionally
didn't use "#!/bin/bash -e" as a user pressing ctrl+c to interrupt the loop
should still get the bash shell from "exec bash -li"

---
Background: https://github.com/taskcluster/taskcluster-tools/blob/7ae2f24954e3cd58f945bdd0b8956d3b137c20e1/shell/app.jsx#L37
Basically, if there is a file: "/etc/taskcluster-interactive-shell.sh" in the container,
then the interactive shell will launch that script instead of bash.
So you can customize the shell the user is given, or even as done above wait for a resource
before letting the user play with the shell.

::: testing/taskcluster/scripts/tester/test-linux.sh
@@ +122,5 @@
>  for cfg in $MOZHARNESS_CONFIG; do
>    config_cmds="${config_cmds} --config-file ${cfg}"
>  done
>  
> +if [ ${RUN_LOCALLY} == true ]; then

I have very limited bash skills. But isn't the norm to use defined vs undefined ?
Or empty string == false, and non-empty string == true.
When doing flags with env vars.

It's just I suspect someone will do RUN_LOCALLY=TRUE,
and similarly for TASKCLUSTER_INTERACTIVE=TRUE :)

Anyways, like I said I have limited bash skills.

@@ +144,5 @@
> +  echo "You're reading this because you were waiting for your interactive session to be ready to be used."
> +  echo "Congratulations! We have set up everything for you (downloaded binaries, tests and set up the Python virtualenv)."
> +  echo ""
> +  echo "You can now run the tests by typing this command on your interactive session:"
> +  echo " sudo -E -u worker  python2.7 $WORKSPACE/${MOZHARNESS_SCRIPT} ${config_cmds} '${@}' --run-tests"

I propose:
echo -e '#!/bin/bash\nsudo -E -u worker python 2.7 $WOR....' > /bin/run-tests.sh
chmod +x /bin/run-tests.sh

Then tell people to just run: "run-tests.sh" :)

@@ +147,5 @@
> +  echo "You can now run the tests by typing this command on your interactive session:"
> +  echo " sudo -E -u worker  python2.7 $WORKSPACE/${MOZHARNESS_SCRIPT} ${config_cmds} '${@}' --run-tests"
> +  echo "You will probably need to append --binary-path to make it work."
> +  echo "If there are any issues please file a bug or ping us on #ateam"
> +  echo "https://bugzilla.mozilla.org/enter_bug.cgi?product=Testing&component=TaskCluster"

You could append this message to "/etc/taskcluster-motd" with: echo "..." >> /etc/taskcluster-motd
Possibly in addition to printing it here :)

@@ +149,5 @@
> +  echo "You will probably need to append --binary-path to make it work."
> +  echo "If there are any issues please file a bug or ping us on #ateam"
> +  echo "https://bugzilla.mozilla.org/enter_bug.cgi?product=Testing&component=TaskCluster"
> +
> +  sleep 3600 # This will keep this shell script from exiting and ending their interactive session

Decent hack for now.. We it's probably going to waste a lot of CPU cycles on nothing :)
But let's improve this later. I suspect we might have to do something smart worker-side.
Attachment #8724259 - Flags: feedback?(jopsen) → feedback+
Flags: needinfo?(jopsen)
(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #8)
> Question: Will this not download the binary that the task would otherwise
> download?
> I think it should. Then there is no need to specify --binary-path, right?
> 
When running Mozharness with the action --run-tests, it forgets that in the previous action it extracted the package.
This could be improved in the MH side.

I will look into the rest of the comments tomorrow.

Thanks Jonas!
See Also: → 1253384
I've taken most of your suggestions into account.
I'm testing this on try.
Attachment #8724259 - Attachment is obsolete: true
Depends on: 1253421
Blocks: 1262260
Blocks: 1262605
Not working on this at the moment.
Assignee: armenzg → nobody
Whiteboard: [frontend]
Going to take a shot at finishing this. I'm hitting a couple problems when connecting with one-click loaner (haven't tried setting interactive in the task yet). Unfortunately the turn around time to test changes is pretty slow on this (as it depends on try jobs finishing and then connecting interactively).
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Depends on: 1272110
Depends on: 1273158
No longer depends on: 1273158
Depends on: 1273926
Component: General → Task Configuration
Depends on: 1274645
Depends on: 1275308
This creates some defaults, enables common built-in extensions and sets up
Ubuntu's CA file for checking host certificates.

Review commit: https://reviewboard.mozilla.org/r/56564/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56564/
Attachment #8755029 - Attachment description: MozReview Request: Bug 1250904 - Pause interactive jobs before running tests → MozReview Request: Bug 1250904 - Re-organize dot-files in testing/docker/desktop-test, r?armenzg
Attachment #8758277 - Flags: review?(armenzg)
Attachment #8758278 - Flags: review?(armenzg)
Attachment #8758279 - Flags: review?(armenzg)
Attachment #8755029 - Flags: review?(armenzg)
Rather than guess what the developer means to do when they checkout an interactive tester,
we'll prompt them from the interactive shell. This means we need to bypass the mozharness
script initially. To make it easy to run again later, we save the exact command to a
'run-mozharness' binary.

Review commit: https://reviewboard.mozilla.org/r/56566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56566/
To make things as easy as possible, run a little wizard to help developers
choose what to do. This way they don't need to memorize any commands or read
any wiki pages.

Review commit: https://reviewboard.mozilla.org/r/56568/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56568/
Comment on attachment 8755029 [details]
MozReview Request: Bug 1250904 - Re-organize dot-files in testing/docker/desktop-test, r?armenzg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54342/diff/1-2/
Here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0074fe7857d99c615430982474da7a7c02c7aa5b

You can test this out by doing the one-click loaner for a job. Mochitests hit a pulseaudio permissions issue that I'll tackle in a separate bug, but other suites like xpcshell work well.
https://reviewboard.mozilla.org/r/54340/#review53942

This looks great.

How long does it take to clone the repo?
If too long, are there any optimizations that the TaskCluster team or gps have?

There might be on the image a tool the TaskCluster team created called tvcs. I don't know if that adds any value but thought of mentioning it.
Comment on attachment 8755029 [details]
MozReview Request: Bug 1250904 - Re-organize dot-files in testing/docker/desktop-test, r?armenzg

https://reviewboard.mozilla.org/r/54342/#review53946
Attachment #8755029 - Flags: review?(armenzg) → review+
Comment on attachment 8758277 [details]
MozReview Request: Bug 1250904 - Add an hgrc for 'worker' user in testing/docker/desktop-test, r?armenzg

https://reviewboard.mozilla.org/r/56564/#review53948
Attachment #8758277 - Flags: review?(armenzg) → review+
Comment on attachment 8758278 [details]
MozReview Request: Bug 1250904 - Don't run mozharness if TASKCLUSTER_INTERACTIVE is set, r?armenzg

https://reviewboard.mozilla.org/r/56566/#review53950
Attachment #8758278 - Flags: review?(armenzg) → review+
Attachment #8758279 - Flags: review?(armenzg) → review+
Comment on attachment 8758279 [details]
MozReview Request: Bug 1250904 - Start a wizard upon initiating an interactive shell, r?armenzg

https://reviewboard.mozilla.org/r/56568/#review53952
https://reviewboard.mozilla.org/r/54340/#review53942

That's a good idea, I think it actually does do some optimizations. If you don't mind I'd like to tackle that in a follow-up though, it's a bit lower priority than things like getting mach working.
Here's a previous up-to-date try run I did to prove it doesn't break existing non-interactive jobs:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0074fe7857d99c615430982474da7a7c02c7aa5b
Actually, I should probably wait for bug 1276216 to get fixed, otherwise all taskcluster jobs will turn orange until the next push (which is harmless in terms of tree health, but will confuse the sheriffs).
Depends on: 1276216
Attachment #8726375 - Attachment is obsolete: true
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa3e7ff72452
Re-organize dot-files in testing/docker/desktop-test, r=armenzg
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b3ad5d93fbd
Add an hgrc for 'worker' user in testing/docker/desktop-test, r=armenzg
https://hg.mozilla.org/integration/mozilla-inbound/rev/12b3b927435f
Don't run mozharness if TASKCLUSTER_INTERACTIVE is set, r=armenzg
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb0507f42e74
Start a wizard upon initiating an interactive shell, r=armenzg
Depends on: 1279040
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: