Skip to content

Conversation

@cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Feb 12, 2020

Try to upgrade travis to xenial from precise.
Ensures that mongo is 3.4 not 4.0. mongo 3.4 is not available in bionic so #4862 probably won't work until dependence on 3.4 is gone.
#4772 tried to update and ran into a many issues esp slowness. It also missed downgrading mongodb.

closes #4772
closes #4862

@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Feb 12, 2020
@cognifloyd cognifloyd force-pushed the travis-xenial branch 3 times, most recently from a23dd5b to 1c76c9e Compare February 12, 2020 07:17
@cognifloyd
Copy link
Member Author

This is a simpler version of #4772 (I cherry-picked the nosetests workarounds). It looks like travis has resolved some of the other issues that Kami was encountering with xenial.

Tests timing comparison:

@cognifloyd cognifloyd changed the title WIP switch travis to xenial switch travis to xenial Feb 12, 2020
@pull-request-size pull-request-size bot added size/M PR that changes 30-99 lines. Good size to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Feb 12, 2020
@cognifloyd
Copy link
Member Author

It looks like mongodb 4.0 was the reason for the slowdown in #4772 because #4862 is hitting similar slowdowns.

@cognifloyd cognifloyd force-pushed the travis-xenial branch 6 times, most recently from 6fdf020 to fb0dafd Compare February 12, 2020 10:58
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Great work, now this is really helpful! 👍

I like this PR more as it follows the initial @Kami effort, brings minimal changes and doesn't cost us timing penalty as it happens with Bionic + Mongo 4.0 PR in #4862

@cognifloyd The build is green in xenial Travis infra, but let me know if the PR is fully ready or there are any minor changes left.

@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Feb 16, 2020
@arm4b arm4b added this to the 3.2.0 milestone Feb 16, 2020
@arm4b arm4b changed the title switch travis to xenial Switch Travis to Xenial infra Feb 16, 2020
@arm4b arm4b changed the title Switch Travis to Xenial infra Switch Travis from Precise to Xenial infra Feb 16, 2020
@blag
Copy link
Contributor

blag commented Feb 17, 2020

At some point we should flip the linter checks to be run under Python 3. That will automatically turn on some additional checks in the linters themselves to keep our code forward compatible with Python 3 linting.

Not saying that has to happen in this PR though.

Copy link
Contributor

@blag blag left a comment

Choose a reason for hiding this comment

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

Looking good, but I still have some questions.

@cognifloyd cognifloyd force-pushed the travis-xenial branch 4 times, most recently from 27665a8 to 756dcdf Compare February 19, 2020 07:47
@cognifloyd
Copy link
Member Author

I had to re-add some permissions workarounds, but I added the permissions to a more selective set of files under /home/travis (instead of the whole thing).

At least for the rabbitmq stuff, we could probably copy the ssl certs to /etc instead of using them in the fixtures dir, but I'm not sure that would make much of an impact because of all the other fixtures that stanley needs to access (as far as I understand).

I also got rid of some of the legacy stuff in .travis.yml, clarified the make targets used in each travis job, and took a stab at updating the COMMAND_THRESHOLD numbers. In the latest build, the ci-py3-integration tests are even longer than what I put, so I could amend that commit with a new number.

@cognifloyd cognifloyd requested review from arm4b and blag February 20, 2020 05:02
arm4b
arm4b previously requested changes Feb 20, 2020
No longer needed without the old xenial permissions workaround that set
the entire /home/travis with 777 perms.
xenial has a new enough git (v2.21.0) so don't upgrade.
xenial comes with 4.0, so we have to downgrade to 3.4
These tests were added in StackStorm#4541.
Now that we're on xenial, we can reenable them.

This skips upgrading rabbitmq.

reverts 44e513e
originally added in 5387ece

reverts 922cb23
@cognifloyd cognifloyd force-pushed the travis-xenial branch 2 times, most recently from 3d21a5d to 8ffdc9f Compare February 21, 2020 03:52
@cognifloyd
Copy link
Member Author

There we go. I heavily rebased and squashed commits. I also included comments and fixes to address reviews. I just pushed one last edit that adjusts only 3 of the thresholds instead of all 5.

Once CI returns green, I think this is ready to merge. :)

@cognifloyd cognifloyd requested a review from arm4b February 21, 2020 03:59
There were several issues with the python3.6 virtualenvs.
- The Makefile defaults PYTHON_VERSION to python2.7 causing some
  virtualenvs to be built with 2.7 instead of 3.6.
- Once a virtualenv was built with 2.7 it was cached for later builds.
- The ci-py3-unit task was running tox for both unit and packs tests.
  Hiding discrete steps in the same task obscured the lack of
  pre-building one of the tox virtualenvs. It also made it hard to
  understand how the job name matched with the TASK/make target.

So, this updates the travis jobs to add PYTHON_VERSION and update the
job TASK to separate ci-py3-unit and ci-py3-packs-tests make targets.
Several other places that use TASK var were adjusted to clarify WHY they
were selecting different CI TASKs, thus allowing TASK to be adjusted or
reorganized without editing so many different files.

In one case, an if statement was selecting all of the TASK options, so
the condition was removed to leave just the command invocation.

TASK & Makefile to separate py3-unit and
py3-packs-tests. And updates several places that use the TASK var to
make it clear WHY they are selecting particular TASKs.
Try to adjust as few permissions as possible under /home/travis.
This includes adding other perms for fixtures, packs, and parent dirs.
The timing of some make targets is higher than the threshold. For
ci-py3-integration, it looks like that threshold is far too short for
what the tests regularly take. So, for the jobs that are signifcantly
different, this adjust the thresholds to be about 60s more than the last
successful build in travis.

make target: orig threshold=>new threshold (reference duration)

ci-checks ci-packs-tests:   280=>430 (369)
...-py3-unit ci-py3-pac...: 680=>750 (688)
ci-py3-integration:         310=>770 (716)
cognifloyd added a commit to cognifloyd/st2 that referenced this pull request Feb 21, 2020
@cognifloyd
Copy link
Member Author

cognifloyd commented Feb 21, 2020

added changelog entries and dropped them again

@blag blag dismissed arm4b’s stale review February 21, 2020 21:54

Resolved. :)

@blag blag merged commit 2b7c400 into StackStorm:master Feb 21, 2020
@cognifloyd cognifloyd deleted the travis-xenial branch February 12, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement infrastructure: ci/cd size/L PR that changes 100-499 lines. Requires some effort to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants