foreman_docker
foreman_docker copied to clipboard
Fixes #8409 - Pull docker image asynchronously
Updated to let the action be a good thread citizen and not block the pool. To be tested with foreman core develop, I'll submit a patch with 1.7 compat fixes separately so we can keep track of these.
cc @witlessbird @daviddavis @parthaa can you test it?
Updated with a test for the provision plan and commented out the tests for the run phase of start and pull. I'll work with @iNecas or @pitr-ch trying to figure out what I'm doing wrong so that these tests don't pass.
re the test failure, what appears to be happening is that foreman-tasks is registering its extensions to the test rake task from its engine.rb initialisers. foreman-docker is of course requiring foreman-tasks, but the 'test' task(s) are now extended with both foreman-docker's and foreman-tasks' tests.
I don't know whether we should be running foreman-tasks' tests from here or not. We run core tests from plugins to check a plugin didn't break a core behaviour, and you could argue whether that should be the case or not for a plugin building on another plugin. It might be harder, since plugin tests may require additional development dependencies.
If you do want to run foreman-tasks tests, then I think something's wrong with the load paths (here?).
If not, then some refactoring of how rake tasks are modified (e.g. so they only happen when testing the plugin itself) or how we choose which sets are run is needed.. I need to think a bit more about that I guess.
This PR should address the issue with the foremant-tasks tests (so that the tasks tests should not be run as part of this engine tests) https://github.com/theforeman/foreman-tasks/pull/103
@iNecas Awesome! Any prospective dates for a release?
Hopefully today, we have other feature that I would like to get in before the release, but if we don't get that done till tomorrow, i will release new version without it anyway.
The new versions in a PR for foreman-packaging https://github.com/theforeman/foreman-packaging/pull/582
[test]
[test]
[test] this got aborted I think
[test] - tests were failing because I was using minitest-reporters here (and it's not used in Foreman core)
@daviddavis @parthaa @witlessbird Could you give a last test at this before merging?
looks good to me.
I've been testing this a bit more thoroughly myself and I think this should:
- Show a link to the task created when creating a container, telling the user to check Monitor > Tasks is not very user friendly.
- If the task fails, show in the show container page that the task in question failed
I'll polish it up with these changes & resubmit when I get a chance.
@dLobatog any updates?
I'll pick this up again when ActiveJob is available