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

Show attached runs in emp ps. #911

Merged
merged 10 commits into from
Jul 11, 2016
Merged

Show attached runs in emp ps. #911

merged 10 commits into from
Jul 11, 2016

Conversation

ejholmes
Copy link
Contributor

@ejholmes ejholmes commented Jul 1, 2016

Closes #905

This allows us to easily show attached runs in the output of emp ps:

$ emp run sh -a acme-inc
$ emp ps -a acme-inc
v33.run.dcc5bf133f56                          256:512.00mb  RUNNING  11s  sh
v33.web.f25d25e2-6b39-4fd2-833a-b2353082feb1  1X            RUNNING  23h  "acme-inc server"

This is implemented by using ListContainers to find any containers tagged with attached-run. Obviously this won't work consistently if you have more than 1 instance of Empire pointed at multiple Docker daemons.

My suggestion is that we update the docs to encourage people to point Empire at a single Docker daemon (either a single physical Docker daemon, or a Docker swarm). There's other benefits to doing this:

  1. docker pulls's when deploying will generally be faster, since it will hit cache more frequently with a single Docker daemon.
  2. It's easier to lock down security group rules if the Docker daemon is off of the host that Empire is running on (if you use the same Docker daemon that Empire is running as, people in emp run's have the same permissions as Empire to create infrastructure).
  3. It gives you more control over when runs are shutdown. For example, it's not uncommon that we roll out new AMI's and that process will kill long running migrations. Having a dedicated cluster/instance for emp run's gives you more control to ensure that no runs are in progress.

TODO

  • Update Production Best Practices docs about pointing Empire at a separate Docker cluster.
  • Update CHANGELOG.
  • Run Instances method in goroutine.

return instances, err
}

attachedInstances, err := s.dockerScheduler.InstancesFromAttachedRuns(ctx, app)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should get kicked off in a goroutine.

@phobologic
Copy link
Contributor

I haven't messed w/ docker swarm, but is there anyway we could run docker swarm's API on both empire hosts, and have them controlling the same docker instances? Then we could just point the Empire daemon at the local swarm, and still have a separate set of instances to do this on?

@@ -1,6 +1,171 @@
// Package docker implements the Scheduler interface backed by the Docker API.
// This implementation is not recommended for production use, but can be used in
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be updated?

@ejholmes
Copy link
Contributor Author

ejholmes commented Jul 1, 2016

@phobologic yep, that would work well too. Although, I think our production recommendation should be to have emp run's be off of the host that Empire runs in for better security.

@ejholmes
Copy link
Contributor Author

ejholmes commented Jul 1, 2016

I think probably the easiest thing to do for most people would be to run a single EC2 host dedicated to emp run's, expose Docker over TCP, then point the Empire daemon at that host. Docker swarm can be used to scale out when 1 host isn't enough (probably unlikely). Getting it off host is good so that emp run's don't get the same IAM perms that Empire has.

I think for now, we should make this feature experimental and only enable it with a flag. I'm sure for some people, the extra complexity in getting Docker over TCP securely isn't worth it.

@ejholmes
Copy link
Contributor Author

ejholmes commented Jul 2, 2016

We should also implement the Stop method in this, so that attached runs can be stopped like everything else.

@ejholmes
Copy link
Contributor Author

ejholmes commented Jul 9, 2016

This should be good to go. I went ahead and feature flagged it with an --x.showattached flag, since it requires some infrastructure changes.

containers, err := s.docker.ListContainers(docker.ListContainersOptions{
Filters: map[string][]string{
"label": []string{
fmt.Sprintf("%s", runLabel),
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be fmt.Sprintf("%s=%s", runLabel, "attached")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! I'll update.

@mwildehahn
Copy link
Contributor

👍

@ejholmes ejholmes merged commit a1b3504 into master Jul 11, 2016
@ejholmes ejholmes deleted the emp-ps-attached-runs branch July 11, 2016 20:54
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.

3 participants