Skip to content

Improve docker container reliability #87

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

Merged
merged 10 commits into from
Dec 8, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ dependencies:
- docker pull plotly/imageserver:latest
post:
- npm run cibuild
- docker run -d --name myimageserver -v $PWD:/var/www/streambed/image_server/plotly.js -p 9010:9010 plotly/imageserver:latest; sleep 20

- docker run -d --name myimageserver -v $PWD:/var/www/streambed/image_server/plotly.js -p 9010:9010 plotly/imageserver:latest
- wget --server-response --spider --tries=8 --retry-connrefused http://localhost:9010/ping
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait until image server returns a pong before starting the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An even better script would restart nw1 if the image server doesn't return a pong after 8 tries.

Looks like the latest container is reliable enough that we don't need this now. But, maybe later.

test:
override:
- sudo lxc-attach -n "$(docker inspect --format '{{.Id}}' myimageserver)" -- bash -c "cd /var/www/streambed/image_server/plotly.js && node test/image/compare_pixels_test.js"
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ dev:
- .:/var/www/streambed/image_server/plotly.js
ports:
- "9010:9010"
- "2022:22"
- "2022:22"
4 changes: 2 additions & 2 deletions tasks/baseline.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ CONTAINER_NAME="imagetest" # same as in docker-compose.yml
CMD=(
"cd /var/www/streambed/image_server/plotly.js &&"
"cp -f test/image/index.html ../server_app/index.html &&"
"monit restart nw1 &&"
"sleep 5 &&"
"supervisorctl restart nw1 &&"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bye bye monit.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

"wget --server-response --spider --tries=8 --retry-connrefused http://localhost:9010/ping &&"
"node test/image/make_baseline.js $1"
)

Expand Down
4 changes: 2 additions & 2 deletions tasks/test_image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ CONTAINER_NAME="imagetest" # same as in docker-compose.yml
CMD=(
"cd /var/www/streambed/image_server/plotly.js &&"
"cp -f test/image/index.html ../server_app/index.html &&"
"monit restart nw1 &&"
"sleep 5 &&"
"supervisorctl restart nw1 && "
"wget --server-response --spider --tries=8 --retry-connrefused http://localhost:9010/ping &&"
"node test/image/compare_pixels_test.js $1"
)

Expand Down
59 changes: 37 additions & 22 deletions test/image/compare_pixels_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,29 @@ var request = require('request');
var test = require('tape');
var gm = require('gm');

var touch = function(fileName) {
fs.closeSync(fs.openSync(fileName, 'w'));
};


// make artifact folders
if(!fs.existsSync(constants.pathToTestImagesDiff)) fs.mkdirSync(constants.pathToTestImagesDiff);
if(!fs.existsSync(constants.pathToTestImages)) fs.mkdirSync(constants.pathToTestImages);
if(!fs.existsSync(constants.pathToTestImagesDiff)) {
fs.mkdirSync(constants.pathToTestImagesDiff);
}
if(!fs.existsSync(constants.pathToTestImages)) {
fs.mkdirSync(constants.pathToTestImages);
}

var userFileName = process.argv[2];

var touch = function(fileName) {
fs.closeSync(fs.openSync(fileName, 'w'));
};

if (!userFileName) runAll();
// run the test(s)
if(!userFileName) runAll();
else runSingle(userFileName);

function runAll () {
test('testing mocks', function (t) {

var files = fs.readdirSync(constants.pathToTestImageMocks);
var allMocks = fs.readdirSync(constants.pathToTestImageMocks);

/*
* Some test cases exhibit run-to-run randomness;
Expand All @@ -35,15 +40,34 @@ function runAll () {
* More info:
* https://github.com/plotly/plotly.js/issues/62
*
* 40 test cases are removed:
* 41 test cases are removed:
* - font-wishlist (1 test case)
* - all gl2d (38)
* - gl2d_bunny-hull (1)
* - gl3d_bunny-hull (1)
* - polar_scatter (1)
*/
t.plan(files.length - 40);
var mocks = allMocks.filter(function(mock) {
return !(
mock === 'font-wishlist.json' ||
mock.indexOf('gl2d') !== -1 ||
mock === 'gl3d_bunny-hull.json' ||
mock === 'polar_scatter.json'
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 mock is also showing some test-to-test randomness, let's skip it for now.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a corresponding issue for these and if so could you link to it in the code here with a
"See issue [issuelink]"

Copy link
Member

Choose a reason for hiding this comment

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

Haha Rodger dodger!

);
});

var BASE_TIMEOUT = 500, // base timeout time
BATCH_SIZE = 5, // size of each test 'batch'
cnt = 0;

function testFunction() {
testMock(mocks[cnt++], t);
}

t.plan(mocks.length);

for (var i = 0; i < files.length; i ++) {
testMock(files[i], t);
for(var i = 0; i < mocks.length; i++) {
setTimeout(testFunction,
BASE_TIMEOUT * Math.floor(i / BATCH_SIZE) * BATCH_SIZE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Send 5 requests at a time, not 235.

Copy link
Member

Choose a reason for hiding this comment

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

Man - I really assumed we were already batching them. Can't believe we went this long without failures! Good to know the image_server can work with such a huge onslaught :)

}

});
Expand All @@ -57,15 +81,6 @@ function runSingle (userFileName) {
}

function testMock (fileName, t) {
if(path.extname(fileName) !== '.json') return;
if(fileName === 'font-wishlist.json' && !userFileName) return;

// TODO fix race condition in gl2d image generation
if(fileName.indexOf('gl2d_') !== -1) return;

// TODO fix run-to-run randomness
if(fileName === 'gl3d_bunny-hull.json') return;

var figure = require(path.join(constants.pathToTestImageMocks, fileName));
var bodyMock = {
figure: figure,
Expand Down