Zeroconf-based Kolibri P2P discovery system #5660

jamalex · 2019-06-12T20:59:27Z

Summary

We support local network peer connections for content import, and soon, for data syncing. But right now, it's necessary to manually enter the IP and port, which is at best inconvenient, and at worst an insurmountable barrier for many users.

Enter zeroconf! It's specifically designed for this type of situation -- allowing nearby devices on the same network to discover one another and the services they offer.

This PR is a preliminary, but usable, backend implementation of a zeroconf-based "Kolibri service broadcasting and discovery" system. When Kolibri starts up, it will register itself on the local network, and then monitor and track any other instances of Kolibri on the same local network. The frontend code can then call an API endpoint to enumerate the nearby Kolibri instances, including information about what IP/port they're listening on, and what facilities/channels they have loaded on them.

One of the lovely side-effects of using zeroconf (aka mDNS) for this is that it also ends up creating local
area domain names that can be resolved and used in the browser at least on Linux and OSX:

Note: includes some changes I made to https://github.com/learningequality/python-zeroconf

Reviewer guidance

To test, you'll want to spin up a couple instances of this. To start with, these could be on the same computer, just using different ports and KOLIBRI_HOME directories, e.g.:

# run this in one terminal tab
kolibri start

# run this in another tab
KOLIBRI_HOME=~/.kolibri-temp KOLIBRI_LISTEN_PORT=9090 kolibri start

Then, login as an admin on each and go to:
http://127.0.0.1:8080/api/discovery/networksearch/
and
http://127.0.0.1:9090/api/discovery/networksearch/

and you should see something like this:

[
    {
        "ip": "192.168.1.25",
        "self": true,
        "data": {
            "channels": [
                {
                    "id": "c7eda62c6489554a941058fa883e7c2c",
                    "name": "Better World Ed"
                },
                {
                    "id": "74f36493bb475b62935fa8705ed59fed",
                    "name": "Thoughtful Learning"
                },
                {
                    "id": "f9da12749d995fa197f8b4c0192e7b2c",
                    "name": "PraDigi"
                }
            ],
            "facilities": [
                {
                    "dataset_id": "d265a3809dd02de42570ef5ba5b1bb86",
                    "id": "318aa7335a2ad82434dee8534caf85d9",
                    "name": "a"
                }
            ]
        },
        "port": 8080,
        "host": "b053.kolibri.local",
        "local": true,
        "id": "b053"
    }
]

Contributor Checklist

PR process:

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Testing:

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

Reviewer Checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

TODO:

Add an option in options.ini to disable zeroconf
Consider switching service type to _http._tcp.local so it's standardized.
Rebroadcast service when channels or facilities change

codecov · 2019-06-12T22:59:19Z

Codecov Report

Merging #5660 into develop will increase coverage by 0.13%.
The diff coverage is 85.71%.

Impacted Files	Coverage Δ
kolibri/core/discovery/api.py	`100% <100%> (ø)`	⬆️
kolibri/core/discovery/api_urls.py	`100% <100%> (ø)`	⬆️
kolibri/utils/server.py	`46.01% <25%> (-0.68%)`	⬇️
kolibri/utils/cli.py	`66.14% <57.14%> (-0.63%)`	⬇️
kolibri/utils/system.py	`40.62% <77.77%> (+6.72%)`	⬆️
kolibri/core/discovery/utils/network/search.py	`92.85% <92.85%> (ø)`
kolibri/utils/env.py	`87.5% <0%> (-6.25%)`	⬇️
...olibri/core/content/utils/import_export_content.py	`84.48% <0%> (-1.73%)`	⬇️
test/__init__.py
... and 2 more

codecov · 2019-06-12T22:59:19Z

Codecov Report

Merging #5660 into develop will increase coverage by 20.34%.
The diff coverage is 83.1%.

Impacted Files	Coverage Δ
kolibri/core/discovery/api.py	`100% <100%> (ø)`	⬆️
kolibri/core/discovery/api_urls.py	`100% <100%> (ø)`	⬆️
kolibri/utils/server.py	`46.01% <25%> (-0.68%)`	⬇️
kolibri/utils/cli.py	`65.94% <46.66%> (-0.83%)`	⬇️
kolibri/utils/system.py	`40.62% <77.77%> (+6.72%)`	⬆️
kolibri/core/discovery/utils/network/search.py	`92.85% <92.85%> (ø)`
kolibri/core/apps.py	`91.3% <0%> (-8.7%)`	⬇️
kolibri/core/content/test/sqlalchemytesting.py	`84% <0%> (-8%)`	⬇️
kolibri/core/deviceadmin/utils.py	`93.58% <0%> (-5.13%)`	⬇️
kolibri/core/notifications/models.py	`87.67% <0%> (-4.11%)`	⬇️
... and 552 more

jredrejo · 2019-06-14T15:52:02Z

kolibri/core/discovery/utils/network/search.py

+    return instances
+
+
+def register_zeroconf_service(port, id):


Why does this function need the port as a parameter?
Is not possible to get it from the options or reading it from the PID_FILE as it is done in other parts of the code?
This port option pushes changes in many parts of this PR code that might not be needed.

Ah, PID_FILE could be a good place, yes. I originally wanted to pull it from conf.OPTIONS, except it can be overridden with --port=XYZ, which doesn't get stored anywhere (except in the PID file, I guess), just passed in to the start command function as an argument, which was the pattern I followed.

Yes, I understand your concerns but, as I understand the code, the value in the PID file is affected both by conf.OPTIONS and env variables, thus it should be the real value for the port. Taking it from there would avoid many modifications you had to do just to drag the port parameter here.

Yep, agreed! Can't see any issues with that approach. Will update.

The PID is also written into there for kolibri-server, yes? Based on what uwsgi/nginx are exposing?

kolibri-server takes the value from conf.OPTIONS because there's no warranty the part of code in Kolibri that saves the PID has been executed before kolibri-server starts. The env variable option does not have sense when running it as a daemon.
After kolibri finishes the starting process, the PID file will have the same value that is in conf.OPTIONS so I think we can consider we are safe on this side too.

kolibri/utils/cli.py

(also means that we start with a soft interrupt and only hard-kill later if needed, as we do on Linux)

benjaoming

This looks so promising!! Am already thinking about what it can contribute to in terms of documentation simplification about static IPs -- that if MDNS is supported widely (IDK) then we could implement the naming of a device in the setup process <unique-device-name>.kolibri.local.

kolibri/utils/server.py

requirements/base.txt

benjaoming · 2019-06-18T21:05:22Z

~~@jamalex a quick idea: When using custom names, could the device try to look up its own name and report back if there is a conflict?~~

Scratch that, you already knew 💯

benjaoming · 2019-06-18T21:07:37Z

kolibri/core/discovery/utils/network/search.py

+        assert self.info is None, "Service is already registered!"
+
+        i = 1
+        id = self.id


This assigns to a builtin, or is it just me?

Seems so, yep! Perhaps I'll rename the local var to unique_id

jamalex · 2019-06-20T01:24:49Z

This looks so promising!! Am already thinking about what it can contribute to in terms of documentation simplification about static IPs -- that if MDNS is supported widely (IDK) then we could implement the naming of a device in the setup process .kolibri.local

It's sooooooo close to a possibility, EXCEPT that Android doesn't implement any version of mDNS/zeroconf/etc.... and that's one of our most common client devices. 😿

What I'm thinking is that we could make lightweight "launcher" apps for Android/Windows/Linux that do nothing other than quickly scan the local network and find the IP for a Kolibri instance (possibly caching/preferring a specific one after first use), and then pop it open in the web browser. Would be a huge timesaver and headache-avoider in itself.

requirements/base.txt

kolibri/core/discovery/utils/network/search.py

indirectlylit · 2019-10-30T22:49:10Z

@rtibbles how do you think this work will be affected by #6007

rtibbles · 2019-10-30T23:04:52Z

Looks like any background tasks are managed inside zeroconf - so, apart from superficial merge conflicts, I don't think it would be affected.

indirectlylit · 2019-10-30T23:12:33Z

Looks like any background tasks are managed inside zeroconf - so, apart from superficial merge conflicts, I don't think it would be affected.

okay cool. wasn't sure if you were going to want to leverage iceqube machinery for this

indirectlylit · 2019-10-30T23:25:45Z

migrating to #6011 and https://github.com/learningequality/kolibri/tree/zeroconf so that @micahscopes can work on it

jamalex added the TODO: needs review Waiting for review label Jun 12, 2019

jamalex added this to the 0.13.0 milestone Jun 12, 2019

jamalex requested review from jredrejo, rtibbles and indirectlylit June 12, 2019 20:59

jamalex changed the title ~~Zeroconf~~ Zeroconf-based Kolibri P2P discovery system (backend only) Jun 12, 2019

jamalex force-pushed the zeroconf branch from 7d50eb7 to a394461 Compare June 12, 2019 22:26

jredrejo reviewed Jun 14, 2019

View reviewed changes

jamalex added 12 commits June 15, 2019 14:30

Update _kill_pid to leverage os.kill for Windows as well (Py2.7+)

a543930

(also means that we start with a soft interrupt and only hard-kill later if needed, as we do on Linux)

Utils for broadcasting/scanning zeroconf Kolibri service for discovery

2c3c0cf

Pass the currently specified port through into the services functions.

2922c0f

Register the Kolibri zeroconf service so it will be discoverable

e923e50

Gracefully clean up and unregister Kolibri zeroconf service on shutdown

7d40f1e

Add network search API endpoint for enumerating found Kolibri instances

ffe47c7

Run black and flake8 on files that still needed it to pass CI

bb2d08a

Fix zeroconf github URL in requirements.txt

6648666

Remove .egg-info directories from kolibri/dist as they cause errors

a75293d

Update to Morango 0.4.6

8ae6bbc

Gracefully handle server being started within a thread (e.g. Android)

df7523b

Initial prototype frontend code for zeroconf device discovery.

2881db3

jamalex force-pushed the zeroconf branch from 389d41b to 2881db3 Compare June 17, 2019 08:48

jamalex added 2 commits June 17, 2019 10:09

Add base_url to network search results expectations for tests

6dec95c

Put facility name in radio item for device list

7c07682

benjaoming reviewed Jun 18, 2019

View reviewed changes

kolibri/utils/server.py Show resolved Hide resolved

requirements/base.txt Outdated Show resolved Hide resolved

benjaoming reviewed Jun 18, 2019

View reviewed changes

jamalex changed the title ~~Zeroconf-based Kolibri P2P discovery system (backend only)~~ Zeroconf-based Kolibri P2P discovery system Jun 20, 2019

Update zeroconf to 0.19.3 from PyPI

30e13a8

benjaoming reviewed Jun 20, 2019

View reviewed changes

requirements/base.txt Show resolved Hide resolved

benjaoming reviewed Jun 20, 2019

View reviewed changes

kolibri/core/discovery/utils/network/search.py Show resolved Hide resolved

indirectlylit mentioned this pull request Oct 30, 2019

Content import peer discovery #6011

Merged

9 tasks

indirectlylit closed this Oct 30, 2019

jonboiser removed the TODO: needs review Waiting for review label Dec 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zeroconf-based Kolibri P2P discovery system #5660

Zeroconf-based Kolibri P2P discovery system #5660

jamalex commented Jun 12, 2019 •

edited

Loading

codecov bot commented Jun 12, 2019

codecov bot commented Jun 12, 2019 •

edited

Loading

jredrejo Jun 14, 2019

jamalex Jun 17, 2019 •

edited

Loading

jredrejo Jun 18, 2019

jamalex Jun 20, 2019

jredrejo Jun 21, 2019

benjaoming left a comment

benjaoming commented Jun 18, 2019 •

edited

Loading

benjaoming Jun 18, 2019

jamalex Jun 20, 2019

jamalex commented Jun 20, 2019

indirectlylit commented Oct 30, 2019

rtibbles commented Oct 30, 2019

indirectlylit commented Oct 30, 2019

indirectlylit commented Oct 30, 2019

Zeroconf-based Kolibri P2P discovery system #5660

Zeroconf-based Kolibri P2P discovery system #5660

Conversation

jamalex commented Jun 12, 2019 • edited Loading

Summary

Reviewer guidance

Contributor Checklist

Reviewer Checklist

codecov bot commented Jun 12, 2019

Codecov Report

codecov bot commented Jun 12, 2019 • edited Loading

Codecov Report

jredrejo Jun 14, 2019

Choose a reason for hiding this comment

jamalex Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

jredrejo Jun 18, 2019

Choose a reason for hiding this comment

jamalex Jun 20, 2019

Choose a reason for hiding this comment

jredrejo Jun 21, 2019

Choose a reason for hiding this comment

benjaoming left a comment

Choose a reason for hiding this comment

benjaoming commented Jun 18, 2019 • edited Loading

benjaoming Jun 18, 2019

Choose a reason for hiding this comment

jamalex Jun 20, 2019

Choose a reason for hiding this comment

jamalex commented Jun 20, 2019

indirectlylit commented Oct 30, 2019

rtibbles commented Oct 30, 2019

indirectlylit commented Oct 30, 2019

indirectlylit commented Oct 30, 2019

jamalex commented Jun 12, 2019 •

edited

Loading

codecov bot commented Jun 12, 2019 •

edited

Loading

jamalex Jun 17, 2019 •

edited

Loading

benjaoming commented Jun 18, 2019 •

edited

Loading