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

travis-ci: fix docker deploy #1684

Merged
merged 6 commits into from Sep 28, 2018

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Sep 28, 2018

This PR fixes some errors in the original travis docker PR (#1670):

  • Accidentally left grondo/flux-core as the repo slug in the test for whether to deploy
  • Left a set -x in the docker-run-checks.sh script.

Problem: Docker image auto-deployment for new merges to master or
tags was broken due to a testing "repo slug" left in the test
to ensure we were running in the official flux-core repository.

Update the repo slug to `flux-framework/flux-core`.

Also, include the DOCKER_TAG=t variable for CentOS 7 builds so
we get a centos docker image.

Finally, update the names of builders in Travis-CI for clarity.
Remove the `set -x` left over from testing in docker-run-checks.sh.
@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

This should fix the Docker auto-deploy for merges to flux-core@master, however, it is difficult to know for certain until we actually try it. Sorry!

@codecov-io
Copy link

codecov-io commented Sep 28, 2018

Codecov Report

Merging #1684 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1684      +/-   ##
==========================================
- Coverage   79.29%   79.28%   -0.01%     
==========================================
  Files         186      186              
  Lines       35059    35059              
==========================================
- Hits        27801    27798       -3     
- Misses       7258     7261       +3
Impacted Files Coverage Δ
src/common/libflux/message.c 80.95% <0%> (-0.24%) ⬇️
src/bindings/lua/lua-hostlist/hostlist.c 58.78% <0%> (-0.23%) ⬇️
src/cmd/flux-job.c 88% <0%> (+0.66%) ⬆️

@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

Don't merge this quite yet, I have a couple other improvements I want to make to the travis build scripts.

Add src/lib/travis-lib.sh with functions for folding and timing
script output in Travis-CI logs, and use this library in
docker-run-checks.sh and travis_run.sh.

By default, folded lines also get timing information.
Move variable setup to before_install section of travis,
so that the output gets collapsed in travis logs for a cleaner
look.
@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

In addition to hopefully fixing the deploy, I added some support for "folding" some of the output from docker-run-checks.sh and travis_run.sh via support functions in a new src/test/travis-lib.sh shell library. This isn't quite as impactful as I hoped, but does make the travis logs a little cleaner.

@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

Ooops.. noticed one other thing. aspell is missing from the images so we're not getting the docs spellchecked. Let me fix that in this PR as well... Sorry for all the blunders!

Problem: spellcheck tests were skipped in Travis because aspell
was not installed in the base images.

Add aspell and aspell-en to CentOS and Ubuntu Dockerfiles.
Problem: t2201-job-cmd.t sets a HAVE_FLUX_SECURITY prereq by
checking for the `sign-type` arg on the flux-job command.
However, this arg exists on `flux job submitbench` not
just flux-job, so the test was failing.

Adjust the test to use `flux job submitbench` so the security
tests are not skipped.
@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

Ok, this one is ready for merge I think. Then we can see if the deploy is still broken.

@garlick garlick merged commit 4923879 into flux-framework:master Sep 28, 2018
@grondo grondo deleted the travis-docker-fixups branch September 28, 2018 20:18
@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

ugh almost all the builds on master failed. This is a new one:

/dev/stdout: resource temporarily unavailable

Also some of the builds have truncated output and then move on as if they've failed.

I tried re-pushing the old master (to my own fork), and it fails in the same way, so I guess these failures were not introduced with this PR, and perhaps Travis is hosed...

@grondo
Copy link
Contributor Author

grondo commented Sep 28, 2018

This does seem to be an output handling issue. I tested a push where all output from make/make check was suppressed and all builds passed. Might have to open a travis-ci issue on this one. 🤷‍♂️

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

3 participants