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

A Simple Maubot role: take 2 #622

Closed
wants to merge 22 commits into from
Closed

A Simple Maubot role: take 2 #622

wants to merge 22 commits into from

Conversation

Cadair
Copy link
Contributor

@Cadair Cadair commented Aug 5, 2020

I recently resurrected this from #373

I know that there is a more detailed maubot role that @aaronraimist and dangersalad were working on, I am interested in what would needed to be added to this though, as it's currently just using the upstream image and seems to be working fine for me.

Todo as I see it at the moment:

  • Make exposing the management port optional and default to off
  • Add integration with the built in nginx server to proxy the management UI
  • Ensure that maubot is registered for systemd start on the always tag

@Cadair Cadair changed the title A Simple Maubot role take 2 A Simple Maubot role: take 2 Aug 5, 2020
Copy link
Owner

@spantaleev spantaleev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken a look and found some little things that can be improved! Thank you for adding maubot support!

@@ -0,0 +1,126 @@
---
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If one does --tags=setup-all now, this won't get configured, would it?

Likewise, when doing --tags=start, this won't be started, because it doesn't inject itself into matrix_systemd_services_list on an always tag.

Other roles split this out into an init.yml file, which executes on the always tag, like this:

- import_tasks: "{{ role_path }}/tasks/init.yml"

And then init.yml takes care of the common setup (systemd service injection, nginx proxy configuration injection) like this:

Would you split things into main.yml, init.yml and setup.yml?

roles/matrix-maubot/templates/maubot_config.yaml.j2 Outdated Show resolved Hide resolved
roles/matrix-maubot/defaults/main.yml Outdated Show resolved Hide resolved
@spantaleev
Copy link
Owner

And one other minor thing: we should probably rename this role from matrix-maubot to matrix-bot-maubot, to be consistent with our other bot roles (well, it's only matrix-bot-matrix-reminder-bot for now). We prefix bridge roles similarly (matrix-bridge-..).

I believe as the number of bridge/bot roles grows, it makes things much easier to figure out.


By the way, setup.yml should also be changed to include matrix-bot-maubot. Since this bot role tries to inject some configuration into matrix-nginx-proxy, it should be listed somewhere before it.

@spantaleev
Copy link
Owner

Should we get this merged?

I've sent some feedback about it before. About these latest changes, I do wonder why that 29316 port gets exposed publicly. Should we make that configurable? Or should some nginx configuration be injected, so that this bot exposes its HTTP interface at some URL served via nginx? If it's an HTTP interface at 29316 even.

@Cadair
Copy link
Contributor Author

Cadair commented Jan 25, 2021

I think I have the config in place for the nginx proxy, but it doesn't seem to be working, so I added the port just as a hack because I needed quick access to the management interface.

The biggest issue I have with this PR at the moment, and the reason why I haven't worked on it much (other than time) is that I can't reliably get it started after synapse, and if synapse isn't responding when it starts up it kills itself irrecoverably.

@spantaleev
Copy link
Owner

Interesting.. doesn't the delay of 60 seconds help with that? Also, irrecoverably doesn't sound good.. Why doesn't systemd restart it like any other failing service?

@Cadair
Copy link
Contributor Author

Cadair commented Jan 25, 2021

A manual restart of the systemd unit gets it back but nothing else does. I am not sure if it ends up reporting as failed to systemd, just sitting there "running" but useless.

I added a delay and it helped, but it doesn't always come back.

@Svarto
Copy link

Svarto commented Feb 25, 2021

@Cadair @spantaleev would love to get this included, hmm, is there no healthcheck or possibility to ping the container to see if it is responding - and if not, restart? Perhaps through a cronjob?

@laszabine
Copy link
Contributor

I fixed some of the things to fix and submitted a PR into @Cadair's fork.

@Cadair
Copy link
Contributor Author

Cadair commented Jun 22, 2021

@laszabine Does the nginx proxy work for you with this branch as is? It is 404ing for me.

@laszabine
Copy link
Contributor

@laszabine Does the nginx proxy work for you with this branch as is? It is 404ing for me.

@Cadair You're right, it's not working. I hadn't understood that it was desirable for the manager website to be publicly exposed. Just submitted a new PR to your branch that should fix this.

@Cadair
Copy link
Contributor Author

Cadair commented Jul 8, 2021

Ah awesome thanks. I wasn't sure if it was supposed to work or not :D

@Cadair
Copy link
Contributor Author

Cadair commented Jul 9, 2021

I am still really short on time to work on this. If anyone wants to get it across the line with the few last changes (todo in the top comment) then send me a PR :)

@0-MegaMind-0
Copy link

We would need to handle pip module installations too. Since maubot currently doesn't support auto dependency installation and any manually installed modules are lost on restart.

@davidmehren
Copy link
Contributor

I'm successfully using this branch to run a maubot instance.
Regarding installing custom python dependencies, I resorted to a hack:
In matrix-maubot.service.j2 I overrode the entrypoint by changing line 31 to

{{ matrix_bot_maubot_docker_image }} sh -c 'apk add py3-pyldap && /opt/maubot/docker/run.sh'

which installs pyldap and then starts maubot.

To add custom python modules, it seems neccessary to override the entrypoint, as otherwise maubot instantly starts while the custom modules are not yet installed. So any automation of that on our side (not maubot adding dependency-installing natively) will allways be quite a hack.

@thefeiter
Copy link

Hey there, I am very interested in this role. Since I have no experience in creating this role myself, I would really appreciate this branch getting finished and merged.

@shukon
Copy link
Contributor

shukon commented May 29, 2022

Anything except for resolving the yaml-lint-complaints one could do to help this @Cadair ? Not sure I can make things better on the python-dep-front, but if there is a specific thing to do and someone could point me there, I'd be willing to help.

@Cadair
Copy link
Contributor Author

Cadair commented Jun 24, 2022

Closing in favour of #1894

@Cadair Cadair closed this Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants