Merge pull request Yelp#252 from Yelp/revert-250-revert-231-mark_for_…

…deployment-tags-take2

Revert "Revert "Mark for deployment tags take2""

general_itests/deployments_json.feature

@@ -8,5 +8,11 @@ Feature: Per-Service Deployments.json can be written and read back
 
     Scenario: New services have a desired_state of "start"
 	Given a test git repo is setup with commits
-	 When we generate deployments.json for that service
+	 When paasta mark-for-deployments is run against the repo
+	  And we generate deployments.json for that service
 	Then that deployments.json has a desired_state of "start"
+
+    Scenario: mark-for-deployments
+	Given a test git repo is setup with commits
+	 When paasta mark-for-deployments is run against the repo
+	 Then the repository should be correctly tagged

general_itests/steps/deployments_json_steps.py

@@ -28,8 +28,11 @@
 from dulwich.repo import Repo
 
 from paasta_tools import generate_deployments_for_service
-from paasta_tools.cli.cmds.start_stop_restart import format_timestamp
+from paasta_tools.cli.cmds.mark_for_deployment import paasta_mark_for_deployment
 from paasta_tools.cli.cmds.start_stop_restart import paasta_stop
+from paasta_tools.utils import format_tag
+from paasta_tools.utils import format_timestamp
+from paasta_tools.utils import get_paasta_tag_from_deploy_group
 from paasta_tools.utils import load_deployments_json
 
 
@@ -59,6 +62,30 @@ def step_impl_given(context):
     context.expected_commit = commit.id
 
 
+@when(u'paasta mark-for-deployments is run against the repo')
+def step_paasta_mark_for_deployments_when(context):
+    fake_args = mock.MagicMock(
+        deploy_group='test_cluster.test_instance',
+        service='fake_deployments_json_service',
+        git_url=context.test_git_repo_dir,
+        commit=context.expected_commit
+    )
+    context.force_bounce_timestamp = format_timestamp(datetime.utcnow())
+    with contextlib.nested(
+        mock.patch('paasta_tools.utils.format_timestamp', autosepc=True,
+                   return_value=context.force_bounce_timestamp),
+        mock.patch('paasta_tools.cli.cmds.mark_for_deployment.validate_service_name', autospec=True,
+                   return_value=True),
+    ) as (
+        mock_format_timestamp,
+        mock_validate_service_name,
+    ):
+        try:
+            paasta_mark_for_deployment(fake_args)
+        except SystemExit:
+            pass
+
+
 @when(u'paasta stop is run against the repo')
 def step_paasta_stop_when(context):
     fake_args = mock.MagicMock(
@@ -71,7 +98,7 @@ def step_paasta_stop_when(context):
     with contextlib.nested(
         mock.patch('paasta_tools.cli.cmds.start_stop_restart.utils.get_git_url', autospec=True,
                    return_value=context.test_git_repo_dir),
-        mock.patch('paasta_tools.cli.cmds.start_stop_restart.format_timestamp', autospec=True,
+        mock.patch('paasta_tools.utils.format_timestamp', autospec=True,
                    return_value=context.force_bounce_timestamp),
     ) as (
         mock_get_git_url,
@@ -130,4 +157,18 @@ def step_impl_then_desired_state(context, expected_state):
     deployments = load_deployments_json('fake_deployments_json_service', soa_dir='fake_soa_configs')
     latest = sorted(deployments.iteritems(), key=lambda(key, value): value['force_bounce'], reverse=True)[0][1]
     desired_state = latest['desired_state']
-    assert desired_state == expected_state
+    assert desired_state == expected_state, "actual: %s\nexpected: %s" % (desired_state, expected_state)
+
+
+@then(u'the repository should be correctly tagged')
+def step_impl_then_correctly_tagged(context):
+    with contextlib.nested(
+        mock.patch('paasta_tools.utils.format_timestamp', autosepc=True,
+                   return_value=context.force_bounce_timestamp),
+    ) as (
+        mock_format_timestamp,
+    ):
+        expected_tag = get_paasta_tag_from_deploy_group(identifier='test_cluster.test_instance', desired_state='deploy')
+    expected_formatted_tag = format_tag(expected_tag)
+    assert expected_formatted_tag in context.test_git_repo.refs
+    assert context.test_git_repo.refs[expected_formatted_tag] == context.expected_commit

paasta_tools/cli/cmds/mark_for_deployment.py

@@ -20,7 +20,9 @@
 from paasta_tools import remote_git
 from paasta_tools.cli.utils import validate_service_name
 from paasta_tools.utils import _log
+from paasta_tools.utils import format_tag
 from paasta_tools.utils import get_paasta_branch_from_deploy_group
+from paasta_tools.utils import get_paasta_tag_from_deploy_group
 
 
 def add_subparser(subparsers):
@@ -39,7 +41,7 @@ def add_subparser(subparsers):
     )
     list_parser.add_argument(
         '-u', '--git-url',
-        help='Git url for service -- where magic mark-for-deployment branches are pushed',
+        help='Git url for service -- where magic mark-for-deployment branches/tags are pushed',
         required=True,
     )
     list_parser.add_argument(
@@ -66,9 +68,11 @@ def add_subparser(subparsers):
 
 def mark_for_deployment(git_url, deploy_group, service, commit):
     """Mark a docker image for deployment"""
-    remote_branch = get_paasta_branch_from_deploy_group(identifier=deploy_group)
+    remote_branch = 'refs/heads/%s' % get_paasta_branch_from_deploy_group(identifier=deploy_group)
+    tag = get_paasta_tag_from_deploy_group(identifier=deploy_group, desired_state='deploy')
+    remote_tag = format_tag(tag)
     ref_mutator = remote_git.make_force_push_mutate_refs_func(
-        target_branches=[remote_branch],
+        targets=[remote_branch, remote_tag],
         sha=commit,
     )
     try:

paasta_tools/cli/cmds/start_stop_restart.py

@@ -68,12 +68,6 @@ def add_subparser(subparsers):
         status_parser.set_defaults(command=cmd_func)
 
 
-def format_timestamp(dt=None):
-    if dt is None:
-        dt = datetime.datetime.utcnow()
-    return dt.strftime('%Y%m%dT%H%M%S')
-
-
 def format_tag(branch, force_bounce, desired_state):
     return 'refs/tags/paasta-%s-%s-%s' % (branch, force_bounce, desired_state)
 
@@ -146,7 +140,7 @@ def paasta_start_or_stop(args, desired_state):
         print "Has it been deployed there yet?"
         sys.exit(1)
 
-    force_bounce = format_timestamp(datetime.datetime.utcnow())
+    force_bounce = utils.format_timestamp(datetime.datetime.utcnow())
     issue_state_change_for_service(
         service_config=service_config,
         force_bounce=force_bounce,

paasta_tools/generate_deployments_for_service.py

@@ -181,7 +181,7 @@ def get_desired_state(branch, remote_refs, deploy_group):
     """
     # (?:paasta-){1,2} supports a previous mistake where some tags would be called
     # paasta-paasta-cluster.instance
-    tag_pattern = r'^refs/tags/(?:paasta-){1,2}%s-(?P<force_bounce>[^-]+)-(?P<state>.*)$' % branch
+    tag_pattern = r'^refs/tags/(?:paasta-){1,2}%s-(?P<force_bounce>[^-]+)-(?P<state>(start|stop))$' % branch
 
     states = []
     head_sha = remote_refs['refs/heads/paasta-%s' % deploy_group]

paasta_tools/remote_git.py

@@ -66,16 +66,16 @@ def list_remote_refs(git_url):
         raise LSRemoteException("Unable to fetch remote refs: %s" % e)
 
 
-def make_force_push_mutate_refs_func(target_branches, sha):
+def make_force_push_mutate_refs_func(targets, sha):
     """Create a 'force push' function that will inform send_pack that we want
-    to mark a certain list of target branches to point to a particular
+    to mark a certain list of target branches/tags to point to a particular
     git_sha.
 
-    :param target_branches: List of branches to point at the input sha
-    :param sha: The git sha to point the branches at
+    :param targets: List of branches/tags to point at the input sha
+    :param sha: The git sha to point the branches/tags at
     :returns: A function to do the ref manipulation that a dulwich client can use"""
     def mutate_refs(refs):
-        for branch in target_branches:
-            refs['refs/heads/%s' % branch] = sha
+        for target in targets:
+            refs[target] = sha
         return refs
     return mutate_refs

paasta_tools/utils.py

@@ -1039,6 +1039,26 @@ def get_paasta_branch(cluster, instance):
     return get_paasta_branch_from_deploy_group('%s.%s' % (cluster, instance))
 
 
+def format_timestamp(dt=None):
+    if dt is None:
+        dt = datetime.datetime.utcnow()
+    return dt.strftime('%Y%m%dT%H%M%S')
+
+
+def get_paasta_tag_from_deploy_group(identifier, desired_state):
+    timestamp = format_timestamp(datetime.datetime.utcnow())
+    return 'paasta-%s-%s-%s' % (identifier, timestamp, desired_state)
+
+
+def get_paasta_tag(cluster, instance, desired_state):
+    timestamp = format_timestamp(datetime.datetime.utcnow())
+    return 'paasta-%s.%s-%s-%s' % (cluster, instance, timestamp, desired_state)
+
+
+def format_tag(tag):
+    return 'refs/tags/%s' % tag
+
+
 class NoDockerImageError(Exception):
     pass
 

tests/test_generate_deployments_for_service.py

@@ -154,3 +154,40 @@ def test_get_deployments_dict():
     assert generate_deployments_for_service.get_deployments_dict_from_deploy_group_mappings(branch_mappings) == {
         'v1': branch_mappings,
     }
+
+
+def test_get_desired_state_understands_tags():
+    remote_refs = {
+        'refs/heads/master': '7894E99E6805E9DC8C1D8EB26229E3E2243878C9',
+        'refs/remotes/origin/HEAD': 'EE8796C4E4295B7D4087E3EB73662B99218DAD94',
+        'refs/remotes/origin/master': '5F7C10B320A4EDBC4773C5FEFB1CD7B7A84FCB69',
+        'refs/tags/paasta-paasta-cluster.instance-20150721T183905-start': '4EF01B5A574B519AB546309E89F72972A33B6B75',
+        'refs/tags/paasta-paasta-cluster.instance-20151106T233211-stop': 'A5AB2A012DC238D4F6DD269C40A4BD3A99D52B1F',
+        'refs/tags/paasta-cluster.instance-20160202T233805-start': 'BE68473F98F619F26FD7824B8F56F9A7ABAEB860',
+        'refs/tags/paasta-cluster2.someinstance-20160202T233805-start': 'D6B9A0F86DC54A132FBB7747460F53F48C9AEEAD',
+        'refs/tags/paasta-cluster2.someinstance-20160205T182601-stop': '9085FD67ED1BB5FADAFA7F2AFAF8DEDEE7342711',
+        'refs/heads/paasta-cluster.instance': '4EF01B5A574B519AB546309E89F72972A33B6B75',
+        'refs/heads/paasta-cluster2.someinstance': '9085FD67ED1BB5FADAFA7F2AFAF8DEDEE7342711',
+    }
+    branch = 'cluster2.someinstance'
+    deploy_group = branch
+    expected_desired_state = ('stop', '20160205T182601')
+    actual = generate_deployments_for_service.get_desired_state(branch, remote_refs, deploy_group)
+
+    assert actual == expected_desired_state
+
+
+def test_get_desired_state_fails_gracefully_with_start():
+    remote_refs = {
+        'refs/heads/master': '7894E99E6805E9DC8C1D8EB26229E3E2243878C9',
+        'refs/remotes/origin/HEAD': 'EE8796C4E4295B7D4087E3EB73662B99218DAD94',
+        'refs/remotes/origin/master': '5F7C10B320A4EDBC4773C5FEFB1CD7B7A84FCB69',
+        'refs/heads/paasta-cluster.instance': '4EF01B5A574B519AB546309E89F72972A33B6B75',
+        'refs/heads/paasta-cluster2.someinstance': '9085FD67ED1BB5FADAFA7F2AFAF8DEDEE7342711',
+    }
+    branch = 'cluster.instance'
+    deploy_group = branch
+    expected_desired_state = ('start', None)
+    actual = generate_deployments_for_service.get_desired_state(branch, remote_refs, deploy_group)
+
+    assert actual == expected_desired_state

tests/test_remote_git.py

@@ -45,25 +45,25 @@ def test_make_determine_wants_func():
 
 
 def test_make_force_push_mutate_refs_func_overwrites_shas():
-    target_branches = ['targeta', 'targetb']
+    targets = ['refs/heads/targeta', 'refs/tags/targetb']
     newsha = 'newsha'
     input_refs = {
         'refs/heads/foo': '12345',
         'refs/heads/targeta': '12345',
-        'refs/heads/targetb': '12345',
+        'refs/tags/targetb': '12345',
         'refs/heads/ignored': '12345',
         'refs/tags/blah': '12345',
     }
     expected = {
         'refs/heads/foo': '12345',
         'refs/heads/targeta': newsha,
-        'refs/heads/targetb': newsha,
+        'refs/tags/targetb': newsha,
         'refs/heads/ignored': '12345',
         'refs/tags/blah': '12345',
     }
 
     mutate_refs_func = remote_git.make_force_push_mutate_refs_func(
-        target_branches=target_branches,
+        targets=targets,
         sha=newsha,
     )
     actual = mutate_refs_func(input_refs)