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.

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

Nightlies #409

Closed
wants to merge 5 commits into from
Closed

Nightlies #409

wants to merge 5 commits into from

Conversation

@edunham
Copy link
Contributor

edunham commented Jun 29, 2016

2 separate config files because the Android upload to the servo-rust bucket in the Rust account, and the everything-else upload to the servo-developer-preview bucket in the Servo account, require different credentials. @aneeshusa, could you remind me of the right way to salt the near-identical files which end up with different pillar data and different names?

Windows nightly requires:

  • Appropriate package command
  • Either refactoring the DynamicServoYAMLFactory to handle how all windows commands need to be run in their own bash shells (correct solution) or writing a new custom Windows factory (fast but bad solution)

This change is Reviewable

edunham added 3 commits Jun 24, 2016
Windows should wait on factories.py refactor to fix::

    windows = ServoFactory([
        # TODO: convert this to use DynamicServoFactory
        # We need to run each command in a bash login shell, which breaks the
        # heuristics used by DynamicServoFactory.make_step
        steps.Compile(command=make_win_command("./mach build -d -v"),
                      env=envs.build_windows),
        steps.Test(command=make_win_command("./mach test-unit"),
                   env=envs.build_windows),
        # TODO: run lockfile_changed.sh and manifest_changed.sh scripts
    ])
Already added the credentials to /srv/pillar on the master.
@aneeshusa
Copy link
Member

aneeshusa commented Jun 29, 2016

Just woke up and haven't looked at this yet, but you must be psychic @edunham - I was about to open an issue/PR for this last night! This should also finish off/fix servo/servo#10339.

builderNames=[
"android-nightly",
"linux-nightly",
"mac-nightly"

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Jun 29, 2016

Member

nit: leave a comma at the end of this line to reduce future diff noise

@aneeshusa
Copy link
Member

aneeshusa commented Jun 29, 2016

I would prefer to use environment variables + logEnviron: false to pass the AWS IAM Key Id and Secret Access Key. Using a config file implies it's readable by any process running as servo, and we don't have any user isolation yet (e.g. IIRC Homu also runs as servo). Using environment variables limits the scope to which these credentials are exposed.
In the future I'd even like to see us using https://docs.aws.amazon.com/STS/latest/APIReference/API_GetSessionToken.html to generate temporary session tokens so even if a token gets leaked it's NBD.

This can be done by modifying the make_step functions and adding a new heurisitic for s3cmd. The environment variables are AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY - I was also looking into this last night.

By the way - any reason we are using s3cmd instead of the AWS CLI?

mac-nightly:
- ./mach build --release
- ./mach package --release
- s3cmd -c ~/.s3cfg-servo put target/release/*.tar.gz s3://servo-developer-preview/nightly/`date +%Y`/

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Jun 29, 2016

Member

Two things:

  • Prefer "$(command)" form to backticks - style guide.
  • I believe that since make_step splits the command up into a list before passing it to Buildbot, Buildbot will exec the command directly, instead of through a shell, and the command substitution won't work. IMO it would be better to make a script for this in the servo repo in the etc/ci directory and call it from here.

This comment has been minimized.

Copy link
@cbrewster

cbrewster Jun 29, 2016

Member

Also the mac package script now generates a .dmg instead of a .tar.gz and it is put in the target directory (not debug or release directories)

- ./mach build --release
- ./mach package --release
- s3cmd -c ~/.s3cfg-servo put target/release/*.tar.gz s3://servo-developer-preview/nightly/`date +%Y`/
- rm -rf target/

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Jun 29, 2016

Member

Why did you add the rm -rf lines? Buildbot will clean out the working directory at the start of each build IIRC (or I think maybe the git step does this), so it should not be necessary to have as a standalone step.

@aneeshusa
Copy link
Member

aneeshusa commented Jun 29, 2016

It may also be nice to set up a MailNotifier so Buildbot sends emails when a nightly build fails, but that can be saved for a follow-up.

@metajack
Copy link
Contributor

metajack commented Jun 29, 2016

Is there a reason to continue to upload android builds to a Rust controlled bucket? Why don't we just dump everything in the servo bucket?

edunham added 2 commits Jun 29, 2016
* Fix directory structure to sort by target and ditch year directory;
  realistically we probably won't be in "developer preview" in 2-3 years when
  the # of files starts getting unmanageable

* Upload .dmg from correct location on osx

* Upload everything to Servo's AWS account
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jun 29, 2016

servo-master1 builds get:

[ FAIL ] Buildbot master config check failed:

         Configuration Errors:

           Unknown builder 'linux-nightly' in scheduler 'Nightly'

           Unknown builder 'mac-nightly' in scheduler 'Nightly'

           Unknown builder 'linux-nightly' in scheduler 'force'

           Unknown builder 'mac-nightly' in scheduler 'force'
@aneeshusa
Copy link
Member

aneeshusa commented Jun 29, 2016

I would rather not rush this PR too much, since we have some time before release and we can always make a build manually for release. @edunham if you want I can make a PR to your branch to implement passing credentials as environment variables.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jun 29, 2016

I'm not super excited about manual builds from user machines and would rather hold off on releasing binaries until we do it from machines that were at least originally installed from a clean state with only the dependencies we've documented & listed in salt. Maybe manual builds from those machines would make me feel less stressed, but publishing bits to the world from laptops the team has taken through security checkpoints in all sorts of interesting countries seems dubious :-)

gpg_passphrase =
guess_mime_type = True
host_base = s3.amazonaws.com
host_bucket = %(bucket)s.s3.amazonaws.com

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Jun 30, 2016

Contributor

Is the s after %(bucket) supposed to be there?

gpg_passphrase =
guess_mime_type = True
host_base = s3.amazonaws.com
host_bucket = %(bucket)s.s3.amazonaws.com

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Jun 30, 2016

Contributor

Same question about the s here.

use_https = True
use_mime_magic = True
verbosity = WARNING
website_endpoint = http://%(bucket)s.s3-website-%(location)s.amazonaws.com/

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Jun 30, 2016

Contributor

And here :-)

use_https = True
use_mime_magic = True
verbosity = WARNING
website_endpoint = http://%(bucket)s.s3-website-%(location)s.amazonaws.com/

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Jun 30, 2016

Contributor

Same question :-)

@aneeshusa
Copy link
Member

aneeshusa commented Jun 30, 2016

This will need servo/servo#11943 at the least, and then to use that script.

bors-servo added a commit that referenced this pull request Jun 30, 2016
…rsbergstrom

Add linux and osx nightly builds

Alternative to #409 that is more secure IMO.
Requires servo/servo#11943.

cc @larsbergstrom @edunham

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/410)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Jun 30, 2016
…rsbergstrom

Add linux and osx nightly builds

Alternative to #409 that is more secure IMO.
Requires servo/servo#11943.

cc @larsbergstrom @edunham

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/410)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 30, 2016

The latest upstream changes (presumably #410) made this pull request unmergeable. Please resolve the merge conflicts.

@aneeshusa
Copy link
Member

aneeshusa commented Jun 30, 2016

Filed #413 and #414 as follow-ups. Closing in favor of #410 (and #411).

@aneeshusa aneeshusa closed this Jun 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.