Fix race condition in default zone creation

Change-Id: I241b83f748b36aad645d0296acb73d9b654ca60a
Closes-Bug: #1905985

hooks/hooks.py

@@ -324,7 +324,8 @@ def mon_relation(rid=None, unit=None):
 
             if multisite_deployment():
                 process_multisite_relations()
-            elif ready_for_service(legacy=legacy) and is_leader():
+            elif (ready_for_service(legacy=legacy) and is_leader() and
+                  'mon' in CONFIGS.complete_contexts()):
                 # In a non multi-site deployment create the
                 # zone using the default zonegroup and restart the service
                 internal_url = '{}:{}'.format(
@@ -334,19 +335,32 @@ def mon_relation(rid=None, unit=None):
                 endpoints = [internal_url]
                 zonegroup = 'default'
                 zone = config('zone')
-                if zone == 'default':
-                    # If the requested zone is 'default' then the charm can
-                    # race with radosgw systemd process in creating it. So,
-                    # retry the zone list if it returns an empty list.
-                    existing_zones = multisite.list_zones(retry_on_empty=True)
-                else:
-                    existing_zones = multisite.list_zones()
+                existing_zones = multisite.list_zones()
                 log('Existing zones {}'.format(existing_zones), level=DEBUG)
                 if zone not in existing_zones:
-                    multisite.create_zone(zone,
-                                          endpoints=endpoints,
-                                          default=True, master=True,
-                                          zonegroup=zonegroup)
+                    log("Zone '{}' doesn't exist, creating".format(zone))
+                    try:
+                        multisite.create_zone(zone,
+                                              endpoints=endpoints,
+                                              default=True, master=True,
+                                              zonegroup=zonegroup)
+                    except subprocess.CalledProcessError as e:
+                        if 'File exists' in e.stderr.decode('UTF-8'):
+                            # NOTE(lourot): may have been created in the
+                            # background by the Rados Gateway daemon, see
+                            # lp:1856106
+                            log("Zone '{}' existed already after all".format(
+                                zone))
+                        else:
+                            raise
+
+                    existing_zones = multisite.list_zones(retry_on_empty=True)
+                    log('Existing zones {}'.format(existing_zones),
+                        level=DEBUG)
+                    if zone not in existing_zones:
+                        raise RuntimeError("Could not create zone '{}'".format(
+                            zone))
+
                     service_restart(service_name())
         else:
             send_request_if_needed(rq, relation='mon')

hooks/multisite.py

@@ -30,7 +30,7 @@ RGW_ADMIN = 'radosgw-admin'
 def _check_output(cmd):
     """Logging wrapper for subprocess.check_ouput"""
     hookenv.log("Executing: {}".format(' '.join(cmd)), level=hookenv.DEBUG)
-    return subprocess.check_output(cmd).decode('UTF-8')
+    return subprocess.check_output(cmd, stderr=subprocess.PIPE).decode('UTF-8')
 
 
 @decorators.retry_on_exception(num_retries=5, base_delay=3,

unit_tests/test_hooks.py

@@ -199,7 +199,10 @@ class CephRadosGWTests(CharmTestCase):
         _ceph.import_radosgw_key.return_value = True
         is_leader.return_value = True
         self.relation_get.return_value = 'seckey'
-        self.multisite.list_zones.return_value = []
+        self.multisite.list_zones.side_effect = [
+            [],           # at first the default zone doesn't exist, then...
+            ['default'],  # ... it got created
+        ]
         self.socket.gethostname.return_value = 'testinghostname'
         ceph_hooks.mon_relation()
         self.relation_set.assert_not_called()
@@ -219,6 +222,10 @@ class CephRadosGWTests(CharmTestCase):
         _ceph.import_radosgw_key.return_value = True
         is_leader.return_value = True
         self.relation_get.return_value = 'seckey'
+        self.multisite.list_zones.side_effect = [
+            [],           # at first the default zone doesn't exist, then...
+            ['default'],  # ... it got created
+        ]
         self.socket.gethostname.return_value = 'testinghostname'
         self.request_per_unit_key.return_value = True
         ceph_hooks.mon_relation()
@@ -242,6 +249,10 @@ class CephRadosGWTests(CharmTestCase):
         _ceph.import_radosgw_key.return_value = False
         self.relation_get.return_value = None
         is_leader.return_value = True
+        self.multisite.list_zones.side_effect = [
+            [],           # at first the default zone doesn't exist, then...
+            ['default'],  # ... it got created
+        ]
         ceph_hooks.mon_relation()
         self.assertFalse(_ceph.import_radosgw_key.called)
         self.service_resume.assert_not_called()

unit_tests/test_multisite.py

@@ -62,7 +62,7 @@ class TestMultisiteHelpers(CharmTestCase):
                 'radosgw-admin', '--id=rgw.testhost',
                 'realm', 'create',
                 '--rgw-realm=beedata', '--default'
-            ])
+            ], stderr=mock.ANY)
 
     def test_list_realms(self):
         with open(self._testdata(whoami()), 'rb') as f:
@@ -97,7 +97,7 @@ class TestMultisiteHelpers(CharmTestCase):
                 '--rgw-realm=beedata',
                 '--default',
                 '--master'
-            ])
+            ], stderr=mock.ANY)
 
     def test_list_zonegroups(self):
         with open(self._testdata(whoami()), 'rb') as f:
@@ -128,7 +128,7 @@ class TestMultisiteHelpers(CharmTestCase):
                 '--access-key=mykey',
                 '--secret=mypassword',
                 '--read-only=0',
-            ])
+            ], stderr=mock.ANY)
 
     def test_modify_zone(self):
         multisite.modify_zone(
@@ -145,7 +145,7 @@ class TestMultisiteHelpers(CharmTestCase):
             '--endpoints=http://localhost:80,https://localhost:443',
             '--access-key=mykey', '--secret=secret',
             '--read-only=1',
-        ])
+        ], stderr=mock.ANY)
 
     def test_modify_zone_promote_master(self):
         multisite.modify_zone(
@@ -160,7 +160,7 @@ class TestMultisiteHelpers(CharmTestCase):
             '--master',
             '--default',
             '--read-only=0',
-        ])
+        ], stderr=mock.ANY)
 
     def test_modify_zone_partial_credentials(self):
         multisite.modify_zone(
@@ -174,7 +174,7 @@ class TestMultisiteHelpers(CharmTestCase):
             '--rgw-zone=brundall-east',
             '--endpoints=http://localhost:80,https://localhost:443',
             '--read-only=0',
-        ])
+        ], stderr=mock.ANY)
 
     def test_list_zones(self):
         with open(self._testdata(whoami()), 'rb') as f:
@@ -234,7 +234,7 @@ class TestMultisiteHelpers(CharmTestCase):
             'realm', 'pull',
             '--url=http://master:80',
             '--access-key=testkey', '--secret=testsecret',
-        ])
+        ], stderr=mock.ANY)
 
     def test_pull_period(self):
         multisite.pull_period(url='http://master:80',
@@ -245,4 +245,4 @@ class TestMultisiteHelpers(CharmTestCase):
             'period', 'pull',
             '--url=http://master:80',
             '--access-key=testkey', '--secret=testsecret',
-        ])
+        ], stderr=mock.ANY)