Make load_service_namespace_config only look at smartstack.yaml, inst…

…ead of calling read_service_configuration which loads 7 files, and then ignoring most of it.

paasta_itests/steps/bounces_steps.py

@@ -227,7 +227,11 @@ def when_setup_service_initiated(context):
         "paasta_tools.mesos_maintenance.get_principal", autospec=True
     ) as mock_get_principal, mock.patch(
         "paasta_tools.mesos_maintenance.get_secret", autospec=True
-    ) as mock_get_secret:
+    ) as mock_get_secret, mock.patch(
+        "paasta_tools.mesos_maintenance.get_mesos_leader",
+        autospec=True,
+        return_value="mesosmaster",
+    ):
         credentials = mesos_maintenance.load_credentials(
             mesos_secrets="/etc/mesos-slave-secret"
         )

paasta_tools/long_running_service_tools.py

@@ -373,10 +373,10 @@ def load_service_namespace_config(
     :returns: A dict of the above keys, if they were defined
     """
 
-    service_config = service_configuration_lib.read_service_configuration(
-        service_name=service, soa_dir=soa_dir
+    smartstack_config = service_configuration_lib.read_extra_service_information(
+        service_name=service, extra_info="smartstack", soa_dir=soa_dir, deepcopy=False,
     )
-    smartstack_config = service_config.get("smartstack", {})
+
     namespace_config_from_file = smartstack_config.get(namespace, {})
 
     service_namespace_config = ServiceNamespaceConfig()

paasta_tools/marathon_tools.py

@@ -406,7 +406,6 @@ def load_marathon_service_config(
     cluster: str,
     load_deployments: bool = True,
     soa_dir: str = DEFAULT_SOA_DIR,
-    # system_paasta_config: Optional[SystemPaastaConfig] = None
 ) -> "MarathonServiceConfig":
     """Read a service instance's configuration for marathon.
 

tests/cli/test_cmds_info.py

@@ -29,6 +29,8 @@ def test_get_service_info():
     ) as mock_read_service_configuration, mock.patch(
         "service_configuration_lib.read_service_configuration", autospec=True
     ) as mock_scl_read_service_configuration, mock.patch(
+        "service_configuration_lib.read_extra_service_information", autospec=True
+    ) as mock_read_extra_service_information, mock.patch(
         "paasta_tools.cli.cmds.info.get_actual_deployments", autospec=True
     ) as mock_get_actual_deployments, mock.patch(
         "paasta_tools.cli.cmds.info.get_smartstack_endpoints", autospec=True
@@ -40,11 +42,13 @@ def test_get_service_info():
             "external_link": "http://bla",
             "smartstack": {"main": {"proxy_port": 9001}},
         }
-        mock_scl_read_service_configuration.return_value = {
-            "description": "a fake service that does stuff",
-            "external_link": "http://bla",
-            "smartstack": {"main": {"proxy_port": 9001}},
-        }
+        mock_scl_read_service_configuration.return_value = (
+            mock_read_service_configuration.return_value
+        )
+        mock_read_extra_service_information.return_value = mock_read_service_configuration.return_value[
+            "smartstack"
+        ]
+
         mock_get_actual_deployments.return_value = ["clusterA.main", "clusterB.main"]
         mock_get_smartstack_endpoints.return_value = [
             "http://foo:1234",
@@ -121,12 +125,13 @@ def test_get_deployments_strings_default_case_with_smartstack():
     with mock.patch(
         "paasta_tools.cli.cmds.info.get_actual_deployments", autospec=True
     ) as mock_get_actual_deployments, mock.patch(
-        "service_configuration_lib.read_service_configuration", autospec=True
-    ) as mock_read_service_configuration:
+        "service_configuration_lib.read_extra_service_information", autospec=True
+    ) as mock_read_extra_service_information:
         mock_get_actual_deployments.return_value = ["clusterA.main", "clusterB.main"]
-        mock_read_service_configuration.return_value = {
-            "smartstack": {"main": {"proxy_port": 9001}}
+        mock_read_extra_service_information.return_value = {
+            "main": {"proxy_port": 9001}
         }
+
         actual = info.get_deployments_strings("fake_service", "/fake/soa/dir")
         assert (
             " - clusterA (%s)"

tests/test_marathon_tools.py

@@ -298,7 +298,6 @@ def test_read_service_namespace_config_exists(self):
             "extra_advertise": {"alpha": ["beta"], "gamma": ["delta", "epsilon"]},
             "extra_healthcheck_headers": {"Host": "example.com"},
         }
-        fake_config = {"smartstack": {namespace: fake_info}}
         expected = {
             "healthcheck_mode": fake_healthcheck_mode,
             "healthcheck_uri": fake_uri,
@@ -326,14 +325,16 @@ def test_read_service_namespace_config_exists(self):
             "extra_healthcheck_headers": {"Host": "example.com"},
         }
         with mock.patch(
-            "service_configuration_lib.read_service_configuration",
+            "service_configuration_lib.read_extra_service_information",
             autospec=True,
-            return_value=fake_config,
-        ) as read_service_configuration_patch:
+            return_value={namespace: fake_info},
+        ) as read_extra_service_information_patch:
             actual = marathon_tools.load_service_namespace_config(
                 name, namespace, soa_dir
             )
-            read_service_configuration_patch.assert_called_once_with(name, soa_dir)
+            read_extra_service_information_patch.assert_called_once_with(
+                name, extra_info="smartstack", soa_dir=soa_dir, deepcopy=False
+            )
             assert sorted(actual) == sorted(expected)
 
     def test_read_service_namespace_config_no_mode_with_no_smartstack(self):
@@ -342,30 +343,34 @@ def test_read_service_namespace_config_no_mode_with_no_smartstack(self):
         soa_dir = "rid_aos"
         fake_config: Dict = {}
         with mock.patch(
-            "service_configuration_lib.read_service_configuration",
+            "service_configuration_lib.read_extra_service_information",
             autospec=True,
             return_value=fake_config,
-        ) as read_service_configuration_patch:
+        ) as read_extra_service_information_patch:
             actual = marathon_tools.load_service_namespace_config(
                 name, namespace, soa_dir
             )
-            read_service_configuration_patch.assert_called_once_with(name, soa_dir)
+            read_extra_service_information_patch.assert_called_once_with(
+                name, extra_info="smartstack", soa_dir=soa_dir, deepcopy=False
+            )
             assert actual.get("mode") is None
 
     def test_read_service_namespace_config_no_mode_with_smartstack(self):
         name = "eman"
         namespace = "ecapseman"
         soa_dir = "rid_aos"
-        fake_config = {"smartstack": {namespace: {"proxy_port": 9001}}}
+        fake_smartstack_config = {namespace: {"proxy_port": 9001}}
         with mock.patch(
-            "service_configuration_lib.read_service_configuration",
+            "service_configuration_lib.read_extra_service_information",
             autospec=True,
-            return_value=fake_config,
-        ) as read_service_configuration_patch:
+            return_value=fake_smartstack_config,
+        ) as read_extra_service_information_patch:
             actual = marathon_tools.load_service_namespace_config(
                 name, namespace, soa_dir
             )
-            read_service_configuration_patch.assert_called_once_with(name, soa_dir)
+            read_extra_service_information_patch.assert_called_once_with(
+                name, extra_info="smartstack", soa_dir=soa_dir, deepcopy=False
+            )
             assert actual.get("mode") == "http"
 
     def test_read_service_namespace_config_no_file(self):
@@ -374,13 +379,15 @@ def test_read_service_namespace_config_no_file(self):
         soa_dir = "an_adventure"
 
         with mock.patch(
-            "service_configuration_lib.read_service_configuration",
+            "service_configuration_lib.read_extra_service_information",
             side_effect=Exception,
             autospec=True,
-        ) as read_service_configuration_patch:
+        ) as read_extra_service_information_patch:
             with raises(Exception):
                 marathon_tools.load_service_namespace_config(name, namespace, soa_dir)
-            read_service_configuration_patch.assert_called_once_with(name, soa_dir)
+            read_extra_service_information_patch.assert_called_once_with(
+                name, extra_info="smartstack", soa_dir=soa_dir, deepcopy=False
+            )
 
     @mock.patch("paasta_tools.mesos_tools.get_local_slave_state", autospec=True)
     def test_marathon_services_running_here(self, mock_get_local_slave_state):

tests/test_mesos_maintenance.py

@@ -627,16 +627,23 @@ def test_schedule(mock_get_maintenance_schedule,):
     assert mock_get_maintenance_schedule.call_count == 1
 
 
+@mock.patch("paasta_tools.mesos_maintenance.get_mesos_config_path", autospec=True)
 @mock.patch("paasta_tools.mesos_maintenance.get_maintenance_status", autospec=True)
-def test_get_hosts_with_state_none(mock_get_maintenance_status,):
+def test_get_hosts_with_state_none(
+    mock_get_maintenance_status, mock_get_mesos_config_path
+):
+    mock_get_mesos_config_path.return_value = "/dev/null"
     fake_status = {"get_maintenance_status": {"status": {}}}
     mock_get_maintenance_status.return_value = mock.Mock()
     mock_get_maintenance_status.return_value.json.return_value = fake_status
     assert get_hosts_with_state(state="fake_state") == []
 
 
+@mock.patch("paasta_tools.mesos_maintenance.get_mesos_config_path", autospec=True)
 @mock.patch("paasta_tools.mesos_maintenance.get_maintenance_status", autospec=True)
-def test_get_hosts_with_state_draining(mock_get_maintenance_status,):
+def test_get_hosts_with_state_draining(
+    mock_get_maintenance_status, mock_get_mesos_config_path
+):
     fake_status = {
         "type": "GET_MAINTENANCE_STATUS",
         "get_maintenance_status": {
@@ -656,8 +663,11 @@ def test_get_hosts_with_state_draining(mock_get_maintenance_status,):
     assert sorted(get_hosts_with_state(state="draining_machines")) == expected
 
 
+@mock.patch("paasta_tools.mesos_maintenance.get_mesos_config_path", autospec=True)
 @mock.patch("paasta_tools.mesos_maintenance.get_maintenance_status", autospec=True)
-def test_get_hosts_with_state_down(mock_get_maintenance_status,):
+def test_get_hosts_with_state_down(
+    mock_get_maintenance_status, mock_get_mesos_config_path
+):
     fake_status = {
         "type": "GET_MAINTENANCE_STATUS",
         "get_maintenance_status": {

tests/test_setup_marathon_job.py

@@ -2034,6 +2034,7 @@ def test_deploy_marathon_service(self):
             side_effect=bounce_lib.LockHeldException,
         ):
             mock_clients: marathon_tools.MarathonClients = mock.Mock()
+            mock_system_paasta_config = mock.Mock()
             mock_marathon_apps_with_clients: List[
                 Tuple[MarathonApp, MarathonClient]
             ] = []
@@ -2043,6 +2044,7 @@ def test_deploy_marathon_service(self):
                 mock_clients,
                 "fake_soa",
                 mock_marathon_apps_with_clients,
+                mock_system_paasta_config,
             )
             assert not mock_setup_service.called
             assert ret == (0, None)