ts_mon: removed support for https:// endpoints

infra_libs/ts_mon/test/config_test.py

Index: infra_libs/ts_mon/test/config_test.py
diff --git a/infra_libs/ts_mon/test/config_test.py b/infra_libs/ts_mon/test/config_test.py
index fa215721d88f015c0a148b3e2dc7959c2b9a56b8..f160b4a21b2626e7f1b567ff07ef1d7860212f9c 100644
--- a/infra_libs/ts_mon/test/config_test.py
+++ b/infra_libs/ts_mon/test/config_test.py
@@ -16,6 +16,8 @@ from testing_support import auto_stub
 from infra_libs.ts_mon import config
 from infra_libs.ts_mon.common import interface
 from infra_libs.ts_mon.common import standard_metrics
+from infra_libs.ts_mon.common import monitors
+from infra_libs.ts_mon.common import targets
 from infra_libs.ts_mon.common.test import stubs
 import infra_libs
 
@@ -24,7 +26,6 @@ class GlobalsTest(auto_stub.TestCase):
 
   def setUp(self):
     super(GlobalsTest, self).setUp()
-    self.mock(interface, 'state', stubs.MockState())
     self.mock(config, 'load_machine_config', lambda x: {})
 
   def tearDown(self):
@@ -32,110 +33,89 @@ class GlobalsTest(auto_stub.TestCase):
     # because any FlushThread started by the test is stored in that mock state
     # and needs to be stopped before running any other tests.
     interface.close()
+    # This should probably live in interface.close()
+    interface.state = interface.State()
     super(GlobalsTest, self).tearDown()
 
   @mock.patch('requests.get', autospec=True)
   @mock.patch('socket.getfqdn', autospec=True)
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
-  @mock.patch('infra_libs.ts_mon.common.targets.DeviceTarget', autospec=True)
-  def test_default_monitor_args(self, fake_target, fake_monitor, fake_fqdn,
-                                fake_get):
-    singleton = mock.Mock()
-    fake_monitor.return_value = singleton
-    fake_target.return_value = singleton
+  def test_pubsub_monitor_args(self, fake_fqdn, fake_get):
     fake_fqdn.return_value = 'slave1-a1.reg.tld'
     fake_get.return_value.side_effect = requests.exceptions.ConnectionError
     p = argparse.ArgumentParser()
     config.add_argparse_options(p)
     args = p.parse_args([
         '--ts-mon-credentials', '/path/to/creds.p8.json',
-        '--ts-mon-endpoint',
-        'https://www.googleapis.com/acquisitions/v1_mon_shared/storage'])
+        '--ts-mon-endpoint', 'pubsub://invalid-project/invalid-topic'])
+
     config.process_argparse_options(args)
-    fake_monitor.assert_called_once_with(
-        '/path/to/creds.p8.json',
-        'https://www.googleapis.com/acquisitions/v1_mon_shared/storage',
-        use_instrumented_http=True)
-    self.assertIs(interface.state.global_monitor, singleton)
-    fake_target.assert_called_once_with('reg', 'default', '1', 'slave1-a1')
-    self.assertIs(interface.state.target, singleton)
+
+    self.assertIsInstance(interface.state.global_monitor,
+                          monitors.PubSubMonitor)
+
+    self.assertIsInstance(interface.state.target, targets.DeviceTarget)
+    self.assertEquals(interface.state.target.hostname, 'slave1-a1')
+    self.assertEquals(interface.state.target.region, 'reg')
     self.assertEquals(args.ts_mon_flush, 'auto')
     self.assertIsNotNone(interface.state.flush_thread)
     self.assertTrue(standard_metrics.up.get())
 
   @mock.patch('requests.get', autospec=True)
   @mock.patch('socket.getfqdn', autospec=True)
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
-  @mock.patch('infra_libs.ts_mon.common.targets.DeviceTarget', autospec=True)
-  def test_default_monitor_args_uppercase_fqdn(self, fake_target, fake_monitor,
-                                               fake_fqdn, fake_get):
-    singleton = mock.Mock()
-    fake_monitor.return_value = singleton
-    fake_target.return_value = singleton
+  def test_default_target_uppercase_fqdn(self, fake_fqdn, fake_get):
     fake_fqdn.return_value = 'SLAVE1-A1.REG.TLD'
     fake_get.return_value.side_effect = requests.exceptions.ConnectionError
     p = argparse.ArgumentParser()
     config.add_argparse_options(p)
     args = p.parse_args([
         '--ts-mon-credentials', '/path/to/creds.p8.json',
-        '--ts-mon-endpoint',
-        'https://www.googleapis.com/acquisitions/v1_mon_shared/storage'])
+        '--ts-mon-endpoint', 'https://www.googleapis.com/some/api'])

[Sergey Berezin, 2015/11/04 23:34:25]

This endpoint now is not supported; how does it pass the test?

[pgervais, 2015/11/04 23:48:35]

Because it falls back to NullMonitor instead of failing. What behavior do you
prefer, failing hard or silently continuing? It's a real question, I'm fine with
both.

[Sergey Berezin, 2015/11/04 23:55:16]

Silent failure with error log is fine; I wouldn't want to break any services
with monitoring.

My comment was mostly for the test itself; perhaps, if you intend the test to
fall back to NullMonitor, maybe set the endpoint to be obviously unsupported?
(e.g. 'unsupported://api')

     config.process_argparse_options(args)
-    fake_target.assert_called_once_with('reg', 'default', '1', 'slave1-a1')
+    self.assertIsInstance(interface.state.target, targets.DeviceTarget)
+    self.assertEquals(interface.state.target.hostname, 'slave1-a1')
+    self.assertEquals(interface.state.target.region, 'reg')
 
   @mock.patch('requests.get', autospec=True)
   @mock.patch('socket.getfqdn', autospec=True)
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
-  @mock.patch('infra_libs.ts_mon.common.targets.DeviceTarget', autospec=True)
-  def test_fallback_monitor_args(self, fake_target, fake_monitor, fake_fqdn,
-                                 fake_get):
-    singleton = mock.Mock()
-    fake_monitor.return_value = singleton
-    fake_target.return_value = singleton
-    fake_fqdn.return_value = 'foo'
+  def test_default_target_fqdn_without_domain(self, fake_fqdn, fake_get):
+    fake_fqdn.return_value = 'SLAVE1-A1'
     fake_get.return_value.side_effect = requests.exceptions.ConnectionError
     p = argparse.ArgumentParser()
     config.add_argparse_options(p)
     args = p.parse_args([
         '--ts-mon-credentials', '/path/to/creds.p8.json',
-        '--ts-mon-endpoint',
-        'https://www.googleapis.com/acquisitions/v1_mon_shared/storage'])
+        '--ts-mon-endpoint', 'https://www.googleapis.com/some/api'])
     config.process_argparse_options(args)
-    fake_monitor.assert_called_once_with(
-        '/path/to/creds.p8.json',
-        'https://www.googleapis.com/acquisitions/v1_mon_shared/storage',
-        use_instrumented_http=True)
-    self.assertIs(interface.state.global_monitor, singleton)
-    fake_target.assert_called_once_with('', 'default', '', 'foo')
-    self.assertIs(interface.state.target, singleton)
+    self.assertIsInstance(interface.state.target, targets.DeviceTarget)
+    self.assertEquals(interface.state.target.hostname, 'slave1-a1')
+    self.assertEquals(interface.state.target.region, '')
 
+  @mock.patch('requests.get', autospec=True)
   @mock.patch('socket.getfqdn', autospec=True)
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
-  @mock.patch('infra_libs.ts_mon.common.targets.DeviceTarget', autospec=True)
-  def test_manual_flush(self, fake_target, fake_monitor, fake_fqdn):
-    singleton = mock.Mock()
-    fake_monitor.return_value = singleton
-    fake_target.return_value = singleton
+  def test_fallback_monitor_args(self, fake_fqdn, fake_get):
     fake_fqdn.return_value = 'foo'
+    fake_get.return_value.side_effect = requests.exceptions.ConnectionError
     p = argparse.ArgumentParser()
     config.add_argparse_options(p)
-    args = p.parse_args(['--ts-mon-flush', 'manual'])
+    args = p.parse_args([
+        '--ts-mon-credentials', '/path/to/creds.p8.json',
+        '--ts-mon-endpoint', 'https://www.googleapis.com/some/api'])
     config.process_argparse_options(args)
-    self.assertIsNone(interface.state.flush_thread)
 
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
-  def test_monitor_args(self, fake_monitor):
-    singleton = mock.Mock()
-    fake_monitor.return_value = singleton
+    self.assertIsInstance(interface.state.global_monitor,
+                          monitors.NullMonitor)
+
+  @mock.patch('requests.get', autospec=True)
+  @mock.patch('socket.getfqdn', autospec=True)
+  def test_manual_flush(self, fake_fqdn, fake_get):
+    fake_fqdn.return_value = 'foo'
+    fake_get.return_value.side_effect = requests.exceptions.ConnectionError
     p = argparse.ArgumentParser()
     config.add_argparse_options(p)
-    args = p.parse_args(['--ts-mon-credentials', '/path/to/creds.p8.json',
-                         '--ts-mon-endpoint', 'https://foo.tld/api'])
+    args = p.parse_args(['--ts-mon-flush', 'manual'])
+
     config.process_argparse_options(args)
-    fake_monitor.assert_called_once_with(
-        '/path/to/creds.p8.json', 'https://foo.tld/api',
-        use_instrumented_http=True)
-    self.assertIs(interface.state.global_monitor, singleton)
+    self.assertIsNone(interface.state.flush_thread)
 
   @mock.patch('infra_libs.ts_mon.common.monitors.PubSubMonitor', autospec=True)
   def test_pubsub_args(self, fake_monitor):
@@ -162,9 +142,8 @@ class GlobalsTest(auto_stub.TestCase):
     fake_monitor.assert_called_once_with('foo.txt')
     self.assertIs(interface.state.global_monitor, singleton)
 
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
   @mock.patch('infra_libs.ts_mon.common.targets.DeviceTarget', autospec=True)
-  def test_device_args(self, fake_target, _fake_monitor):
+  def test_device_args(self, fake_target):
     singleton = mock.Mock()
     fake_target.return_value = singleton
     p = argparse.ArgumentParser()
@@ -179,9 +158,8 @@ class GlobalsTest(auto_stub.TestCase):
     fake_target.assert_called_once_with('reg', 'role', 'net', 'host')
     self.assertIs(interface.state.target, singleton)
 
-  @mock.patch('infra_libs.ts_mon.api_monitor.ApiMonitor', autospec=True)
   @mock.patch('infra_libs.ts_mon.common.targets.TaskTarget', autospec=True)
-  def test_task_args(self, fake_target, _fake_monitor):
+  def test_task_args(self, fake_target):
     singleton = mock.Mock()
     fake_target.return_value = singleton
     p = argparse.ArgumentParser()
@@ -194,7 +172,7 @@ class GlobalsTest(auto_stub.TestCase):
                          '--ts-mon-task-hostname', 'host',
                          '--ts-mon-task-number', '1'])
     config.process_argparse_options(args)
-    fake_target.assert_called_once_with('serv', 'job' ,'reg', 'host', 1)
+    fake_target.assert_called_once_with('serv', 'job', 'reg', 'host', 1)
     self.assertIs(interface.state.target, singleton)
 
   @mock.patch('infra_libs.ts_mon.common.monitors.NullMonitor', autospec=True)