Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Hass.io #3732

Merged
merged 34 commits into from Jan 19, 2020
Merged

Add support for Hass.io #3732

merged 34 commits into from Jan 19, 2020

Conversation

@fredrike
Copy link
Contributor

fredrike commented Jul 4, 2019

Motivation: The Home Assistant package runs HA natively in Python, the preferred way is to run hass.io and let it manage HA via Docker.

Checklist

  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully
@fredrike fredrike force-pushed the fredrike:hass.io branch from 8302cbc to d00caa4 Jul 9, 2019
@chickenandpork

This comment has been minimized.

Copy link
Member

chickenandpork commented Jul 11, 2019

FWIW I See you've taken great care to provide a lot of things to make this a well-behaved and functionality-complete config. I worry that the supported architecture is very limited.

In hassio.sh@32 it might be worth noting that 172.30.32/24 is hard-coded in hass supervisor? (is it?) Same file, line 36, might be worth documenting the logic of the "attach to running image or attempt to start a new image to attach to" if I'm understanding correctly.

You've clearly looked closely at details, and I expect this is running on your person system? It makes me want to try it on my avoton.

I notice you're commenting on #3731 as well.

@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jul 11, 2019

172.30.32/24 is hardcoded in hassio-supervisor. I've been in contact with the team of hassio for the changes in this and most of the hass.sh is adapted from the original hassio-service file. https://github.com/home-assistant/hassio-installer/blob/master/files/hassio-supervisor

@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jul 20, 2019

@chickenandpork When can I expect this to be merged?

@chickenandpork

This comment has been minimized.

Copy link
Member

chickenandpork commented Jul 21, 2019

Often that depends on who else has time to test and verify. The builds for Synology are really expensive in terms of time and test tiers, so they get scheduled when there are some ready and the personal projects are also complete. My original python3.5.6 work took so long, 3.6.8 was out. Maybe needed more cross-platform verification.

I do think we need a way to automate some portion of testing so that we can check for compatibility a bit more autonomously. I’m used to working at companies that do this automatically, trying to think on how we can move to something a bit more efficient. I’ve got some work in that area to explore.

Personally, I’m curious to try your work in a week or so when I get back from travels.

@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jul 21, 2019

I have 16 ❤️ here not sure if that means users that have tested and verified but at least it's something 😄

Tack your time and enjoy your travels @chickenandpork

Edit:

I've also got 40 ⬆️ here

@jokerigno

This comment has been minimized.

Copy link

jokerigno commented Jul 24, 2019

Well, if this can help, I use it on my xpenology NAS for 2 weeks.
Very stable and reliable!

@ymartin59

This comment has been minimized.

Copy link
Contributor

ymartin59 commented Jul 28, 2019

@fredrike Thanks for this contribution.
I have a question about service start/stop, I find it strange to chain busybox start-stop-daemon, a custom script and then docker run. From my point of view (probably naive), it should be enough start_daemon method to invoke "docker run" and stop_daemon to invoke "docker stop". May you explain why implementation seems so "complicated"?

Copy link
Contributor

ymartin59 left a comment

As it is our first Docker-based package, many questions...
Then improvement proposal to reduce maintenance effort.

spk/hassio/Makefile Outdated Show resolved Hide resolved
spk/hassio/Makefile Outdated Show resolved Hide resolved
spk/hassio/src/hassio.sh Outdated Show resolved Hide resolved
spk/hassio/src/hassio.sh Outdated Show resolved Hide resolved
spk/hassio/Makefile Outdated Show resolved Hide resolved
@fredrike fredrike force-pushed the fredrike:hass.io branch from 4c86912 to eb505bb Aug 1, 2019
@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Aug 1, 2019

@ymartin59 When a package is upgraded, will the files in the new package replace the old ones (in my case the hassio.sh file)?

What is the best way to ensure that the hassio.json file is moved to etc when upgrading? Will this do it 46e4571?

@chickenandpork

This comment has been minimized.

Copy link
Member

chickenandpork commented Jan 13, 2020

What needs to happen to get this package/pr merged and published?

@ymartin59 has concerns
I raised one stopper

fredrike added 2 commits Jan 13, 2020
@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jan 13, 2020

What needs to happen to get this package/pr merged and published?

@ymartin59 has concerns
I raised one stopper

I think everything is addressed now :)

@fredrike fredrike requested review from chickenandpork and ymartin59 Jan 13, 2020
@fxstein

This comment has been minimized.

Copy link

fxstein commented Jan 13, 2020

@fredrike @chickenandpork Thank you both!

@bruvv

This comment has been minimized.

Copy link

bruvv commented Jan 14, 2020

After 6 months are we finally going to see this merged 😳
Thanks everyone involved especially @fredrike for your work! Can't wait to see this pop up in my app store

@G3rry71

This comment has been minimized.

Copy link

G3rry71 commented Jan 14, 2020

After 6 months are we finally going to see this merged 😳
Thanks everyone involved especially @fredrike for your work! Can't wait to see this pop up in my app store

Congratulation team!

@jokerigno

This comment has been minimized.

Copy link

jokerigno commented Jan 16, 2020

Thank you all!

@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jan 16, 2020

Thank you all!

It is still not merged so I'm not super optimistic..

@fxstein

This comment has been minimized.

Copy link

fxstein commented Jan 16, 2020

@chickenandpork @ymartin59 We all would very much appreciate your attention on getting this PR close out. Thank you very much!

@chickenandpork

This comment has been minimized.

Copy link
Member

chickenandpork commented Jan 16, 2020

7abccb39 eliminates the concerns I raised:

I think the existing work still suffers from lacking the restart that happens on a hassio on a RaspPi: when HA stops in a Pi, IIRC the Pi restarts. This means it doesn't need to get right the shutdown of addons and such. I want to clarify that I don't see this as a failing: there's a previous comment about being similar to upstream, but upstream has some luxuries :) I really appreciate @fredrike 's work here.

Stop and Start are cleaner than I saw before; this is awesome.

  • Stop still leaves homeassistant/amd64-hassio-dns:1 running, plus my selected add-ons, but I'm OK accepting a later fix for that as a bugfix. Removal stops homeassistant/amd64-hassio-dns:1 but not my selected add-ons

"Updating" 20191224-1 to 20191224-1 seems to make no change. Delete and install looks OK if I manually stop and delete my add-ons. Unlike previous runs, this is good in the mode of "just reinstall and it'll do the right thing". More people using this will clarify what circumstances images might still be running or any other issues.

I accept but I can't speak for @ymartin59

Be aware that accepted PRs become mergeable but there's a lag between that and building a release (based on space and cycles on a CI I think) but this one is a quick build so not as bad for this SPK.

@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jan 18, 2020

7abccb39 eliminates the concerns I raised:

I think the existing work still suffers from lacking the restart that happens on a hassio on a RaspPi: when HA stops in a Pi, IIRC the Pi restarts. This means it doesn't need to get right the shutdown of addons and such. I want to clarify that I don't see this as a failing: there's a previous comment about being similar to upstream, but upstream has some luxuries :) I really appreciate @fredrike 's work here.

Stop and Start are cleaner than I saw before; this is awesome.

  • Stop still leaves homeassistant/amd64-hassio-dns:1 running, plus my selected add-ons, but I'm OK accepting a later fix for that as a bugfix. Removal stops homeassistant/amd64-hassio-dns:1 but not my selected add-ons

Uninstall of the package should remove the dns container. It would be fairly easy to make sure that all containers are stopped when stopping the package. Together with removing all related images.

"Updating" 20191224-1 to 20191224-1 seems to make no change. Delete and install looks OK if I manually stop and delete my add-ons. Unlike previous runs, this is good in the mode of "just reinstall and it'll do the right thing". More people using this will clarify what circumstances images might still be running or any other issues.

I accept but I can't speak for @ymartin59

Be aware that accepted PRs become mergeable but there's a lag between that and building a release (based on space and cycles on a CI I think) but this one is a quick build so not as bad for this SPK.

@fxstein, do you have an idea of how to detect the missing entrypoint.js that happens once in a while (the fix is to restart the package)?

@chickenandpork

This comment has been minimized.

Copy link
Member

chickenandpork commented Jan 18, 2020

@ymartin59 I think this resolved the four issues you raised 2019-11-30:

  • SPK_VER: @fredrike went with a static YYYYMMDD manually changed as a version
  • SPK_REV: @fredrike change to 1
  • ADMIN_PORT: @fredrike removed
  • SERVICE_PORT 8123 collides with homeassistant::SERVICE_PORT: @fredrike marked the incompatibility in the SPK Makefile.

Looking for your OK with this work, and that any remaining issues can be pushed to a bug fix, so that @fredrike can get this merged.

docker network rm hassio

if [ "${wizard_remove_addons}" == "true" ]; then
# Remove addons

This comment has been minimized.

Copy link
@fredrike

fredrike Jan 19, 2020

Author Contributor

@chickenandpork, so you are saying that this doesn't work?

Copy link
Contributor

ymartin59 left a comment

OK I publish for x64 architectures as beta for a first revision.

STARTABLE = yes
DISPLAY_NAME = Hass.io
CHANGELOG = ""

This comment has been minimized.

Copy link
@ymartin59

ymartin59 Jan 19, 2020

Contributor
Suggested change
BETA = 1
@ymartin59 ymartin59 merged commit 814a835 into SynoCommunity:master Jan 19, 2020
@fredrike

This comment has been minimized.

Copy link
Contributor Author

fredrike commented Jan 20, 2020

OK I publish for x64 architectures as beta for a first revision.

Great, thanks! Docker is only supported on x64. May i ask why beta and why you are so skeptical on this?

@mp68

This comment has been minimized.

Copy link

mp68 commented Jan 23, 2020

Due to popular demand: When can we expect this to become available through the package manager?


service_poststop ()
{
docker stop hassio_supervisor homeassistant

This comment has been minimized.

Copy link
@fredrike

fredrike Jan 26, 2020

Author Contributor
ADDONS=$(jq -r '.user| keys| map("addon_"+.)| join(" ")' $(jq -r '.data + "/addons.json"' ${CFG_FILE}))
docker stop $ADDONS
@ymartin59

This comment has been minimized.

Copy link
Contributor

ymartin59 commented Jan 26, 2020

At the moment, publish fails. A fix in spkrepo or in package is required: SynoCommunity/spkrepo#39. Investigations in progress.

@ymartin59

This comment has been minimized.

Copy link
Contributor

ymartin59 commented Jan 26, 2020

@fredrike May you please register here with your github username: https://synocommunity.com/register so that I declare you as package owner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

10 participants
You can’t perform that action at this time.