[UPD] Update Docky for Docker Compose v2 #157
Conversation
|
Thanks @mbcosta to tackle this problem. I think it's also time to get rid of not much used features and remove dead code. _use_specific_user -> I guess it's not needed anymore docky.main.service: this one is a bit tricky. The goal is to tell which container to open when
display_service_tooltip: may be replace it with traefik hostname dcpatched: one of the main features of docky. It allow us to _get_services_info() can be removed; create_volume can be done with docker-compose docky init is gone by the way. About your commits, everything looks fine execpt the logging. |
|
BTW, @mbcosta if you have any trouble running Docky projects on Slackware meanwhile, you can always follow my little guide to run them using virtualenv (I switched my old computer to a Samsung Android tablet recently (using a Debian chroot in a rooted Termux), it works well but I didn't manage to compile a proper Docker enabled kernel for it (I have a simple Docker in an Alpine/qemu VM for simple things like wkhtmltopdf)). |
There was a problem hiding this comment.
Hi Magno,
I tried to use your PR and I found the 2 small fixes below.
But even with these fixes I'm still having some trouble with docky build :
$ docker compose build
WARN[0000] The "ENCRYPTION_KEY_DEV" variable is not set. Defaulting to a blank string.
WARN[0000] The "ENCRYPTION_KEY_PROD" variable is not set. Defaulting to a blank string.
WARN[0000] The "ENCRYPTION_KEY_PREPROD" variable is not set. Defaulting to a blank string.
[+] Building 45.2s (6/9) docker:default
=> [odoo internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 1.07kB 0.0s
=> [odoo internal] load metadata for ghcr.io/akretion/odoo-docker:14.0-latest 44.8s
=> [odoo internal] load .dockerignore 0.0s
=> => transferring context: 137B 0.0s
=> CANCELED [odoo base 1/1] FROM ghcr.io/akretion/odoo-docker:14.0-latest@sha256:561352d131f442e8d1b8d6f69de96759cc20d624afa37eb811b7ea85cd605512 0.1s
=> => resolve ghcr.io/akretion/odoo-docker:14.0-latest@sha256:561352d131f442e8d1b8d6f69de96759cc20d624afa37eb811b7ea85cd605512 0.0s
=> => sha256:561352d131f442e8d1b8d6f69de96759cc20d624afa37eb811b7ea85cd605512 856B / 856B 0.0s
=> => sha256:d3968c6de152fd586fe0c347c6df69e6c08fb1f8f7c3e0cdfafb2c549b0ccb52 4.09kB / 4.09kB 0.0s
=> => sha256:eff392f77c6d4ec2931ee23a820685bde98aef01da29d10eda724611d9ac368e 13.80kB / 13.80kB 0.0s
=> [odoo internal] load build context 0.0s
=> => transferring context: 38B 0.0s
=> ERROR [odoo dev 1/4] COPY ./src /odoo/src 0.0s
------
> [odoo dev 1/4] COPY ./src /odoo/src:
------
failed to solve: failed to compute cache key: failed to calculate checksum of ref c0035a07-c35d-4db4-a519-bd6426497823::x1b4hoho5p8zi134hhp98pey0: "/src": not found
I'm using the docky-template odoo projetct:
https://github.com/akretion/docky-odoo-template
I don't even know if it's related to this PR, but I'm stuck here.
setup.py
Outdated
| version_match = re.search(r"VERSION = ['\"]([^'\"]*)['\"]", | ||
| version_match = re.search(r"VERSION = ["\"]([^"\"]*)["\"]", | ||
| version_file, re.M) |
There was a problem hiding this comment.
Need to keep
version_match = re.search(r"VERSION = ['\"]([^'\"]*)['\"]",
version_file, re.M)
otherwise the regex doesn't work
There was a problem hiding this comment.
Thanks @clementmbr fixed, I made a general replace and didn't noticed this problem
docky/common/project.py
Outdated
| def get_containers(self, service=None): | ||
| kwargs = {'one_off': OneOffFilter.include} | ||
| kwargs = {} | ||
| if service: | ||
| kwargs['service_names'] = [service] | ||
| return self.project.containers(**kwargs) | ||
| kwargs["service_names"] = [service] | ||
| return docker.compose.ps(services=service) |
There was a problem hiding this comment.
services needs to be a list :
def get_containers(self, service=None):
kwargs = {}
if service:
kwargs["services"] = [service]
return docker.compose.ps(**kwargs)
There was a problem hiding this comment.
Fixed, it's seems the error appear after initial PR, just for record the error
File "/home/test/.local/pipx/venvs/docky/lib64/python3.11/site-packages/docky/common/project.py", line 43, in get_containers
return docker.compose.ps(services=service)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/test/.local/pipx/venvs/docky/lib64/python3.11/site-packages/python_on_whales/components/compose/cli_wrapper.py", line 479, in ps
result = run(full_cmd)
^^^^^^^^^^^^^
File "/home/test/.local/pipx/venvs/docky/lib64/python3.11/site-packages/python_on_whales/utils.py", line 174, in run
raise NoSuchService(
python_on_whales.exceptions.NoSuchService: The docker command executed was `/usr/bin/docker compose ps --quiet o d o o`.
It returned with code 1
The content of stdout is ''
The content of stderr is 'no such service: o
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
2 times, most recently
from
8de0dfd
to
fbfabd1
Compare
|
Thank all for review @hparfr About "we loose other use case of docky, but do we have some ?" and by reading the ISSUE #155 I think one way to make Docky more abstract and usefful for other cases besides Odoo can be make possible change default command "docky run" by .env or compose.yml, so even for us if necessary each project can has a especific "docky run" or "docky logs" in the cases where some parameter need to be change or in the different stages of a project implementation, but this is just a speculation and should be debate in another PR or Issue. In the last commit I make the "docky open" work as you explained, I have tested when a container start with "docky run" and "docky up" and just let the old code for comparison reason and after your tests and OK it can be removed.
Removed the commit of change Log, so back to use debug, but included "$ " before to simulate/show as a command if it's a problem I can see to remove
@rvalyi @clementmbr $ docker system pruneBut didn't solve, the problem was in the folder /var/lib/docker and mostly /var/lib/docker/overlay2 , by check the folder # du -lha /var/lib/docker/
99G /var/lib/docker/In my case by saw some Folders with almost 1 year old I decided to delete, with the command bellow you be able to see cases with more than 300 days ( ref https://stackoverflow.com/questions/13868821/shell-script-to-delete-directories-older-than-n-days) Check
$ find /var/lib/docker/overlay2 -type -d -ctime +300
Delete
$ find /var/lib/docker/overlay2 -type d -ctime +300 -exec rm -rf {} +After did it start appear similar messages as yours, I think the reason was delete some overlays2 folder but there are still some references in the other folders, to solve it I runned the same command but in main folder $ find /var/lib/docker/ -type d -ctime +300 -exec rm -rf {} +Be careful, make backups if you have something important, you need to restart Odoo Service or reboot the machine after this, and if you don't have anything important and want to make a clean up $ find /var/lib/docker/ -type d -ctime +1 -exec rm -rf {} +Maybe another way to solve can be remove docker package and install again, the folder will be removed and recreate in the process, after this when you run "docky build" instead to look for Cache Folder will be create a new one. EDIT: you ca also try to use the command |
|
Hehe guess who is installing a Docky instance right now and may crash test the PR... |
|
for the record I got the same mentioned: Yes this is not the proper solution but it got the job done meanwhile. I'll look better at the PR later. EDIT: so in fact to get it work I had to install Docky with pip and not pipx: Recent Ubuntu or Debian release will force you to use this --break-system-packages option, but as I used the --user flag, it won't actually mess with my host packages. And then I edited/fixed the problematic file: easy peasy |
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
from
fbfabd1
to
c97ee88
Compare
|
After merge of #158 was possible reduce the code to review here and solve the conflict file, now the main changes are direct related with adapt Docky to use Docker Compose version 2. @rvalyi with the last commit "docky open" should be work as expect (I'm just waiting review to confirm if everything is right and after remove the old code), "docky pull" need to be test (but as a simple command it should be work without problem), so I think this PR can be use to avoid the necessity of patchs in other files. |
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
from
c97ee88
to
96122e9
Compare
|
@mbcosta thank you for the work. I'll try to create a basic CI workforce as I did for our boleto_cnab_api project and then things will be easier to review. |
|
thanks @rvalyi will be better has a CI to check |
docky/cmd/run_open.py
Outdated
| "run", "--rm", "--service-ports", "--use-aliases", "-e", "NOGOSU=True", | ||
| self.service] + self.cmd) | ||
| # Default command | ||
| docky_cmd = ["run", "--rm", "--use-aliases", "-e", "NOGOSU=True", self.service] + self.cmd |
There was a problem hiding this comment.
Why the service-ports removal? I'm using it to open a port for debugging purpose.
There was a problem hiding this comment.
Thank you @paradoxxxzero for your review, fixed
…ry python-on-whales to get some Config information from compose.yml files.
…cker info' for check errors related to OS or older or newer libraries versions.
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
from
96122e9
to
a9532d6
Compare
| # TODO: Is there a better way to do it, for example with Plumbum? | ||
| container_id = subprocess.check_output(command, shell=True,text=True).strip() | ||
|
|
||
| self._exec("docker", ["exec", "-u", user, "-it", container_id, "/bin/bash"]) |
There was a problem hiding this comment.
When using docky open --root user is False and this command crashes since it expects strings.
Something along this line should fix it:
self._exec("docker", ["exec", "-u", user or 'root', "-it", container_id, "/bin/bash"])There was a problem hiding this comment.
Fixed, in the method to get the User in order to has a general solution
def _use_specific_user(self, service):
user = self.project.get_user(service)
if self.root:
user = "root"
return userThere was a problem hiding this comment.
You removed the --service-ports again though 😅
There was a problem hiding this comment.
Sorry @paradoxxxzero , it was a mistake between the branches, fixed
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
from
a9532d6
to
652e648
Compare
mbcosta
force-pushed
the
UPD-use_docker_compose_v2
branch
from
652e648
to
1c986e6
Compare
|
@mbcosta @sebastienbeau |
|
I use docky open and never docky open --root, I didn't even know it exists. In any case, when root access is required, docker exec and docker cp usually give enough options. |
|
Still it should be an easy fix to restore this feature, please don't release a broken version |
|
@florian-dacosta maybe you has tested with an old branch, the change made in _use_specific_user method solve this problem, I'm test with the branch of the @sebastienbeau and it's seems work as expected
@paradoxxxzero I agree to fixed the problem before release and it's seems fixed, as you can see in the image above |
|
Yes there was a little problem in seb's branch and it's fixed now, thanks! |
|
I have merged this work here : #159 |
Update Docky for Docker Compose v2
Docker Compose V1 is deprecated
https://github.com/docker/compose
Warning
https://docs.docker.com/compose/migrate/
Some recently updates in the Python and PyYAML seems cause errors in Docky
If you try to reinstall Docky
PyYAML
Maybe related with the Issue 724 https://github.com/yaml/pyyaml/issues
To fix it's need to update from 'docker-compose' V1 in Python to 'docker compose' V2 in GO, by the change of the language is not more possible import docker compose direct by python, some projects try to reimplement:
Docker-Py https://github.com/docker/docker-py
Python on whales https://github.com/gabrieldemarmiesse/python-on-whales
Docker Composer V2 https://github.com/jensenkairos/docker-composer-v2
I'm choosed Python-on-whales
Some considerations
Here python-on-whales are just use for get Config files from project, it's possible use some of the commands like docker.compose.run() and others https://gabrieldemarmiesse.github.io/python-on-whales/sub-commands/compose/ but I keep it as before by Plumbum run in bash '$ docker compose run ...'.
Included a new command 'docky system' to get some System Infos OS Type, Kernel, OS, Docker, Docker Compose, and Docky versions; just to make easy debug errors related to some OS or old or new libraries versions, it's just a resume of '$ docker info'.
Tests was made with:
In the case you are testing with another OS check the versions of the main libraries, tests with other OSs are welcome, should be better extract this commit? For testing the change for v2 seems usefull for debug but if necessary I can extract.
The Docky commnands working are build run logs ps up down kill restart in both OS , command docky open return error, the problem seems related to file dcpatched.py ( I let this file commented in the __init to avoid error with import, need to check if still is necessary, any help are welcome ) and docky pull need to be test.
Integrated the PR #151 , with this LOG show the command(s) execute, I'm add "$ " before to show as a command
docker compose ps
$ docker compose ps / Should be better keep without $ ?
It's been usefull see what the command will be execute for check errors but there are a debate in the PR about this change, if necessary I can extract this commit.
I'm also tried integrated the PR #152 for use pyproject.toml, but return error when try to install
$ pipx install path_of_file/ --force
Other simple changes was made to removed deprecated encoding, change in License Header from '2018' to '2018-TODAY', call method super() don't need parameters, change simple quotes ' for double quotes " in python files and in the message about missing compose_file.py change print for logger.error.
The tests was made with a generic project in v14 with Brazilian Localization, futher tests with complex compose.yml parameters are welcome and necessary for a better review.
There are a error with Traefik when you run 'docky build', the log message don't show the complete error as 'docker compose build'
Docker Compose show the complete erro with the fix
Should be better if Docky show the same message or included the suggestion to solve.
If necessary, as said before, some committs can be extract for others PRs for a better review.
cc @rvalyi @renatonlima