Merge pull request #281 from Mirantis/add-lock-to-save

Add lock on save to etcd operation

kqueen/auth/test_ldap.py

@@ -20,7 +20,7 @@ def teardown(self):
         self.user.delete()
 
     def test_raise_on_missing_creds(self):
-        with pytest.raises(Exception, msg='Failed to configure LDAP, please provide valid LDAP credentials'):
+        with pytest.raises(Exception, message='Failed to configure LDAP, please provide valid LDAP credentials'):
             LDAPAuth()
 
     def test_login_pass(self):
@@ -39,5 +39,5 @@ def test_login_bad_pass(self):
         assert error == 'Failed to validate full-DN. Check CN name and defined password of invited user'
 
     def test_bad_server(self):
-        with pytest.raises(ImproperlyConfigured, msg='Failed to bind connection for Kqueen Read-only user'):
+        with pytest.raises(ImproperlyConfigured, message='Failed to bind connection for Kqueen Read-only user'):
             LDAPAuth(uri='ldap://127.0.0.1:55555', admin_dn='cn=admin,dc=example,dc=org', password='heslo123')

kqueen/blueprints/api/test_cluster.py

@@ -17,6 +17,7 @@ def setup(self, client):
         self.user = self.test_user.obj
         self.test_provisioner = ProvisionerFixture(self.test_user)
         self.provisioner = self.test_provisioner.obj
+        self.provisioner.save()
 
     def teardown(self):
         super().teardown()
@@ -160,7 +161,6 @@ def test_progress_format(self):
 
     def test_create(self):
 
-        self.provisioner.save()
         post_data = {
             'name': 'Testing cluster',
             'provisioner': 'Provisioner:{}'.format(self.provisioner.id),
@@ -181,7 +181,6 @@ def test_create(self):
         assert response.json['provisioner'] == self.provisioner.get_dict(expand=True)
 
     def test_provision_after_create(self, monkeypatch):
-        self.provisioner.save()
         self.user.save()
 
         def fake_provision(self, *args, **kwargs):
@@ -212,7 +211,6 @@ def fake_provision(self, *args, **kwargs):
         assert obj.name == 'Provisioned'
 
     def test_provision_failed(self, monkeypatch):
-        self.provisioner.save()
         self.user.save()
 
         def fake_provision(self, *args, **kwargs):

kqueen/blueprints/api/test_helpers.py

@@ -1,16 +1,25 @@
 from .helpers import get_object
 
 from kqueen.storages.exceptions import BackendError
+from kqueen.conftest import ClusterFixture
 
 import pytest
 
 
-@pytest.mark.usefixtures('cluster', 'user')
+@pytest.mark.usefixtures('user')
 class TestGetObject:
+    def setup(self):
+        self.test_cluster = ClusterFixture()
+        self.cluster = self.test_cluster.obj
+        self.cluster.save()
 
-    def test_get_objects(self, cluster, user):
-        cluster.save()
-        obj = get_object(cluster.__class__, cluster.id, user)
+    def teardown(self):
+        self.test_cluster.destroy()
+
+    def test_get_objects(self, user):
+
+        obj = get_object(self.cluster.__class__,
+                         self.cluster.id, user)
 
         assert obj.get_dict(True) == obj.get_dict(True)
 
@@ -20,6 +29,6 @@ def test_get_objects(self, cluster, user):
         None,
         {},
     ])
-    def test_get_object_malformed_user(self, cluster, bad_user):
+    def test_get_object_malformed_user(self, bad_user):
         with pytest.raises(BackendError, match='Missing namespace for class'):
-            get_object(cluster.__class__, cluster.id, bad_user)
+            get_object(self.cluster.__class__, self.cluster.id, bad_user)

kqueen/storages/etcd.py

@@ -583,6 +583,7 @@ def verify_id(self):
     def save(self, validate=True, assign_id=True):
         """Save object to database
 
+        Hold lock during saving to avoid interruption into unique fields check
 
         Attributes:
             validate (bool): Validate model before saving. Defaults to `True`.
@@ -592,24 +593,24 @@ def save(self, validate=True, assign_id=True):
             bool: `True` if model was saved without errors, `False` otherwise.
 
         """
+        with etcd.Lock(current_app.db.client, 'customer1'):
+            if assign_id:
+                self.verify_id()
 
-        if assign_id:
-            self.verify_id()
+            validation_status, validation_msg = self.validate()
+            if validate and not validation_status:
+                raise ValueError('Validation for model failed with: {}'.format(validation_msg))
 
-        validation_status, validation_msg = self.validate()
-        if validate and not validation_status:
-            raise ValueError('Validation for model failed with: {}'.format(validation_msg))
+            key = self.get_db_key()
+            logger.debug('Writing {} to {}'.format(self, key))
 
-        key = self.get_db_key()
-        logger.debug('Writing {} to {}'.format(self, key))
+            try:
+                current_app.db.client.write(key, self.serialize())
 
-        try:
-            current_app.db.client.write(key, self.serialize())
-
-            self._key = key
-            return True
-        except Exception:
-            raise
+                self._key = key
+                return True
+            except Exception:
+                raise
 
     def delete(self):
         """Delete the object"""
@@ -637,12 +638,17 @@ def validate(self):
                 return False, 'Required field {} is None'.format(field)
 
             if field_object.unique and field_object.value:
-                for k, v in self.list(self.namespace).items():
+                if self.is_namespaced():
+                    namespace = self._object_namespace
+                else:
+                    namespace = self.namespace
+
+                for k, v in self.list(namespace).items():
                     # Skip checking for uniqueness on object update
                     if getattr(v, 'id') == self.id:
                         continue
                     if getattr(v, field) == field_object.value:
-                        return False, 'Field {name} should be unique'.format(name=field)
+                        return False, 'Field "{name}" should be unique'.format(name=field)
 
             if field_object.value and not field_object.validate():
                 return False, 'Field {} validation failed'.format(field)

kqueen/storages/test_model_fields.py

@@ -16,18 +16,18 @@
 import pytest
 
 
-def create_model(required=False, global_ns=False, encrypted=False):
+def create_model(required=False, global_ns=False, encrypted=False, unique=False):
     class TestModel(Model, metaclass=ModelMeta):
         if global_ns:
             global_namespace = global_ns
 
-        id = IdField(required=required, encrypted=encrypted)
-        string = StringField(required=required, encrypted=encrypted)
-        json = JSONField(required=required, encrypted=encrypted)
-        password = PasswordField(required=required, encrypted=encrypted)
-        relation = RelationField(required=required, encrypted=encrypted)
-        datetime = DatetimeField(required=required, encrypted=encrypted)
-        boolean = BoolField(required=required, encrypted=encrypted)
+        id = IdField(required=required, encrypted=encrypted, unique=unique)
+        string = StringField(required=required, encrypted=encrypted, unique=unique)
+        json = JSONField(required=required, encrypted=encrypted, unique=unique)
+        password = PasswordField(required=required, encrypted=encrypted, unique=unique)
+        relation = RelationField(required=required, encrypted=encrypted, unique=unique)
+        datetime = DatetimeField(required=required, encrypted=encrypted, unique=unique)
+        boolean = BoolField(required=required, encrypted=encrypted, unique=unique)
 
         _required = required
         _global_ns = global_ns
@@ -123,7 +123,7 @@ def test_field_property_getters(self, attr, group):
 
 class TestSave:
     def setup(self):
-        model = create_model(required=True)
+        model = create_model(required=True, unique=True)
         self.obj = model(namespace)
 
     def test_model_invalid(self):
@@ -160,6 +160,17 @@ def test_required(self, required):
         assert validation != required
 
 
+class TestUniqueFields:
+    def test_required(self, unique=True):
+        model = create_model(unique=unique)
+        obj1 = model(namespace, **model_kwargs)
+        obj2 = model(namespace, **model_kwargs)
+
+        obj1.save()
+        with pytest.raises(ValueError, match='Validation for model failed with: Field "string" should be unique'):
+            obj2.save()
+
+
 class TestGetFieldNames:
     def test_get_field_names(self, get_object):
         field_names = get_object.__class__.get_field_names()