rvm / rvm1-ansible

The official ansible RVM role to install and manage your Ruby versions.
MIT License
270 stars 136 forks source link

Update and enhance the test environment #215

Closed gildegoma closed 4 years ago

gildegoma commented 4 years ago

As a prerequisite to further works (e.g. rebasing #52), I propose here to refresh the test environment with following changes:

pkuczynski commented 4 years ago

Looks good, but is it worth dropping old systems, instead of keeping them and adding newer ones?

gildegoma commented 4 years ago

is it worth dropping old systems [...] ?

Sorry, I forgot to give my rationale about this proposition of removals.

It is mainly motivated by the fact that these versions are (or will "soon" be) end of life (see summary and references below), and thus rvm1-ansible project may not want to support them any longer. As a wild guess, I thought that the goal was here to test and support the two last major releases of each operating system platforms.

EOL Dates Summary:

Please let me know the list of systems you want to keep in the CI setup, and I'll update it.


References:

gildegoma commented 4 years ago

@pkuczynski I added c5594e1 as an example, but I'll adapt it as soon as you decide which versions you want to keep in the test matrix.

derekgottlieb commented 4 years ago

Is it overly aggressive to drop OS versions that are still supported upstream? Should we wait until they become EOL and put up quick PRs to remove them at that time?

gildegoma commented 4 years ago

As I said, your list will be my list 😉

PS: I am baking a next PR adding support for Arch Linux 😇.

pkuczynski commented 4 years ago

I would keep testing old systems even beyond the EOL as long as it's not too costly. There has been many cases in RVM where people took ownership of some very old project and they had to maintain it before they could upgrade it.

In this case, having the older systems does not cost much, so I would keep them in.

gildegoma commented 4 years ago

😒 second time that the Travis build fails due to "transient problems". See here an example of rate limiting issue on https://github.com.

I'll give a try to parallelise a little bit more. Not only via Ansible from the same run, but with the different travis jobs to try to avoid such problems, and speed up the build time.

gildegoma commented 4 years ago

I think that a1bd852 is a good move. This will

image

gildegoma commented 4 years ago

@pkuczynski I think this PR is now ready for your review.

pkuczynski commented 4 years ago

Possibly we could consider moving to Github Actions in case Travis is not being fast or stable...

gildegoma commented 4 years ago

@pkuczynski about ASC vs DESC ordering in docker-compose.yml and inventory files:

I intentionally reversed it, with the intention to put the most recent stuff on the top (as you typically do with release notes in a CHANGELOG file). At this occasion, the diff looks a little bit weird, but it won't be the case in the future (if we keep the convention). If you prefer the ASC sorting convention, no problem, but I just needed to give you these details.

gildegoma commented 4 years ago

Possibly we could consider moving to Github Actions in case Travis is not being fast or stable...

In this particular case, I don't think that Travis is performing bad at all (I faced similar problems on my local testing, but didn't want to tackle this problem in the scope of this PR).

That said, moving to GitHub Actions could be an interesting move, but certainly in a distinct pull request 😉

pkuczynski commented 4 years ago

@thbar can you have a look at this PR please?

thbar commented 4 years ago

@pkuczynski approved overall, yet the build is failing, so we will need presumably to work on this before merging!

pkuczynski commented 4 years ago

Possibly we could consider moving to Github Actions in case Travis is not being fast or stable...

In this particular case, I don't think that Travis is performing bad at all (I faced similar problems on my local testing, but didn't want to tackle this problem in the scope of this PR).

That said, moving to GitHub Actions could be an interesting move, but certainly in a distinct pull request 😉

I did move some of my projects from Travis to GHA and it was pretty smooth yet seemed to be much faster than Travis. Should be done in a separate PR of course...

pkuczynski commented 4 years ago

@pkuczynski approved overall, yet the build is failing, so we will need presumably to work on this before merging!

Yeah, I agree. If this supposed to update the test env, it should also make sure that it's green. @gildegoma can you try to fix the failing build?

gildegoma commented 4 years ago

The only suggestion I would make (but that will be a good topic for a separate PR) is to have variables for the 2 versions of ruby installed (currently ruby-2.6.6 and ruby-2.7.1), so that we can update in DRY fashion.

I have prepared a patch for this, ~and will submit it as a new PR, as soon as we get this one merged~.

Edit: Finally, this will be part of this PR (was good to use to demonstrate the new caching setup). See f5d48d3f4b33cc87fc24a91b600ed1b8e6a99207

gildegoma commented 4 years ago

yet the build is failing, so we will need presumably to work on this before merging!

About the Travis CI failures... These are transient problems (that partially motivated me to split the jobs, so one can restart the failing parts without having to wait for a full rebuild. We'll come back onto this when we'll study a migration to GitHub Actions,...

For the records, I put here some comments about the two jobs that failed (#395.1 and #395.2).

395.1: The rvm-installer script couldn't be download (from raw.githubusercontent.com, which returned a 502 error).

I'll propose an until-retries-delay construct for the related get_url task (but again outside of the scope of this PR). Same fix could be applied elsewhere

image

395.2: This one is quite strange (never seen in previous builds). It looks like docker-compose detects a syntax error in the docker-compose.yml file, which is not the case at all.

image

gildegoma commented 4 years ago

Now, I will kick these two failed jobs, for the example...

image

and then... (still the same commit)

image

gildegoma commented 4 years ago

⏫ rebased to amend my previous commits (I had GPG signing disabled on my current laptop).

gildegoma commented 4 years ago

@Kulgar @thbar @pkuczynski I think I have addressed the requested changes, and added a few more improvements. I know, that it's getting too large to be easily reviewed, and that's why I would like to get it merged asap. It will unlock other PRs (having CI working), and allow to provide additional improvements for the test environments via future (smaller) PR iterations.

thbar commented 4 years ago

I see that @pkuczynski already reviewed, so that's 3 pair of eyes, I'll go ahead and merge.