From b1d0feeba4e16046c7840bd4966f31a0e7527e67 Mon Sep 17 00:00:00 2001
From: Cyril Roelandt <cyril@redhat.com>
Date: Wed, 3 Aug 2022 18:57:05 +0200
Subject: Rbd: Deprecate unused rados_connect_timeout

This option determines the value we pass as the "timeout" argument to
rados.Rados.connect. Unfortunately, this argument is silently ignored by
librados[1].

[1] https://docs.ceph.com/en/latest/rados/api/python/#rados.Rados.connect

Closes-Bug: #1983499
Change-Id: I819df987f690c7a8c0a1153ad0e30bd5dbeb7b57
---
 glance_store/_drivers/rbd.py                                | 13 ++++++++-----
 glance_store/tests/unit/test_multistore_rbd.py              |  8 --------
 glance_store/tests/unit/test_rbd_store.py                   |  8 --------
 .../deprecate-rados_connect_timeout-767ed1eaa026196e.yaml   |  7 +++++++
 4 files changed, 15 insertions(+), 21 deletions(-)
 create mode 100644 releasenotes/notes/deprecate-rados_connect_timeout-767ed1eaa026196e.yaml

diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py
index b632854..a9409e9 100644
--- a/glance_store/_drivers/rbd.py
+++ b/glance_store/_drivers/rbd.py
@@ -133,6 +133,13 @@ Related options:
 
 """),
     cfg.IntOpt('rados_connect_timeout', default=0,
+               deprecated_for_removal=True,
+               deprecated_since='Zed',
+               deprecated_reason="""
+This option has not had any effect in years. Users willing to set a timeout for
+connecting to the Ceph cluster should use 'client_mount_timeout' in Ceph's
+configuration file.
+""",
                help="""
 Timeout value for connecting to Ceph cluster.
 
@@ -288,7 +295,7 @@ class Store(driver.Store):
         client = rados.Rados(conffile=conffile, rados_id=rados_id)
 
         try:
-            client.connect(timeout=self.connect_timeout)
+            client.connect()
         except (rados.Error, rados.ObjectNotFound) as e:
             if self.backend_group and len(self.conf.enabled_backends) > 1:
                 reason = _("Error in store configuration: %s") % e
@@ -319,8 +326,6 @@ class Store(driver.Store):
                 user = getattr(self.conf, self.backend_group).rbd_store_user
                 conf_file = getattr(self.conf,
                                     self.backend_group).rbd_store_ceph_conf
-                connect_timeout = getattr(
-                    self.conf, self.backend_group).rados_connect_timeout
                 thin_provisioning = getattr(self.conf,
                                             self.backend_group).\
                     rbd_thin_provisioning
@@ -329,7 +334,6 @@ class Store(driver.Store):
                 pool = self.conf.glance_store.rbd_store_pool
                 user = self.conf.glance_store.rbd_store_user
                 conf_file = self.conf.glance_store.rbd_store_ceph_conf
-                connect_timeout = self.conf.glance_store.rados_connect_timeout
                 thin_provisioning = \
                     self.conf.glance_store.rbd_thin_provisioning
 
@@ -343,7 +347,6 @@ class Store(driver.Store):
             self.pool = str(pool)
             self.user = str(user)
             self.conf_file = str(conf_file)
-            self.connect_timeout = connect_timeout
         except cfg.ConfigFileValueError as e:
             reason = _("Error in store configuration: %s") % e
             LOG.error(reason)
diff --git a/glance_store/tests/unit/test_multistore_rbd.py b/glance_store/tests/unit/test_multistore_rbd.py
index 4113e6c..5b37d0a 100644
--- a/glance_store/tests/unit/test_multistore_rbd.py
+++ b/glance_store/tests/unit/test_multistore_rbd.py
@@ -450,14 +450,6 @@ class TestMultiStore(base.MultiStoreBaseTest,
         self.assertRaises(exceptions.StoreRandomGetNotSupported,
                           self.store.get, loc, chunk_size=1)
 
-    @mock.patch.object(MockRados.Rados, 'connect')
-    def test_rados_connect_timeout(self, mock_rados_connect):
-        socket_timeout = 1
-        self.config(rados_connect_timeout=socket_timeout, group="ceph1")
-        self.store.configure()
-        with self.store.get_connection('conffile', 'rados_id'):
-            mock_rados_connect.assert_called_with(timeout=socket_timeout)
-
     @mock.patch.object(MockRados.Rados, 'connect', side_effect=MockRados.Error)
     def test_rados_connect_error(self, _):
         rbd_store.rados.Error = MockRados.Error
diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py
index c1c3f05..08ade51 100644
--- a/glance_store/tests/unit/test_rbd_store.py
+++ b/glance_store/tests/unit/test_rbd_store.py
@@ -656,14 +656,6 @@ class TestStore(base.StoreBaseTest,
         self.assertRaises(exceptions.StoreRandomGetNotSupported,
                           self.store.get, loc, chunk_size=1)
 
-    @mock.patch.object(MockRados.Rados, 'connect')
-    def test_rados_connect_timeout(self, mock_rados_connect):
-        socket_timeout = 1
-        self.config(rados_connect_timeout=socket_timeout)
-        self.store.configure()
-        with self.store.get_connection('conffile', 'rados_id'):
-            mock_rados_connect.assert_called_with(timeout=socket_timeout)
-
     @mock.patch.object(MockRados.Rados, 'connect', side_effect=MockRados.Error)
     def test_rados_connect_error(self, _):
         rbd_store.rados.Error = MockRados.Error
diff --git a/releasenotes/notes/deprecate-rados_connect_timeout-767ed1eaa026196e.yaml b/releasenotes/notes/deprecate-rados_connect_timeout-767ed1eaa026196e.yaml
new file mode 100644
index 0000000..f0bdd3c
--- /dev/null
+++ b/releasenotes/notes/deprecate-rados_connect_timeout-767ed1eaa026196e.yaml
@@ -0,0 +1,7 @@
+---
+deprecations:
+  - |
+    The 'rados_connect_timeout' config option for the RBD store has been
+    deprecated and will be removed in the future. It has been silently ignored
+    for multiple releases. Users willing to set a timeout for the connection to
+    the cluster can use Ceph's 'client_mount_timeout' option.
-- 
cgit v1.2.1