Project tag must be defined (documentation unclear).

[scraton]

Hello!

I was interested in using this gem to simplify our Capistrano logic. But the documentation was a little unclear about its requirements. Hopefully you can word things a bit differently to help out others in the future, or alternatively make the code match the documentation to make this lib more configurable.

The line I'm talking about is this one:

project_tag

If this is defined, Cap-EC2 will look for a tag with this name when searching for instances belong to this project. Cap-EC2 will look for a value which matches the :application setting in your deploy.rb. The tag name defaults to "Project".

This would seem to indicate it can be undefined and simply not look for the project tag. But in fact it will always look for a project tag, even if this is nil:

    def get_servers_for_role(role)
    # ...
        instances = ec2.instances
          .filter(tag(project_tag), "*#{application}*")
          .filter('instance-state-name', 'running')
    # ...
          instance_has_tag?(i, roles_tag, role) &&
            instance_has_tag?(i, stages_tag, stage) &&
            instance_has_tag?(i, project_tag, application) &&
            (fetch(:ec2_filter_by_status_ok?) ? instance_status_ok?(i) : true)
    # ...
    end

Perhaps this could be improved with something like this:

    def get_servers_for_role(role)
      servers = []
      @ec2.each do |_, ec2|
        instances = ec2.instances.filter('instance-state-name', 'running')
        instances = instances.filter(tag(project_tag), "*#{application}*") unless project_tag.nil?

        servers << instances.select do |i|
          instance_has_tag?(i, roles_tag, role) &&
            instance_has_tag?(i, stages_tag, stage) &&
            (fetch(:ec2_filter_by_status_ok?) ? instance_status_ok?(i) : true)
        end
      end
      servers.flatten.sort_by {|s| s.tags["Name"] || ''}
    end

This way a dependency for having a Project tag is dropped, in cases where you have an AWS account dedicated to a particular project. The second check to instance_has_tag?(i, project_tag, stag) is also removed since it appears to be a duplicate.

Or just reword the documentation:

Cap-EC2 will look for a tag with this name when searching for instances belong to this project. Cap-EC2 will look for a value which matches the :application setting in your deploy.rb. The tag name defaults to "Project" and must be set on your instances.

Let me know what you think. :)

[andytinycat]

I don't think removing the need for a Project tag is a good idea - it's breaking behaviour that has existed since the original capify-ec2, and new users already find the instance matching behaviour hard to understand initially.

I'll reword the documentation as you suggest, since that's much clearer than my ramblings.