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

Wrong IP addresses registered when using Docker overlay network #20

Open
znarthur opened this issue Oct 30, 2019 · 4 comments
Open

Wrong IP addresses registered when using Docker overlay network #20

znarthur opened this issue Oct 30, 2019 · 4 comments
Assignees
Labels

Comments

@znarthur
Copy link

znarthur commented Oct 30, 2019

Using 170: peername = request.transport.get_extra_info('peername') in headnode.py fetches the node IP of the overlay network, and not the IP of the registering service/data node.

I replaced line 170 with a few lines of code into the register function to accept an 'addr' field from the registering node:

   if 'addr' not in body:
       peername = request.transport.get_extra_info('peername')
       if peername is None:
           raise HTTPBadRequest(reason="Can not determine caller IP")
   elif len(body['addr']) == 2:
       peername = body['addr']
   else:
       raise HTTPBadRequest(reason="Can not determine caller IP")

and I added the following function to basenode.py to be invoked while sending the register signal:

def getNodeIp(node_type):
    """ Gets IP of local host (container) for the default route """
    log.info("Node type: %s" % node_type)
    if node_type == 'sn':
        if config.get("sn_port"):
            port = config.get("sn_port")
    elif node_type == 'dn':
        if config.get("dn_port"):
            port = config.get("dn_port")
    else:
        port = 6101
    IP = ((([ip for ip in socket.gethostbyname_ex(socket.gethostname())[2] if not ip.startswith("127.")] or [[(s.connect(("8.8.8.8", 53)), s.getsockname()[0], s.close()) for s in [socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]]) + ["no IP found"])[0], port)
    log.info("sending node ip: %s:%s" % IP)
    return IP

async def register(app):
    """ register node with headnode
    OK to call idempotently (e.g. if the headnode seems to have forgotten us)"""
    head_url = getHeadUrl(app)
    if not head_url:
        log.warn("head_url is not set, can not register yet")
        return
    req_reg = head_url + "/register"
    log.info("register: {}".format(req_reg))
    addr = getNodeIp(app["node_type"])
    if addr:
        body = {"id": app["id"], "port": app["node_port"], "node_type": app["node_type"], "addr": addr}
    else:
        body = {"id": app["id"], "port": app["node_port"], "node_type": app["node_type"]}
.... etc
@znarthur
Copy link
Author

I should mention I was using a docker image I built from a version of hsds that was cloned from the master branch two weeks ago. I am using docker version 19.03.1, and running the hsds nodes using docker swarm.

@s004pmg
Copy link
Contributor

s004pmg commented Mar 11, 2020

I agree that the behavior around HSDS binding to a particular IP needs improvement. However, I'm not sure that this part is the way to do it:

IP = ((([ip for ip in socket.gethostbyname_ex(socket.gethostname())[2] if not ip.startswith("127.")] or [[(s.connect(("8.8.8.8", 53)), s.getsockname()[0], s.close()) for s in [socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]]) + ["no IP found"])[0], port)

My concerns here are that there are more private networks allowed than just the 127 network, some of the HSDS deployments won't be able to reach out to 8.8.8.8, some hostnames might not be exposed to the local DNS, and this code doesn't work on IPV6 networks.

However, I do agree that the IP discovery code needs improvement.

@jreadey
Copy link
Member

jreadey commented Mar 11, 2020

Just to throw this out... for Kubernetes deployments there is no head node at all. Instead SN and Dn nodes use the Kubernetes API to get IPs for the other nodes (see line 279 bin basenode.py).

I don't know if this approach would work for Docker. I couldn't find anything on using the Docker SDK from a container.

@s004pmg
Copy link
Contributor

s004pmg commented Mar 11, 2020

I think that the main point @znarthur makes here is that there are usually multiple IPs associated with a docker container - a local docker network only IP, and possibly a public facing IP. Picking the wrong one can be bad a few different ways. For one, the bound port might be for the other IP. Also, putting the service node on a docker private IP breaks access.

For HSDS on docker, it might just be too hard to predict what the sysadmin wants, and HSDS might have to use an explicit IP:port configuration approach.

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

No branches or pull requests

3 participants