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

Agent does not support bidirectional comms between service dependencies #2095

Closed
TheMosquito opened this issue Sep 23, 2020 · 9 comments
Closed

Comments

@TheMosquito
Copy link
Member

TheMosquito commented Sep 23, 2020

There is a problem with the way that we create Docker private virtual bridge networks between containers. In the situation where service A declares service B in its requiredServices array, but service B also needs to communicate with service A, it does not work. Declaration of a circular dependency graph for this is not allowed (i.e., we cannot declare that A requires B and B requires A) but it should be fine because A and B are on the same network, except for one thing . The agent fails to create an alias for service A on this network. That means that service B is unable to discover and connect with service A.

So currently, service B either needs to use the hex container ID, or the hard IP address of service A on this network to connect and it has no way to get either of these bits of information within the container context.

A workaround for this is to:

  1. remove the container from this network: docker network disconnect NETWORK CONTAINER
  2. add the container back onto this network with the appropriate alias (i.e., the name given in the services array): docker network connect --alias ALIAS NETWORK CONTAINER

The services A and B have names defined in the services arrays in their definition JSON, and this is the name we set as an alias in the bridge networks we create, except that right now we only apply this alias on service B. This means service B cannot reach service A. To fix this problem we need to add the alias (like my work-around does) to enable service B to connect to service A.

Doc issue 772 is open to remove this limitation from the documentation.

@RobHigh RobHigh added the p2 label Sep 23, 2020
@dabooz dabooz added p3 and removed p2 labels Sep 24, 2020
@dabooz
Copy link
Member

dabooz commented Sep 24, 2020

This is currently working as designed, so I can't agree that it is a bug. But I do agree it should be addressed at some point in the near future, so I'll compromise and leave it as a bug but lowering it to p3.

@RobHigh
Copy link
Member

RobHigh commented Sep 24, 2020

@dabooz - We may have address this different. It is a relatively urgent issue as it inhibits the case demonstrated with LF Edge itself as a deployable set of containers -- that is, we probably want to treat it with a higher priority than a P3 bug. It is constraining the work we're doing with Intel on their integration with IEAM.

@RobHigh
Copy link
Member

RobHigh commented Sep 24, 2020

Philosophically it does seem odd to me that subordinate services can not communicate with their superior services on the network that is configured for the dependency between them.

@linggao linggao assigned linggao and unassigned linggao Sep 24, 2020
@sirotsinskuy
Copy link

@linggao Working on it

@linggao linggao assigned sirotsinskuy and unassigned linggao Sep 29, 2020
@TheMosquito
Copy link
Member Author

@sirotsinskuy Is there any update on this issue? This issue is blocking work on a project with one of our partners.

@TheMosquito
Copy link
Member Author

I just tested putting a set of containers into a single Open-Horizon service. I discovered that when multiple containers are deployed together inside a single service:

  1. they share yet another virtual bridge network (one I had never heard of before), and
  2. most importantly they are given alias names on that network (so DNS should work).
    This means that using a single service is a work-around for this issue.
    Of course, this is not best practice, because it completely eliminates the advantages of a microservice architecture with multiple services (because this way all of the services must be updated together).

@sirotsinskuy
Copy link

@TheMosquito I proposed the possible solution to @dabooz and currently waiting for his decision/review of it.

@dabooz dabooz changed the title Agent fails to create a network alias for the *calling* service (required for bidirectional comms) Agent does not support bidirectional comms between service dependencies Oct 21, 2020
@dabooz dabooz added feature and removed bug p3 labels Jan 11, 2021
@dabooz
Copy link
Member

dabooz commented Jan 14, 2021

Once this fix is incorporated, here is what users can expect:
Suppose A depends on B and C, and B Depends on D and E.

  1. A will be able to call B and C, but not D or E.
  2. B and C will be able to call A, but no other service can call A. Likewise, D and E can call B.
  3. Suppose we replace E with C and C is a singleton, then there will be 1 C and it can call both B and A, because it is a child of both. If it weren't a singleton, there would be 2 C's, one of them could call B and other could call A.
  4. Back to the original dependency tree, neither B nor C can call each other, same for D and E. Dependencies are not in some sort of "common pool" that enables them to interact with each other.

@dabooz
Copy link
Member

dabooz commented Feb 4, 2021

Finally have a PR for this issue. my testing covered various combinations of the following:

  1. hzn dev service start and stop
  2. use of the real agent in the built in (e2edev) test environment
  3. service dependency trees larger than 3
  4. more than 1 to 3 peers at various levels of the dependency tree
  5. singletons shared across agreements
  6. singletons within the same dependency tree and at various levels in the dependency tree
  7. non-singletons as per (6)
  8. services with multiple containers
  9. curl from parent to dependency and reverse, both working and correctly non-working cases
  10. killing containers and allow the agent to restart and re-construct the networks correctly

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

No branches or pull requests

5 participants