Justintime50 / harvey

The lightweight Docker Compose deployment runner.
https://github.com/Justintime50/harvey-ui
MIT License
13 stars 3 forks source link

Building docker image using docker api #43

Closed gurneesh closed 3 years ago

gurneesh commented 3 years ago

Changing the image build process to use docker API. Changes Made to

  1. harvey/images.py Changing variables to the values only rather than making them command-line arguments that are run through subprocess. Under if condition for context == 'test' i. We are changing the language variable to the value of the language key provided in config dictionary, same procedure is followed for variables version and project. ii. We are changing tag_arg variable to get the tag for the image from the webhook. iii. Initiating docker client, for now I have only allowed to use docker on host machine. (I am not sure but I think harvey has no support to connect to docker on other machines to build images or run containers) iv. Lastly, we are creating the image using image.build method. Now we don't have to cd into the directory as we have provided the path for dockerfile as parameter to the method and then we are returning the tag of newly created image.

  2. test/unit/test_images.py

    1. We also have to change the test_build_image method as now no subprocess is being run. So we are mocking the build object (https://stackoverflow.com/questions/64089226/how-to-mock-call-to-python-docker-sdk-using-mockito)

I'll change the double quotes to single quotes. Also, clean up the docstrings and comments and I'll also add docker to setup.py.

I only ran tests for test_images.py, I'll make sure to run and pass other tests as well.

These are just suggestions feel free to reject if these changes don't seem to go with the project.

gurneesh commented 3 years ago

Building image using docker api. Made changes to

  1. harvey/images.py
  2. test/unit/test_images.py
gurneesh commented 3 years ago

Hi, I didn't realize this earlier but stages need to timeout after a certain time. Which is quite easy to implement in case of subprocesses. But when building image using the docker api, it is going to be impossible as there is no timeout feature provided by docker package which can stop the build process if timeout accurs. Only http timeout is possible which might stop the build process (Not useful to us).

I am thinking of using multiprocessing module, which could start a docker image build in a new process and we could timeout. Not sure if this will sort things.

Justintime50 commented 3 years ago

I've tried the multiprocessing module previously in this project and determined I didn't want to use it due to the expensive overhead. There should be ways we could accomplish something similar via the threading module that is already in use.

We'll definitely want a timeout function so we don't run builds for eternity.

gurneesh commented 3 years ago

Hi, After replying yesterday regarding the timeout issue, I tried things using the multiprocessing module. Please look into this once. Meanwhile, I am going to check the threading module and see if we can accomplish similar results.

Thanks

Justintime50 commented 3 years ago

After a bit of research, it seems threads cannot be killed in Python (after some timeout for example). I'm hesitant to adopt the multiprocessing package in this project due to the overhead it brings; however, it could allow us to use the Docker package while still having a timeout but would be more CPU intensive than I think I want at this time. Whatever the solution, it'll require some planning. I've opened #49 to track the progress of this work.

gurneesh commented 3 years ago

Hi, I understand your concern with the multiprocessing module and threads cannot be killed so implementing the timeout feature would be an issue but it is essential as we cannot let pipelines run for an infinite amount of time.

And, other options like concurrent.futures or gevent are totally out of scope for our use case.

I'll look for other solutions where we can allow building image and then timeout when it exceeds the given limit.

Justintime50 commented 3 years ago

Just so I have some context, are you currently using Harvey? If so, what pieces are you using it for? Meaning - are you using docker-compose workflows or are you using it to run CI tests, etc?

Justintime50 commented 3 years ago

I've been thinking for awhile now that the scope of the project blew up larger than initially anticipated. The reason I ask how you are using Harvey at the moment is because I think I'm going to move forward with https://github.com/Justintime50/harvey/pull/50 which will actually remove this logic completely and return Harvey to a docker compose only deployment platform which is what I initially built it out as.

I fear the rest of the code outside of docker-compose workflows isn't well supported and as such I haven't given it the time it deserves to be really useful - plus there are other deployment and testing platforms out there such as Rancher that will do a better job at bigger tasks like that. I wanted to keep Harvey lightweight and focused on simple compose deployments. I'll stew on this for a couple more days before doing anything drastic. Would love your thoughts on any of this since you've been working on this PR (many thanks for digging in and taking a look!)

gurneesh commented 3 years ago

Hi, I am using harvey for running CI tests only on some personal projects nothing big or production ready even. I came across this repository few days ago and thought that it would be great to have an easy to use CI/CD such as this one, so I decided to take a look at source and suggested some changes. I would be great to use harvey for compose deployments.

You are right that other tools like Jenkins and Rancher can be used.

Feel free to narrow the scope of project to compose only.

Thanks

Justintime50 commented 3 years ago

@gurneesh thank you very much for your thoughts here and the work produces for this PR. I think for now I'm going to cut back on the scope of Harvey, polish it up really nicely, and then grow the scope from there potentially back to including other aspects like testing and non-compose deployments.

The beauty of open source is you have the fork and can continue making improvements to the testing framework in parallel to the improvements I'll be making on the compose deployment side of things. The next release of Harvey will not include its testing framework as it's being removed. I'd suggest either using your own local copy to continue running tests via Harvey or switching to something well supported like GitHub Actions. You can even have GitHub Actions run your tests for you and then deploy it to your server with Harvey.

Once again, I appreciate your work here and apologize I'm unable to include it at this time. If you'd like, feel free to watch or star this project to keep an eye on its future development and there may come a point where I grow the project scope out again. Best!