josmo / drone-ecs

Drone plugin for triggering Amazon EC2 Container Service (ECS) deployments
Apache License 2.0
30 stars 41 forks source link

Merge existing task definitions #4

Closed fridaystreet closed 8 years ago

fridaystreet commented 8 years ago

More of a feature request, apologies if not the right place to post.

We're migrating from Jenkins to drone and it's great, but in regards to publishing to ECS it would be good if could be a bit more dynamic. We use cloud formation to bring up all our services so the ECS service, definition and task names are all dynamic along with the environment variables we inject for things like database IP addresses etc. Currently we have a script in jenkins that pulls down the current service definition as a template and replaces the values for the image to be used. This way we don't need to hard code any of the environment variables into jenkins that could change through cloud formation.

Also it takes the environment (eg production) and finds the appropriate cluster / service first rather than needing to hard code those names too, which may change. eg an api cluster might be called production-api-JH67JGSJK, with a random value appended to the base name production-api through the cloud formation process and a db cluster might be production-db-HGDTY13TH.

It's just a simple bash script using the aws cli, first we pull down all the services and grep on the environment, then on the service ie db or api. This way we can then drill in to the service and find the latest task def for say production-api for a cloud formation built service

So I was wondering if this sort of functionality could be considered to be added. Rather than specifying a fixed service name or family, could it be derived from a substring. Then pull the existing definition and merge any new settings like cpu, mem, env vars, image name etc if they are specified in the yml otherwise use the existing tasks settings. So you only need to specify in the drone.yml what has changed.

I've attached an example bash script, this script is for a cluster with a base name of ServicesCluster which contains a service with a base name that contains the word 'Dashboard'. The service only has one task so it derives the current task from that, but it could easily handle finding a specific task in the service as well if you passed in a base name for the task.

Thanks in advance for any consideration and review

Cheers Paul

jenkins_build.txt

fridaystreet commented 8 years ago

Thought an example yml might help. $$BRANCH = production. So rather than a fixed name, have additional search parameters. In this case it would search for a service with production-api in the name and a task definition in that service with Dashboard in the name. Then copy the existing definition, but override just the memory settings changing it to 8GB

deploy: ecs: image: plugins/drone-ecs region: ap-southeast-2 access_key: $$AWS_KEY secret_key: $$AWS_SECRET service_search: $$BRANCH-api family_search: Dashboard image_name: myreg/frontend-$$BRANCH image_tag: "1.0.$$BUILD_NUMBER" memory: 8000

furybyname commented 8 years ago

Hi @fridaystreet

Personally I think this is a good idea. Cloud formation is being used more heavily than manually setting up stacks.

Might want to tweak the yaml to give a clear difference between using the "search" and a standard hardcoded deploy.

@bradrydzewski @tboerger I'm happy to give this a go if no objections? Might be a couple of weeks though.

tboerger commented 8 years ago

I'm open for improvements as long as we stay backwards compatible

furybyname commented 8 years ago

Good call @tboerger will make sure it is

bradrydzewski commented 8 years ago

I don't use ECS or cloud formation so I definitely look toward the community to suggest the best approach to plugins. That being said I'm not sure if this should or should not be included in this plugin. Some questions and concerns I have below.

First, is this something that will be useful to others? This description mentions your team is using a custom script. If this is going to be included in the official plugin we need to verify that this is something that will be used by more than just your team, out-of-the-box. The best way to do this is fork the plugin, incubate your feature, and gather community feedback.

Second is that plugins should be simple and targeted. In some cases it may make sense to have multiple plugins, for example we have two slack plugins (slack and slack blame) and two s3 plugins (s3 and s3-sync). We prefer simple, targeted plugins. This sounds like it should be an ecs_cloudformation plugin, but again, that is based on my limited understanding of the domain.

Third is that not everything needs to be an official plugin. If you are doing something very specific to your team (and not generally useful to others) there is nothing wrong with maintaining your own plugin fork. Plugins should be simple and forkable. The goal of our official plugins is not to do everything for everyone, but to provide a simple way to fork, build and extend upon the base plugins.

bradrydzewski commented 8 years ago

I'm going to request this be incubated as a custom plugin. Once we have feedback we can re-open the issue and decide if this should be included in the official plugin, or deployed as a second plugin, or if it should be kept as a custom 3rd party plugin.

fridaystreet commented 8 years ago

Hi Guys,

Thanks for the quick feedback and consideration, appreciate you taking the time to respond. Reading it back to myself, I guess in reality it's actually 2 features. 1. The ability to copy the settings from an existing task definition so they don't all need to be specified in the drone.yml file. 2. The wildcard selection of the services and definitions.

@bradrydzewski @tboerger do you mean for us to fork and incubate it internally and try and create our own plugin first or is it something you guys are able to assist with, just not as a priority?

tboerger commented 8 years ago

I'm not using this plug-in, so I don't know if everything makes sense for more people than your team.

The official plugins must be maintainable, and backward compatible, that is the most important.

Maybe you should fork, build it matching your flow and write a proposal how it improves the official plugin if you can keep it general enough.

bradrydzewski commented 8 years ago

@bradrydzewski @tboerger do you mean for us to fork and incubate it internally and try and create our own plugin first or is it something you guys are able to assist with, just not as a priority?

Yes, let's start with a fork and then find a way to get community input. This will help us decide if it should be a separate plugin, or if this functionality should be rolled into this ecs plugin, or if there are any improvements that need to be made before publishing.

The goal here, as @tboerger mentioned, is trying to keep backward compatibility. If we merge a feature or functionality now, we can never remove or change it (ok, maybe not never, but we try not to). As a result, we like to incubate stuff and flush out the requirements before we make changes.

Also, creating two separate plugins is not unusual -- we have an s3 and s3-sync. We decided to keep these separate initially. We will soon be combining s3 and s3-sync based on community feedback. We could have combined them from the start, but it was nice to keep them separate to allow them to evolve independently, even if we ultimately end up combining them :)