Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

docs(src/applications/inter-app-communication.md) Add first draft #723

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

chexxor
Copy link
Contributor

@chexxor chexxor commented Feb 4, 2017

Here's a first draft at documenting communication between Deis apps, which would fix https://github.com/deis/workflow/issues/

I spent a lot of time trying to come up with a simple example of an http-client app which resolves an app's URL to its internal IP address. I couldn't find an existing docker image which runs properly on Deis, as I don't have a great understanding of what kind of docker image a Deis app can simply run. I found a "ping" image which successfully demonstrates DNS resolution, but the pings fail when run in Deis.

LMK of any other info which should be contained here. I'll probably take a second review later.

@chexxor chexxor changed the title Add first draft of inter-app communication. docs(src/applications/inter-app-communication.md) Add first draft Feb 4, 2017
@mboersma mboersma added this to the v2.12 milestone Feb 15, 2017
@mboersma
Copy link
Member

This is a good start on something that would be very useful to users. We should refer to "Workflow" or "Deis Workflow" consistently, instead of the old shorthand "Deis."

@mboersma mboersma modified the milestones: v2.13, v2.12 Feb 27, 2017
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

This is great! Requested change is same as #723 (comment)

@vdice vdice requested a review from mboersma March 10, 2017 17:52
@chexxor
Copy link
Contributor Author

chexxor commented Mar 10, 2017

I've got a lot of PRs in many repos, so this one slipped off my radar. Thanks for the review!

@vdice vdice added the LGTM1 label Mar 10, 2017
@chexxor
Copy link
Contributor Author

chexxor commented Mar 10, 2017

Do we need to do something in mkdocs.yml to surface this new page? I was browsing this repo and came across that file.

@bacongobbler
Copy link
Member

Yes indeed, a change to mkdocs.yml to add it to the TOC will be required in order for this page to be available.

@chexxor
Copy link
Contributor Author

chexxor commented Mar 11, 2017

I've added the page to the mkdocs.yml.

Also, I've added a section to explain a closely related Deis Workflow feature: deis routing:disable. I want to add this so that I can link to the Deis blog post which served as great reference while creating this doc page.

@vdice
Copy link
Member

vdice commented Mar 13, 2017

LGTM still stands -- good catch on mkdocs addition!

For example, if you deployed a Deis Workflow app like this:

$ deis create http-server --no-remote
$ deis pull deis/example-dockerfile-python:latest -a http-server
Copy link
Member

@bacongobbler bacongobbler Mar 13, 2017

Choose a reason for hiding this comment

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

example-go is usually the app of choice in the documentation, as the image is 10x smaller than example-dockerfile-python:

><> docker images | grep example
deis/example-dockerfile-python                   latest                        811c39971f48        12 weeks ago        43.5 MB
deis/example-go                                  latest                        6d128cdcba34        2 years ago         4.072 MB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get a readiness check failure when using deis/example-go.

$ deis pull deis/example-go -a http-server
Creating build... Error: Unknown Error (400): {"detail":"(app::deploy): Readiness probe failed: dial tcp 10.68.7.165:8080: getsockopt: connection refused"}

Same error when pulling "deis/example-dockerfile-python:latest", so I dunno how to fix.

$ deis create http-client --no-remote
$ # Need to set a port because the docker image doesn't use one.
$ deis config:set PORT=8080 -a http-client
$ deis config:set HOSTNAME=http-server.http-server
Copy link
Member

Choose a reason for hiding this comment

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

You can set both of these in one line:

deis config:set PORT=8080 HOSTNAME=http-server.http-server -a http-client


Clean up:

$ deis destroy -a http-server --confirm=http-server
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe cleanup should be documented. The user following the guide may want to play around with the example some more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagree. The kubernetes tutorial docs demonstrate cleanup hello-minikube. Also, this command and its comment are pretty clear, so a user wanted to continue playing can simply wait to execute this until they are done playing. Or they can restart from the beginning to start playing again.

$ # Need to set a port because the docker image doesn't use one.
$ deis config:set PORT=8080 -a http-client
$ deis config:set HOSTNAME=http-server.http-server
$ deis pull -a http-client willfarrell/ping:latest
Copy link
Member

@bacongobbler bacongobbler Mar 13, 2017

Choose a reason for hiding this comment

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

Running deis pull on this image fails because it does not listen on port 8080 and fails the default health check. I don't think it's useful for a demonstration if the app fails to be deployed.

><> deis pull -a http-client willfarrell/ping:latest
Creating build... Error: Unknown Error (400): {"detail":"(app::deploy): Readiness probe failed: dial tcp 172.17.0.26:8080: getsockopt: connection refused"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need a better suggestion if I'm going to change it. I spent a lot of time thinking about how to demonstrate inter-app comms, and this is the best I could find.

Copy link
Member

Choose a reason for hiding this comment

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


### Environment Variable Service Discovery

It's useful to note that Kubernetes also supports [environment variable service discovery](https://kubernetes.io/docs/user-guide/services/#environment-variables), which works by adding environment variables to an app's environment to provide a reference to other services in the app's namespace. This method is currently not supported by Deis Workflow because Workflow creates a separate namespace for each app it deploys.
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't document this if we don't plan on supporting it. Either this should reference a ticket like deis/controller#710 to make an action item or should be removed completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's my understanding that Deis wants to follow Heroku's model, and Heroku doesn't support separate namespaces for each. That issue doesn't indicate any desire by core team to support defining a namespace. It's better to document this design decision than make people wonder.


### Related Topics

A common architecture pattern of multi-process applications is to have one process serve public requests while having multiple other processes supporting the public one to, for example, perform actions on a schedule or process work items from a queue. To implement this system of apps in Deis Workflow, set up the apps to communicate using DNS resolution, as shown above, and hide the supporting processes from public view by removing them from the Deis Workflow router.
Copy link
Member

Choose a reason for hiding this comment

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

I think this should go up further as a supporting use case for inter-app communication rather than a separate heading.


A common architecture pattern of multi-process applications is to have one process serve public requests while having multiple other processes supporting the public one to, for example, perform actions on a schedule or process work items from a queue. To implement this system of apps in Deis Workflow, set up the apps to communicate using DNS resolution, as shown above, and hide the supporting processes from public view by removing them from the Deis Workflow router.

deis routing:disable -a supporting-scheduled-app
Copy link
Member

Choose a reason for hiding this comment

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

missing $ character to note this is a console command.


Deis Workflow supports deploying a single app composed of a system of processes. Each Deis Workflow app communicates on a single port, so communicating with another Workflow app means finding that app's address and port. All Workflow apps are mapped to port 80 externally, so finding its IP address is the only challenge. Workflow creates a [Kubernetes Service](https://kubernetes.io/docs/user-guide/services/) for each app, which effectively assigns a name and one cluster-internal IP address to an app. The DNS service running in the cluster adds and removes DNS records which point from the app name to its IP address as services are added and removed. Deis Workflow apps, then, can simply send requests to the domain name given to the service, which is "app-name.app-namespace".

For example, if you deployed a Deis Workflow app like this:
Copy link
Member

@bacongobbler bacongobbler Mar 13, 2017

Choose a reason for hiding this comment

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

Overall I think the documentation is going in the right direction, but I don't think the demonstration is useful. Perhaps just remove the demo and leave the last sentence?

The blog post "Private Applications on Workflow" provides more details by walking through an example of internal cluster DNS communication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What isn't useful about the demonstration? Can we find a better one?

Copy link
Member

Choose a reason for hiding this comment

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

The blog post I linked has two working apps that communicate internally using DNS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The blog post is nice, but the http client is more complicated than I like. It requires knowledge of nginx, proxy, and authentication. This example only requires knowledge of unix ping, which is more simple, IMO.

Also, a blog post is different than a doc page. Linking to a blog post isn't an acceptable form of docs, IMO. For example, if CLI tool changes, then we can't grep the doc repo to know there's a doc page we need to update.

@bacongobbler
Copy link
Member

ping @chexxor, will you be able to come back to this, or shall I remove this from the v2.13.0 milestone?

@chexxor
Copy link
Contributor Author

chexxor commented Mar 22, 2017

Updated. Thanks for review!

@bacongobbler
Copy link
Member

bacongobbler commented Mar 22, 2017

If you wouldn't mind amending your commit(s) such that it follows the commit style guide, that would be awesome! That way it'll get picked up by deisrel when we generate the release changelog. Thanks!

Add first draft of inter-app communication.
Apply changes requested by review.
@chexxor chexxor force-pushed the add-inter-app-comm-docs branch from ab525fa to 8980c62 Compare March 22, 2017 20:57
@chexxor
Copy link
Contributor Author

chexxor commented Mar 22, 2017

Thanks for maintaining!

@bacongobbler
Copy link
Member

No, thank you.

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

Successfully merging this pull request may close these issues.

4 participants