search

dokku / ansible-dokku

Ansible modules for installing and configuring Dokku
MIT License
169 stars 44 forks source link

test for dokku hostname failing #77

Closed ltalirz closed 4 years ago

ltalirz commented 4 years ago

For whatever reason, the test checking that the dokku hostname is set correctly started failing on master: https://travis-ci.org/github/dokku/ansible-dokku/jobs/709225176#L1266

I would like to point out that the tests on both the PR adding the test, and the following PR updating the dokku versions were passing: image

Is it the version bump that resurfaced the bug?

@josegonzalez As I mentioned before, I think it would be very useful to add a branch protection rule for the master branch that requires PRs to be "up to date" (I don't have permissions to do this).

This should avoid such nasty surprises in the future.

josegonzalez commented 4 years ago

I think you have repo admin right? If not, I can set that up when I’m back at a computer.

ltalirz commented 4 years ago

Github has a number of tiers - at the moment these are all the settings I see: image

josegonzalez commented 4 years ago

I setup moderation for the repo meow.

ltalirz commented 4 years ago

Thanks!

As for the issue itself, do you have an idea what could have caused this?

It seems like setting the hostname via dokku_hostname no longer works after the update from 0.19 to 0.21 - for the specific versions bumped, see https://github.com/dokku/ansible-dokku/pull/76/files#diff-04c6e90faac2675aa89e2176d2eec7d8

josegonzalez commented 4 years ago

Not sure, we haven't changed the postinst handling in a while...

ltalirz commented 4 years ago

The previous version v0.19.11 was released December 10th, 2019. The only commit that touched the postinst file since then is https://github.com/dokku/ansible-dokku/pull/76/files#diff-04c6e90faac2675aa89e2176d2eec7d8 which replaced egrep by grep - I guess that's not the source of the issue (?).

Unless there are better ideas, we'll need to start doing a bisection search on the dokku version. One thing that makes this a bit tedious is that we'll likely need to pick the right herokuish, plugn and sshcommand versions as well... is there a list of the correct versions of thise for each dokku version?

josegonzalez commented 4 years ago

I would just switch dokku versions until you find the issue. It's unlikely to be herokuish or sshcommand. plugn is the only other one I'd be slightly worried about.

What happens if you re-run tests with the older Dokku release?

ltalirz commented 4 years ago

Ok, I finally have a bit of time to look back into this and, indeed, when reverting back to the old versions, the tests start to pass again: https://travis-ci.org/github/ltalirz/ansible-dokku/builds/727116910

I'll now start upgrading versions one by one.

ltalirz commented 4 years ago

@josegonzalez It turns out that the issue is independent of the plugin versions and was introduced at dokku version 0.20.0 (version 0.19.3 passes with the current plugins).

Commits between 0.19.13 and 0.20.0: https://github.com/dokku/dokku/compare/v0.19.13...v0.20.0 0.20.0 Changelog: https://github.com/dokku/dokku/releases/tag/v0.20.0

Can you have a look and see whether you can spot it?

josegonzalez commented 4 years ago

Maybe somewhere here? https://github.com/dokku/dokku/compare/v0.19.13...v0.20.0#diff-24a947d0d17068c3d22fbc3fb60fa554

ltalirz commented 4 years ago

Sorry, that link does not resolve for me, and there doesn't seem to be a commit starting with 24a94 in the list (?)

josegonzalez commented 4 years ago

https://github.com/dokku/dokku/commit/e19b52442bdfd36c64cecc9a8a25bef76347d66a

ltalirz commented 4 years ago

Looks like it was not the functionality that was broken but the test - dokku domains changed behavior in https://github.com/dokku/dokku/commit/848373def5edfdea41d736fc3a730e08bff79ff1

josegonzalez commented 4 years ago

Oh heh yeah forgot about that.