Revert "Mark for deployment tags take2"

general_itests/deployments_json.feature

@@ -8,11 +8,5 @@ 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 paasta mark-for-deployments is run against the repo
-	  And we generate deployments.json for that service
+	 When 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,11 +28,8 @@
 from dulwich.repo import Repo
 
 from paasta_tools import generate_deployments_for_service
-from paasta_tools.cli.cmds.mark_for_deployment import paasta_mark_for_deployment
+from paasta_tools.cli.cmds.start_stop_restart import format_timestamp
 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
 
 
@@ -62,30 +59,6 @@ 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(
@@ -98,7 +71,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.utils.format_timestamp', autospec=True,
+        mock.patch('paasta_tools.cli.cmds.start_stop_restart.format_timestamp', autospec=True,
                    return_value=context.force_bounce_timestamp),
     ) as (
         mock_get_git_url,
@@ -157,18 +130,4 @@ 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, "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
+    assert desired_state == expected_state

paasta_tools/cli/cmds/mark_for_deployment.py

@@ -20,9 +20,7 @@
 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):
@@ -41,7 +39,7 @@ def add_subparser(subparsers):
     )
     list_parser.add_argument(
         '-u', '--git-url',
-        help='Git url for service -- where magic mark-for-deployment branches/tags are pushed',
+        help='Git url for service -- where magic mark-for-deployment branches are pushed',
         required=True,
     )
     list_parser.add_argument(
@@ -68,11 +66,9 @@ def add_subparser(subparsers):
 
 def mark_for_deployment(git_url, deploy_group, service, commit):
     """Mark a docker image for deployment"""
-    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)
+    remote_branch = get_paasta_branch_from_deploy_group(identifier=deploy_group)
     ref_mutator = remote_git.make_force_push_mutate_refs_func(
-        targets=[remote_branch, remote_tag],
+        target_branches=[remote_branch],
         sha=commit,
     )
     try:

paasta_tools/cli/cmds/start_stop_restart.py

@@ -68,6 +68,12 @@ 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)
 
@@ -140,7 +146,7 @@ def paasta_start_or_stop(args, desired_state):
         print "Has it been deployed there yet?"
         sys.exit(1)
 
-    force_bounce = utils.format_timestamp(datetime.datetime.utcnow())
+    force_bounce = 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>(start|stop))$' % branch
+    tag_pattern = r'^refs/tags/(?:paasta-){1,2}%s-(?P<force_bounce>[^-]+)-(?P<state>.*)$' % 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(targets, sha):
+def make_force_push_mutate_refs_func(target_branches, sha):
     """Create a 'force push' function that will inform send_pack that we want
-    to mark a certain list of target branches/tags to point to a particular
+    to mark a certain list of target branches to point to a particular
     git_sha.
 
-    :param targets: List of branches/tags to point at the input sha
-    :param sha: The git sha to point the branches/tags at
+    :param target_branches: List of branches to point at the input sha
+    :param sha: The git sha to point the branches at
     :returns: A function to do the ref manipulation that a dulwich client can use"""
     def mutate_refs(refs):
-        for target in targets:
-            refs[target] = sha
+        for branch in target_branches:
+            refs['refs/heads/%s' % branch] = sha
         return refs
     return mutate_refs

paasta_tools/utils.py

@@ -1039,26 +1039,6 @@ 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,40 +154,3 @@ 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():
-    targets = ['refs/heads/targeta', 'refs/tags/targetb']
+    target_branches = ['targeta', 'targetb']
     newsha = 'newsha'
     input_refs = {
         'refs/heads/foo': '12345',
         'refs/heads/targeta': '12345',
-        'refs/tags/targetb': '12345',
+        'refs/heads/targetb': '12345',
         'refs/heads/ignored': '12345',
         'refs/tags/blah': '12345',
     }
     expected = {
         'refs/heads/foo': '12345',
         'refs/heads/targeta': newsha,
-        'refs/tags/targetb': newsha,
+        'refs/heads/targetb': newsha,
         'refs/heads/ignored': '12345',
         'refs/tags/blah': '12345',
     }
 
     mutate_refs_func = remote_git.make_force_push_mutate_refs_func(
-        targets=targets,
+        target_branches=target_branches,
         sha=newsha,
     )
     actual = mutate_refs_func(input_refs)