-
Notifications
You must be signed in to change notification settings - Fork 85
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
Resolve Travis concurrent jobs or consider Azure pipelines #118
Comments
please also test cirrus-ci: https://cirrus-ci.org/#comparison-with-popular-ciaas |
@aboe76 Would you mind bringing that up in the Slack room? More eyes will see it there. In the meantime, what do you think about limiting the number of runs per formula? Should it be one for each |
most formula's only needs these different os's, :
when switching to circus ci the concurency limit isn't a factor any more:
|
I agree with @aboe76: having a 20+ images available does not mean that we need to test with all of them. I think that to test that a formula works, the rationale should be something like:
tests for the required combination of the above should be added to the test matrix. I think that part of the review of PRs should check the testing matrix to be enough but not an excess. Testing is easy, knowing what and when to test is not 😋 Also, cirrus-ci sounds interesting 👍 My 2c |
So after some discussion on Slack prompted by @aboe76, we've got Cirrus CI enabled for the whole org. Some specific notes about BSD:
I've started work on testing Cirrus CI with |
https://cirrus-ci.org/guide/writing-tasks/#additional-containers |
So a first example of the difference between 15 and 5, thanks to @daks:
@aboe76 @javierbertoli Are these the 5 images you're suggesting or did you mean stick with the By the way, also note that we're leaving all of the platforms available in |
@myii I would prefer to stick with the os_family variants, and then a little bit conservative, Having to wait 16 minutes to have a test run for a PR is a bit much... |
@aboe76 Thanks for that. What do you think of my comment I've just made after you suggested that: saltstack-formulas/ufw-formula#7 (comment)? |
Yes @myii, I think that is a good comment, like with the testing of the the formula works on OS, the same thing can be applied to salt versions. The testing should be able to spot regressions and issues earlier, but not in a way that is hindering or slowing everything down. I know about edge cases that we might miss, but I don't think that most of the community will be better served if we stick to the broad road instead of narrow paths. |
…s#118) * The selected 5 in a "T"-shaped matrix: - 2019.2 (py3): debian-9 fedora-29 opensuse-leap-15 - 2018.3 (py2): ubuntu-1604 - 2017.7 (py2): centos6 * Covers each `os`, each Salt version and both Python versions
So #121 is a suggested workaround to use for the time being. Please have a look so that this selection can be propagated throughout all of the formulas using 10+ instances (i.e. all |
I've just noticed that |
…s#118) * The selected 6 in a "tree"-shaped matrix: - 2019.2 (py3): debian-9 centos7 opensuse-leap-15 - 2018.3 (py2): fedora-29 ubuntu-1604 - 2017.7 (py2): centos6 * Covers each `os`, each Salt version and both Python versions
ci(travis): reduce matrix down to 5/6 instances (ref: #118)
## [2.1.12](v2.1.11...v2.1.12) (2019-05-24) ### Continuous Integration * **travis:** improve recommended matrix usage comment ([b08a0fd](b08a0fd)) * **travis:** reduce matrix down to 6 instances (ref: [#118](#118)) ([a8834e2](a8834e2)) ### Documentation * **contributing:** add `bind-formula` to `semantic-release` formulas ([3da78b0](3da78b0))
So a little update. With some help from the Cirrus folk and working together with @javierbertoli, we've started making progress with Cirrus CI. Some parts of the developments are mentioned from this point forward, for anyone interested in more of the details. Or to get straight to the output, you can see it here: https://cirrus-ci.com/build/5647302937542656. I'd like to pull in @alxwr here: Cirrus CI allows testing on *BSD as well. Is this something you'd be interested in helping out with? Or for whoever fancies taking this on, this comment above is pertinent to that. We're also seriously considering keeping both Travis and Cirrus. Travis for the the release process (i.e. To gauge whether this is really going to make a significant difference, we need some "lambs to the slaughter" -- suggestions of formulas that can be converted to this hybrid system. Preferably formulas that involve *BSD, MacOS and Windows as well, so that we can test those features as well. It's not really much of a risk since we can always re-enable the matrix in Travis again if it doesn't work out. |
Why not... :) I didn't reply until now because I was thinking about the CI config file : it's cool to have a performant CI but it's cool to have nice config files too ;) And IMHO, Azure's config file is not very sexy, so not very easy to understand and to maintain/evolve. On the other hand, Cirrus' config file is as easy to understand than Travis one so it's a good point for Cirrus. |
@n-rodriguez You spoke too soon... wait until we start adding *BSD, MacOS and Windows! |
Just noting another benefit of keeping the release process in Travis: all of the GitHub tokens that have already been set across the repos. Having to do that again for Cirrus isn't appealing. |
https://cirrus-ci.org/faq/#are-there-any-limits:
|
In terms of ensuring all checks are passing before allowing merges, it's probably best to leave that to the GitHub UI rather than cross-checking from Travis and Cirrus: If there really was a need to check, there's probably a way via. the GitHub API: |
Here's the first PR merged using Cirrus CI alongside Travis CI: Note, we enabled all of the instances in the matrix for initial testing. We'll reduce that eventually, if all goes well. |
First negative points to collect:
Unfortunately, I've already seen it happen a few times. |
Some more bad news, unfortunately. Reduced the test matrix back to 6 (like our reduced Travis runs) but the first stage of the PR (before merge) hasn't completed yet: https://cirrus-ci.com/build/5778854061277184 -- over an hour. Update: I went away from this comment for a few moments and it's just finished. Now to try the PR merge to see how long that takes... |
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
* Use balanced matrix based on `template-formula` guidelines * Initial ref: saltstack-formulas/template-formula#118
https://freenode.logbot.info/saltstack-formulas/20190516#c2180979:
https://freenode.logbot.info/saltstack-formulas/20190516#c2181226:
https://freenode.logbot.info/saltstack-formulas/20190516#c2181237:
The text was updated successfully, but these errors were encountered: