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

Fix ASP.NET #1287

Merged
merged 18 commits into from Apr 4, 2015
Merged

Fix ASP.NET #1287

merged 18 commits into from Apr 4, 2015

Conversation

pdonald
Copy link
Contributor

@pdonald pdonald commented Dec 22, 2014

Plaintext, JSON and database tests should all work except for the fortunes tests.

Had to downgrade to ASP.NET MVC 4. When mono is released with these changes, we can upgrade to ASP.NET MVC 5.2.2 and the fortunes tests will also work.

When I launch nginx myself, everything seems to work. But toolset/run-tests.py --mode verify --install server --test aspnet-mono doesn't work for some reason.

vagrant@TFB-all:~/FrameworkBenchmarks$ cat results/ec2/latest/logs/aspnet-mono/out.txt
test.os.lower() = linux  test.database_os.lower() = linux
self.results['frameworks'] != None: True
test.name: aspnet-mono
self.results['completed']: {u'aspnet-mono': u'port 8080 is not available before start'}

@msmith-techempower
Copy link
Member

You have something bound to port 8080 before you run the suite.

cd xsp
git checkout 8a31bc625727594d42f94173768bee5cf8afd0a4
echo "Installing XSP"
sudo apt-get -y install mono-xsp4 mono-fastcgi-server4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to discourage the use of apt-get whenever possible moving forward. Is there a reason that apt-get must be used over building mono manually (other than it takes a long time)?

@pdonald
Copy link
Contributor Author

pdonald commented Dec 22, 2014

Tests are still running but I think they will all pass
https://travis-ci.org/pdonald/FrameworkBenchmarks/jobs/44878766

  • All tests including fortunes should work now
  • We're using the latest versions of everything
  • I needed the very latest mono to get everything to work

I'm installing mono from CI packages which install mono in /opt/mono-20141234567. It takes just seconds to install mono now. But you can git clone the mono repo (with all its submodules) and compile it from source instead if you want.

@hamiltont
Copy link
Contributor

absolutely stellar work @pdonald !! Looks like travis finished as you predicted - except fortunes, all tests pass.

me or mike will probably look at the sudo issue a bit more before merging, but a huge THANK YOU!!!! is in order for getting .NET back on track

@hamiltont
Copy link
Contributor

Re: sudo stuff - we should also consider that the build for aspnet takes >30 minutes, even when it downloads a mono prebuilt. If we try to build mono we may run into the travis limit of 50 minutes per job. If downloading source + building mono on travis takes >20 minutes then we're boned

@pdonald
Copy link
Contributor Author

pdonald commented Dec 22, 2014

Fortunes are fixed.

Setup takes about 5 minutes, the rest is spent sleeping between tests (60 sec each).

Get rid of the outdated nuget.targets and replace it with a custom build file
@msmith-techempower
Copy link
Member

Ugh, I really would prefer to not use apt-get, but mono has been such a beast that I'm basically willing to through in the towel for it. @hamiltont I want to have plain *nix supported, but I am willing to concede defeat for mono.

Thought: we COULD move away from sudo and apt-get in the framework-specific files and have another user (elevated) running the actual installation of "foundation software" (like mono, python, etc). Then, since we are going to have separate files for windows/linux (powershell vs. bash, for example) we could also break out a scheme for different distributions. Like?

@pdonald
Copy link
Contributor Author

pdonald commented Dec 23, 2014

What's wrong with apt-get? You can also download .debs separately if you want.

What about docker? docker pull mono:latest and docker run -it --rm mono:latest mono --version and voila.

@hamiltont
Copy link
Contributor

Sudo apt has a few issues:

  • ties our scripts to a specific flavor of Linux (while we only test on
    ubuntu at the moment, it would be good to expand that to things like cent
    os)
  • modifies the base machine, which can impact other tests, especially with
    regards to conflicting dependencies
  • makes it hard to test PRs that upgrade a framework, as we may have to
    reinstall the os just to ensure that both an upgrade and a clean install
    work

Ill need to take a closer look at this before i can recommend a course of
action

@msmith-techempower
Copy link
Member

^ What he said.

Essentially, we should strive to have all our required software in the $IROOT so that the environment is relatively untouched and each framework test is isolated. It is fine for many tests to say "I use that particular version of mono," but we want to avoid tests saying "I use any installed mono." Often, this leads us into trouble like half the frameworks relying on a particular version of PHP and the other half not caring, so when we moved to Ubuntu14, the PHP version that was installed via apt-get was updated and half the PHP tests suddenly started failing.

As @hamiltont pointed out, at some point we would like to drop the requirement of a debian-like operating system and move to a more linux-agnostic build, and to do that we would have to either move away from apt-get (building from source or using generic binaries) or come up with a scheme that allows multiple setups using apt-get, yum, etc. The latter sounds rather painful to me while the former should "just work."

@pdonald
Copy link
Contributor Author

pdonald commented Dec 23, 2014

What about docker? You can use Ubuntu, CentOS or whatever as a base image, install whatever version of mono you want however you want it, then you add your framework code and if it changes you just rebuild the part that changed, no more countless compilations. Or rebuild the whole thing. Similar frameworks can share one Dockerfile and if one framework needs a newer version, it can make a copy and use that from now on, others can switch later.

It should also make it easier for new contributors. They just need to create a Docker image and if it works for them it should work for everyone else. Since there's almost no overhead to running Docker containers, it should not affect benchmarks too much.

@msmith-techempower
Copy link
Member

"almost no overhead to running docker containers" adds a dubious amount of unknown delta to the suite. We have imposed a rule of "everything has to run on bare metal first" before considering software like docker.

@hamiltont
Copy link
Contributor

By the way, I have a branch that runs everything inside of docker, so once
around 10 is published I may be able to provide some part numbers on the
overhead
On Dec 23, 2014 6:44 PM, "Mike Smith" notifications@github.com wrote:

"almost no overhead to running docker containers" adds a dubious amount of
unknown delta to the suite. We have imposed a rule of "everything has to
run on bare metal first" before considering software like docker.


Reply to this email directly or view it on GitHub
#1287 (comment)
.

@hamiltont
Copy link
Contributor

*hard numbers
On Dec 23, 2014 7:18 PM, "Hamilton Turner" hamiltont@gmail.com wrote:

By the way, I have a branch that runs everything inside of docker, so once
around 10 is published I may be able to provide some part numbers on the
overhead
On Dec 23, 2014 6:44 PM, "Mike Smith" notifications@github.com wrote:

"almost no overhead to running docker containers" adds a dubious amount
of unknown delta to the suite. We have imposed a rule of "everything has to
run on bare metal first" before considering software like docker.


Reply to this email directly or view it on GitHub
#1287 (comment)
.

@kbrock
Copy link
Contributor

kbrock commented Dec 24, 2014

another note on docker. That ties the images to a base OS as well.

@hamiltont
Copy link
Contributor

@msmith-techempower my vote is that we merge this as-is, including the use of sudo. Once R10 is out the door we will (at some point) remove sudo permission from testrunner, which will break this setup. We resolve that problem when it comes, because it's been such a long road to get mono-based stuff to work. I say let's rely on sudo for mono-based stuff at the moment and deal with the problem more properly when we have time, either by 1) building mono ourselves, or 2) modifying toolset to have some sort of elevated-privilege install stage

BTW, I tested the current status of this PR in my local travis (commit 85169bc) and it was all good, so in my mind once travis verifies it I'm happy to merge this, then open an issue "Mono-based tests currently require sudo"

@pdonald
Copy link
Contributor Author

pdonald commented Dec 24, 2014

We can wget the .debs and extract them into $IROOT without sudo.

@hamiltont
Copy link
Contributor

@pdonald oh, that would be amazing - I didn't know you could extract a deb to a prefix folder. Please do this - it would actually be really useful to see how it's done (perhaps we can use that for other big frameworks)

EDIT: Latest travis run passed everything. I'll wait to merge in the hope pdonald can add a commit using wget on the deb to avoid sudo

@pdonald
Copy link
Contributor Author

pdonald commented Dec 26, 2014

@@ -19,7 +19,23 @@ http {

location / {
fastcgi_pass mono;
include /usr/local/nginx/conf/fastcgi_params;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wish to clean this up a little, a number of setup.sh files are doing this

@hamiltont
Copy link
Contributor

LGTM! I've left some minor comments, feel free to address if you wish, and I'll merge this in tonight!


./autogen.sh --prefix=${IROOT}/mono-3.6.0-install
# build
./autogen.sh --prefix=${MONO_HOME} --disable-docs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$MONO_HOME might not be available to everyone at this point. Example, a test that does not require xsp is run so mono is installed, but then another test is run later to run xsp+mono, now mono.installed exists, $MONO_HOME isn't set, and xsp missing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add fw_depends mono to the top of xsp.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line . ${IROOT}/mono.installed loads $MONO_HOME which is created when mono is installed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the first couple of lines from https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/toolset/setup/linux/languages/mono.sh checks to see if mono.installed exists and returns without setting the envvars if it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Envvars are stored in the mono.installed file and the . loads them from the file. It's confusing at first but I think it beats copy & pasting 6 lines of exports in every file that needs them. If mono is installed, they will be there. If mono is not installed, then xsp installation will fail because you need mono to compile and install and run xsp.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it; sounds good.

@pdonald
Copy link
Contributor Author

pdonald commented Dec 26, 2014

Please merge this and make those small changes


# download .debs and extract them into $TEMP dir
fw_get http://jenkins.mono-project.com/repo/debian/pool/main/m/mono-snapshot-${SNAPDATE}/mono-snapshot-${SNAPDATE}_${SNAPDATE}-1_amd64.deb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants