Support running Heron on Amazon Ecs. #1837

ananthgs · 2017-05-05T01:19:27Z

Pre requisite: AWS and ECS-CLI needs to be installed on the submitter machine as per guidelines from Amazon.

kramasamy · 2017-05-09T20:44:49Z

heron/config/src/yaml/conf/ecs/statemgr.yaml

+#heron.statemgr.connection.string: "xx.xx.xx.xx:2181"
+
+
+# path of the root address to store the state in a local file system


Is this a local file system or it expects zookeeper?

It does expect zookeeper to be running on one of the ec2 instances. As suggested I will put up a document explaining the offline set ups on Amazon ec2 and the following steps on the scheduler.

kramasamy · 2017-05-09T20:47:47Z

heron/config/src/yaml/conf/ecs/statemgr.yaml

+
+heron.statemgr.zookeeper.retry.interval.ms:               10000
+
+####################################################################


Do we require tunneling? If not, please remove the tunnel configuration.

Tunneling could be needed for any of the schedulers and they're valid configs so I think it's ok to keep them in the config file. Maybe we set it to False by default though.

Done set default to false

kramasamy · 2017-05-09T20:52:39Z

heron/executor/src/python/heron_executor.py

@@ -155,7 +171,14 @@ def init_parsed_args(self, args):
    # id within docker, rather than the host's hostname. NOTE: this 'HOST' env variable is not
    # guaranteed to be set in all Docker executor environments (outside of Marathon)
    if is_docker_environment():
-      self.master_host = os.environ.get('HOST') if 'HOST' in os.environ else socket.gethostname()
+       # Need to set the HOST environment vairable if docker is for AWS ECS tasks


Instead of adding code to this function, can we make a overall function called get_host - in which we can check whether it is running in docker or ECS or use socket.gethostname() and return the hostname.

+1 for ways to do this without one-off logic to support ECS.

added gethost function

kramasamy · 2017-05-09T20:55:27Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+  public static final String ECS_CLUSTER_BINARY = "heron-examples.jar";
+  public static final String COMPOSE_CMD = "ecs-cli compose --project-name ";
+  public static final String UP = " up";
+


There is a lot of hard coding of values here. Not sure why you need them?

+1 for removing as much of this hard-coding as possible.

kramasamy · 2017-05-09T20:58:21Z

@ananthgs - can you write a good concise PR description about how it works with ECS?

kramasamy · 2017-05-09T21:02:06Z

@ananthgs - you might want to add a detailed documentation of how to set it up and use, as described in other schedulers for heron.io. However, this can be done in a different PR.

billonahill

Thanks for the submission! Please add unit tests and accept the CLA:

https://twitter.github.io/heron/docs/contributors/community/

billonahill · 2017-05-09T21:41:36Z

heron/config/src/yaml/conf/ecs/statemgr.yaml

+
+heron.statemgr.zookeeper.retry.interval.ms:               10000
+
+####################################################################


Tunneling could be needed for any of the schedulers and they're valid configs so I think it's ok to keep them in the config file. Maybe we set it to False by default though.

billonahill · 2017-05-09T21:44:37Z

heron/executor/src/python/heron_executor.py

@@ -155,7 +171,14 @@ def init_parsed_args(self, args):
    # id within docker, rather than the host's hostname. NOTE: this 'HOST' env variable is not
    # guaranteed to be set in all Docker executor environments (outside of Marathon)
    if is_docker_environment():
-      self.master_host = os.environ.get('HOST') if 'HOST' in os.environ else socket.gethostname()
+       # Need to set the HOST environment vairable if docker is for AWS ECS tasks


+1 for ways to do this without one-off logic to support ECS.

billonahill · 2017-05-09T21:47:04Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+  public static final String ECS_CLUSTER_BINARY = "heron-examples.jar";
+  public static final String COMPOSE_CMD = "ecs-cli compose --project-name ";
+  public static final String UP = " up";
+


+1 for removing as much of this hard-coding as possible.

billonahill · 2017-05-09T21:47:18Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+      + "  herondata:\n"
+      + "    driver: local";
+  public static final String DESTINATION_JVM = "/usr/lib/jvm/java-8-oracle";
+  public static final String ECS_CLUSTER_BINARY = "heron-examples.jar";


Why does this code require a dep on heron-examples.jar?

billonahill · 2017-05-09T21:47:39Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+      + "volumes:\n"
+      + "  herondata:\n"
+      + "    driver: local";
+  public static final String DESTINATION_JVM = "/usr/lib/jvm/java-8-oracle";


Why does jvm location need to be hardcoded?

If your Docker image contains a pre-installed version of the Java 8 JRE, you should be able to reference $JAVA_HOME here. You should also be able to set that from within the scheduler configuration as well, via the heron.directory.sandbox.java.home option

billonahill · 2017-05-09T22:11:17Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+  }
+
+  public String replacePortNumbers(int container, String content) {
+    int basePortnumber = 5000;


The 5000 here and the logic in this method looks duplicative of other logic in this class. Can you consolidate?

billonahill · 2017-05-09T22:13:26Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+  }
+
+  public String getDockerFileContent(String execCommand, int container) {
+    String commandBuiler = EcsContext.PART1 + EcsContext.CMD;


Favor StringBuilder over string concat. Or better yet, use String.format(..)

I agree. Will modify

billonahill · 2017-05-09T22:14:20Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+  public String replacePortNumbers(int container, String content) {
+    int basePortnumber = 5000;
+    String localContent = new String(content);
+    for (int i = 0; i < SchedulerUtils.PORTS_REQUIRED_FOR_EXECUTOR; i++) {


please clarify in the javadocs what this method does. Also I suspect StringBuilder might be a better fit here.

billonahill · 2017-05-09T22:16:42Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+      }
+      builder.append(string);
+    }
+    String stringToReturn = builder.toString();


return builder.toString();

billonahill · 2017-05-09T22:17:43Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+    return null;
+  }
+
+  public boolean onKill(Scheduler.KillTopologyRequest request) {


These methods all need to be implemented and annotated with @OverRide.

+1, please implement those methods in the following commits

kramasamy · 2017-05-10T04:13:44Z

heron/executor/src/python/heron_executor.py

@@ -108,6 +109,21 @@ def log_pid_for_process(process_name, pid):
 def is_docker_environment():
  return os.path.isfile('/.dockerenv')

+def isEcsAmiInstance():
+  meta = 'http://169.254.169.254/latest/meta-data/ami-id'


@ananthgs - why is this URL hard coded?

please remove hard coded url addresses

pass it from out side if necessary

done, Passing it as argument

kramasamy · 2017-05-10T04:15:52Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+      + "services:\n"
+      + "  container_number:\n"
+      + "    image: ananthgs/onlyheronandubuntu\n";
+  public static final String CMD = "    command: [\"sh\", \"-c\", \"mkdir /s3; cd /s3 ;"


@ananthgs - Is this image a generic ubuntu image downloaded from docker hub? or specially created image? If this is specially created image - what does it contain other than the basic OS?

kramasamy · 2017-05-10T04:16:50Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+  public static final String ECSNETWORK = "    networks:\n"
+      + "      - heron\n"
+      + "    ports:\n"
+      + "    - \"5000:5000\"\n"


Why these ports are hard coded? Can we ask ECS to provide a dynamic port?

Done removed hard coded ports

jrcrawfo · 2017-05-11T04:28:31Z

heron/executor/src/python/heron_executor.py

+        self.master_host = subprocess.Popen(["curl",
+                                             "http://169.254.169.254/latest/meta-data/local-ipv4"]
+                                            , stdout=subprocess.PIPE).communicate()[0]
+        os.environ['HOST'] = self.master_host


No need for this line since you won't be grabbing os.environ['HOST'] after this

jrcrawfo · 2017-05-11T04:32:36Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsContext.java

+      + "    image: ananthgs/onlyheronandubuntu\n";
+  public static final String CMD = "    command: [\"sh\", \"-c\", \"mkdir /s3; cd /s3 ;"
+      + "aws s3 cp s3://herondockercal/TOPOLOGY_NAME/topology.tar.gz /s3 ;"
+      + "aws s3 cp s3://herondockercal/heron-core-testbuild-ubuntu14.04.tar.gz /s3 ;cd /s3;"


That paths to the core binaries will need to be more dynamic than this and will need to feed out of the heron.package.core.uri configuration option in the scheduler library right?

jrcrawfo · 2017-05-11T04:37:55Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+    String content = null;
+    try {
+      tempDockerFile = File.createTempFile("docker", ".yml");
+      content = getDockerFileContent(finalExecCommand, container);


Can you please explain what the process is here for creating this docker.yml file just so I understand what's going on? What is it used for and what is it composed of -- just so I can understand a little better.

The AWS ECS task can be triggered via a docker style compose command. This Task is then run on the AWS Cluster in a Container instance. The overall approach is explained here :
https://docs.google.com/document/d/1ecbCuA46cIKPfY0SP0F1dcRlei4DIPz3pZ6ZSZ5zZgc/edit?usp=sharing.
Please feel free to comment on the approach.

nlu90 · 2017-05-11T05:17:51Z

heron/executor/src/python/heron_executor.py

@@ -108,6 +109,21 @@ def log_pid_for_process(process_name, pid):
 def is_docker_environment():
  return os.path.isfile('/.dockerenv')

+def isEcsAmiInstance():
+  meta = 'http://169.254.169.254/latest/meta-data/ami-id'


please remove hard coded url addresses

pass it from out side if necessary

nlu90 · 2017-05-11T05:19:19Z

heron/executor/src/python/heron_executor.py

+       # Need to set the HOST environment vairable if docker is for AWS ECS tasks
+      if isEcsAmiInstance():
+        self.master_host = subprocess.Popen(["curl",
+                                             "http://169.254.169.254/latest/meta-data/local-ipv4"]


same hard code url problem

nlu90 · 2017-05-11T05:27:07Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+    return null;
+  }
+
+  public boolean onKill(Scheduler.KillTopologyRequest request) {


+1, please implement those methods in the following commits

nlu90 · 2017-05-11T05:27:49Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+  }
+
+  public List<String> getJobLinks() {
+    return null;


an empty ArrayList is better than a null

Added the get joblink feature to return the ecs tasks

nlu90 · 2017-05-11T05:33:46Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+  private String[] getExecutorCommand(int container) {
+    List<Integer> freePorts = new ArrayList<>(SchedulerUtils.PORTS_REQUIRED_FOR_EXECUTOR);
+    for (int i = 0; i < SchedulerUtils.PORTS_REQUIRED_FOR_EXECUTOR; i++) {
+      //freePorts.add(SysUtils.getFreePort());


any specific reason not using this getFreePort() method but hard code the port number?

nlu90 · 2017-05-11T05:43:23Z

heron/schedulers/src/java/com/twitter/heron/scheduler/ecs/EcsScheduler.java

+    LOG.info("Starting to deploy topology: " + EcsContext.topologyName(config));
+    LOG.info("Starting executor for TMaster");
+    startExecutor(0);
+      // for each container, run its own executor


log the event of starting normal container here

billonahill · 2017-05-11T16:54:58Z

@ajorgensen you guys also run on EC2, correct? If so could you review this to see if your approach and this approach could coexist in this codebase?

modified: ecs-heron-role.json

modified: ecs-heron-policy.json

…eeper.

# Conflicts: # heron/executor/src/python/heron_executor.py

ananthgs added 17 commits May 3, 2017 15:13

Added Ecs Docker Host Support

370873e

Adding the ecs scheduler

8a10b03

AWS ECS schedulers added

cee04fe

Adding AWS ECS files

c5b7f3e

adding ecs to build scripts

8c02617

adding ecs to build

725891d

Adding AMI Check dependancy

5336c3c

Adding ecs conf

e368e9a

Added Compose Commands

9a80784

Cleaned up non ecs functions

0aae022

Added temp file for compose

83bf1a1

Removed reference to temp dir

f1379f6

Removed Working Dir reference

c38322c

Removed commented lines for unused imports

7e575b9

emoved commented lines on unused imports

4c6de0c

Removing un referenced EcsKeys class

fefbbc8

Removed the host env setting if its an Amazon ECS instance

edaa5fa

kramasamy suggested changes May 9, 2017

View reviewed changes

billonahill suggested changes May 9, 2017

View reviewed changes

kramasamy suggested changes May 10, 2017

View reviewed changes

jrcrawfo reviewed May 11, 2017

View reviewed changes

nlu90 reviewed May 11, 2017

View reviewed changes

ananthgs added 5 commits May 12, 2017 00:46

remving had coded values

8efc62e

removing hard coded values and ports

0b74033

new file: EcsKey.java

fceacfa

modified: EcsLauncher.java

532fffe

fixing java and other paths

2d08fe5

ananthgs added 30 commits May 16, 2017 00:00

adde jackson JSON jars for ecs Schedulers

5204be2

new file: heron/config/src/yaml/conf/ecs/setupEcs.sh

52f40bb

new file: heron/config/src/yaml/conf/ecs/set-ecs-cluster-name.sh

e74b0ef

new file: heron/config/src/yaml/conf/ecs/ecs-heron-policy.json

0824fa0

new file: heron/config/src/yaml/conf/ecs/ecs-heron-role.json

d3a7df5

fixed echo messages

f65f865

Fixed Log messages

15a91f7

new file: EcsSchedulerTest.java

96a150d

modified: setupEcs.sh

f2e4267

fixed version number

be514cb

modified: ecs-heron-role.json

fixed version number

0ece9cb

modified: ecs-heron-policy.json

modified: set-ecs-cluster-name.sh

2ef6665

new file: README

c224c82

Clean up file for AWS

30f4348

modified: ecs_compose_template.yaml

b362b4d

modified: setupEcs.sh

4b05a87

modified: scheduler.yaml

2d9dbec

added cluster binary context

0119231

Added scheduler realated changes for scheduler info to be put in zook…

986e7f7

…eeper.

Added files for on schedule testing

f13b137

JAVA home for enable AWS scheduler

4cc598b

Updating with correct document link

168a474

Adding Ecs AMI check as non mandatory field

f19d354

modified: tools/rules/heron_core.bzl

034eb1a

modified: scripts/packages/BUILD

246b922

merging with changes on upstream

f6f52d9

Merge remote-tracking branch 'upstream/master'

3767043

# Conflicts: # heron/executor/src/python/heron_executor.py

Fixed duplicate lines due to merge with head branch

84b669c

Fixed Cluster name to ecs-heron-cluster

88d5ae6

modified: heron/config/src/yaml/conf/ecs/setupEcs.sh

2f1bde1

		#heron.statemgr.connection.string: "xx.xx.xx.xx:2181"


		# path of the root address to store the state in a local file system


		heron.statemgr.zookeeper.retry.interval.ms: 10000

		####################################################################

Support running Heron on Amazon Ecs. #1837

Are you sure you want to change the base?

Support running Heron on Amazon Ecs. #1837

Conversation

ananthgs commented May 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kramasamy commented May 9, 2017

kramasamy commented May 9, 2017

billonahill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

billonahill commented May 11, 2017