proposed PR: automate dockerbuild and push to dockerhub

scyto commented 3 years ago

Hi,

When I find a project like this i tend to fork, make my own multi-architecture containers using github workflows and push to dockerhub as that then becomes easy for me to use down the line.

I am in middle of doing this, i saw you like discussing things before a PR is raised (and that i need to raise it against the development branch).

1) when I get this working would you like me to submit the PR? (it will include doc changes and file changes to docker-compose example, the docker file and new workflow files, alternately I can keep it mine, but may not keep it that upto date.) 2) if so it would require you to create your own secrets in the master project for your docker repo 3) wow this a large image - are all those reqs really required - esp the dev version of python etc - i find i can reduce image size by switching over to run time version only.
4) what does the gateway env var do (i am building the container before i have even run the app, lol, so maybe that will become clear to me, so apologies if a dumb question.

(nice project, looking forward to converting 4 devices i have)

Scyto

scyto commented 3 years ago

for anyone who wants to try my completed solution and comment here see my forked instructions https://github.com/scyto/tuya-convert/blob/master/README.md#use-prebuilt-image

kueblc commented 3 years ago

Hi @scyto

Thanks for sharing your thoughts. I'm not familiar with Docker but happy to accept community contributions to improve the experience for everyone.

I'm including @Cyber1000 @ohadlevy @aravindkoneru on this discussion as they too have thoughts on the Docker approach. I defer to the experts, please let me know when you reach a consensus on the best approach and I'll be happy to merge.

Cyber1000 commented 3 years ago

@scyto sounds nice, had a look on your sources Some points: ad 3:

are all those reqs really required

I don't know exactly for now, the install_prereq.sh was here before and has nothing to do with docker itself - I just used the existing install_prereq.sh, maybe @kueblc can elaborate a little bit more on this (why python3-dev is in like you asked). I can't remember if I tried to remove some of the dependencies in it, it's too long since.

ad 4:

what does the gateway env var do

exactly the same as the GATEWAY-variable in config.txt, so no docker specific part here too (except that it actually replaces the GATEWAY-variable in config.txt within the docker container) Normally it works with 10.42.42.1, sorry I've forgotten which values are allowed here or if this is more or less fixed @kueblc

One thing to consider: In my PR #915 I've changed to alpine as base, as phusion/baseimage wasn't the best choice to run this on arm like raspberry and seemed to have issues with fedora too. I don't know debian:buster-slim well, but since you've added a lot of platforms/architectures in .github/workflows/build-and-push.yml I assume it may work. Could you try it on different architectures? Alpine is more lightweight (at least without installs 2-3 MB compared to 20+ in debian-buster-slim), so this might be a better choice here.

But all in all well done

scyto commented 3 years ago

@Cyber1000 sure i can try alpine, many of my other containers use it and it is often my starting point. Can't recall why I did it here (did i just default to debian slim or did i hit an alpine roadblock); ah just checked your PR.

Oh I think it was because i couldn't be bothered to chase all the req's on alpine - its always a PITA :-) - that was likely my root of the question about reqs too! (alpine is esp annoying if one has something that needs glibc).

I actually ran these tests on my pi - everything I do is multiarchitecture :-) - i haven't tested on the more obscure platforms, the pull and no issues raised is my indicator if there are issues.

What size is your image coming out at?

scyto commented 3 years ago

probably better for me to wait for your move to alpine to be merged, then i can rebase and then re-do the github build actions if folks want

Cyber1000 commented 3 years ago

Have to admit I had a starting point for alpine from an other-users-issue and had still some dependencies to add. Worked on a x86_64 and a user reported that at least the Dockerfile worked on his rpi4 (he used his own docker-compose.yml, since my branch wasn't ready at this time)

Well I think you got me with the size, it is ~50MB bigger than yours, though alpine itself is smaller :-) I think git (and perhaps bash) aren't neccessary (at least not at the end of the process), python3-dev may be too much too, as said my starting point was an issue in this repo and install_prereq.sh istself.

I would take my branch with alpine as base, we may reduce dependencies lateron (think I'll have some time this or next weekend)

scyto commented 3 years ago

yes, its good to remove git and other items once not needed and yes -dev is generally not needed for just runtime. (and yes i turned trying to get images small into a personal war :-) - some of my alpine ones for other projects are tiny)

also don't need bash as the built in ash is fine for most purposes (unless some hard core bash scripting going on with syntax issues wrt ash?)

If i have time at weekend i will take a look.

Cyber1000 commented 3 years ago

as far as I've seen:

git is needed for git+https://github.com/drbild/sslpsk.git (in the next line with "RUN python3"), perhaps there is an alternative, lateron git isn't needed by the container itself
-dev seem too much for me too
bash: the shebang in the scripts is #!/bin/bash, we'll have to try if it is really needed
there may be an alternative for sudo too: the scripts use heavily sudo, so it is installed here, but within docker we wouldn't need sudo. Perhaps we can fake sudo here somehow

Pr0mises commented 3 years ago

for anyone who wants to try my completed solution and comment here see my forked instructions https://github.com/scyto/tuya-convert/blob/master/README.md#use-prebuilt-image

@scyto Thank you! This version finally worked for my raspi4 docker version I also tried the new docker version from @Cyber1000 merged into development branch had some errors with it tho.

tput not found

ssl errors: ssl.SSLEOFError: EOF occurred in violation of protocol (_ssl.c:1131) (when connecting my phone)

smarthack-psk.log smarthack-wifi.log

Cyber1000 commented 3 years ago

Funny that it worked on my machine, no pi4 but that shouldn't matter with docker, maybe another alpine-version available there (which misses something)

Anyway, many thanks for the information! tput seems to be a part of ncurses, I'll have a look at it within the next few days. @Pr0mises Which OS version: rasberry OS 32 or 64bit? (Or something different?)

Notice to myself:

search usages of tput
install ncurses: RUN apk add ncurses

Pr0mises commented 3 years ago

@Cyber1000 I use the 64bit version of raspberry OS

ct-Open-Source / tuya-convert

proposed PR: automate dockerbuild and push to dockerhub #920