[Kimchi-devel] [PATCH] Fix issue #738 - Split Wok and Kimchi object stores

Lucio Correia luciojhc at linux.vnet.ibm.com
Thu Oct 15 12:54:26 UTC 2015


On 14-10-2015 18:21, Aline Manera wrote:
> I was not able to apply this patch. Could you rebase on master and
> resend, please?
>
> More comments below:
>
> On 13/10/2015 18:54, Lucio Correia wrote:
>> Wok and Kimchi currently share the same object store. This
>> patch creates a specific store for Kimchi to avoid name
>> conflicts with Wok and other plugins in future.
>>
>> It also makes objectstore location a required parameter for
>> model instantiation, since there is no more default location
>> provided by Wok.
>
> So we should provide a default option to wok.
> By instance, wok does not use the objstore, it only provides API to deal
> with that.
>
>> Each plugin will have its own object store
>> in its own location
Agreed, will try to remove the objstore from wok.


>>
>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>> ---
>>   Makefile.am                                          | 5 ++++-
>>   contrib/wok.spec.fedora.in                           | 3 ++-
>>   contrib/wok.spec.suse.in                             | 3 ++-
>>   src/wok/model/model.py                               | 2 +-
>>   src/wok/objectstore.py                               | 5 ++---
>>   src/wok/plugins/kimchi/Makefile.am                   | 4 ++--
>>   src/wok/plugins/kimchi/config.py.in                  | 4 ++++
>>   src/wok/plugins/kimchi/contrib/kimchi.spec.fedora.in | 2 +-
>>   src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in   | 2 +-
>>   src/wok/plugins/kimchi/mockmodel.py                  | 2 +-
>>   src/wok/plugins/kimchi/root.py                       | 7 +++++--
>>   src/wok/plugins/kimchi/tests/test_yumparser.py       | 3 ++-
>>   src/wok/server.py                                    | 2 +-
>>   13 files changed, 28 insertions(+), 16 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 8d2f346..54bc521 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -83,6 +83,8 @@ install-deb: install
>>           cp -R $(top_srcdir)/contrib/wokd-upstart.conf.debian \
>>               $(DESTDIR)/etc/init/wokd.conf; \
>>       fi
>> +    $(MKDIR_P) -p $(DESTDIR)/$(localstatedir)/lib/wok/
>> +    touch $(DESTDIR)/$(localstatedir)/lib/wok/objectstore
>>       $(MKDIR_P) $(DESTDIR)/usr/lib/firewalld/services
>>       cp -R $(top_srcdir)/src/firewalld.xml \
>>           $(DESTDIR)/usr/lib/firewalld/services/wokd.xml
>> @@ -128,12 +130,13 @@ install-data-local:
>>           mkdir -p $(DESTDIR)/etc/init.d/; \
>>           $(INSTALL_DATA) contrib/wokd.sysvinit
>> $(DESTDIR)/etc/init.d/wokd; \
>>           chmod +x $(DESTDIR)/etc/init.d/wokd; \
>> -    fi; \
>> +    fi; \
>>       if test -d /usr/lib/firewalld/services/; then \
>>           mkdir -p $(DESTDIR)/usr/lib/firewalld/services/; \
>>           $(INSTALL_DATA) src/firewalld.xml
>> $(DESTDIR)/usr/lib/firewalld/services/wokd.xml; \
>>       fi; \
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/wok/
>> +    touch $(DESTDIR)/$(localstatedir)/lib/wok/objectstore
>>       mkdir -p $(DESTDIR)/$(localstatedir)/log/wok/
>>       touch $(DESTDIR)/$(localstatedir)/log/wok/wok-access.log
>>       touch $(DESTDIR)/$(localstatedir)/log/wok/wok-error.log
>> diff --git a/contrib/wok.spec.fedora.in b/contrib/wok.spec.fedora.in
>
> We don't need to create the objectstore file prior to it is needed.
> We firstly did it to remove the objectstore file while uninstalling the
> package but thinking more about it we don't need to do that.
> As user may use the DB later without losing data.

Agreed. I will remove them from kimchi Makefile as well because I just 
mimic it.

>
>
>> index 8941ff7..ad9988e 100644
>> --- a/contrib/wok.spec.fedora.in
>> +++ b/contrib/wok.spec.fedora.in
>> @@ -122,6 +122,7 @@ rm -rf $RPM_BUILD_ROOT
>>   %if 0%{?with_systemd}
>>   %{_sysconfdir}/nginx/conf.d/wok.conf
>>   %{_sharedstatedir}/wok/
>> +%{_sharedstatedir}/wok/objectstore
>>   %{_localstatedir}/log/wok/*
>>   %{_localstatedir}/log/wok/
>>   %{_unitdir}/wokd.service
>> @@ -135,7 +136,7 @@ rm -rf $RPM_BUILD_ROOT
>>   %endif
>>
>>   %changelog
>> -* Fri Jun 19 2015 Lucio Correia <luciojhc at linux.vnet.ibm.com> 1.6
>> +* Fri Jun 19 2015 Lucio Correia <luciojhc at linux.vnet.ibm.com> 2.0
>>   - Rename to wokd
>>   - Remove kimchi specifics
>>
>> diff --git a/contrib/wok.spec.suse.in b/contrib/wok.spec.suse.in
>> index 161b996..4b5e4f3 100644
>> --- a/contrib/wok.spec.suse.in
>> +++ b/contrib/wok.spec.suse.in
>> @@ -95,6 +95,7 @@ rm -rf $RPM_BUILD_ROOT
>>   %{_sysconfdir}/nginx/conf.d/wok.conf.in
>>   %{_sysconfdir}/nginx/conf.d/wok.conf
>>   %{_var}/lib/wok/
>> +%{_var}/lib/wok/objectstore
>>   %{_localstatedir}/log/wok/*
>>   %{_localstatedir}/log/wok/
>>   %{_mandir}/man8/wokd.8.gz
>> @@ -115,7 +116,7 @@ rm -rf $RPM_BUILD_ROOT
>>
>>
>>   %changelog
>> -* Fri Jun 19 2015 Lucio Correia <luciojhc at linux.vnet.ibm.com> 1.6
>> +* Fri Jun 19 2015 Lucio Correia <luciojhc at linux.vnet.ibm.com> 2.0
>>   - Rename to wokd
>>   - Remove kimchi specifics
>>
>> diff --git a/src/wok/model/model.py b/src/wok/model/model.py
>> index e8826f2..cb213d3 100644
>> --- a/src/wok/model/model.py
>> +++ b/src/wok/model/model.py
>> @@ -28,7 +28,7 @@ from wok.utils import import_module, listPathModules
>>
>>
>>   class Model(BaseModel):
>> -    def __init__(self, objstore_loc=None):
>> +    def __init__(self, objstore_loc):
>
> Keep it optional and add a default location.
>
>>           self.objstore = ObjectStore(objstore_loc)
>>           kargs = {'objstore': self.objstore}
>> diff --git a/src/wok/objectstore.py b/src/wok/objectstore.py
>> index 670a363..810c5be 100644
>> --- a/src/wok/objectstore.py
>> +++ b/src/wok/objectstore.py
>> @@ -29,7 +29,6 @@ except ImportError:
>>       from ordereddict import OrderedDict
>>
>>
>> -from wok import config
>>   from wok.exception import NotFoundError
>>   from wok.utils import wok_log
>>
>> @@ -83,10 +82,10 @@ class ObjectStoreSession(object):
>>
>>
>>   class ObjectStore(object):
>> -    def __init__(self, location=None):
>> +    def __init__(self, location):
>>           self._lock = threading.Semaphore()
>>           self._connections = OrderedDict()
>> -        self.location = location or config.get_object_store()
>> +        self.location = location
>>           with self._lock:
>>               self._init_db()
>>
>> diff --git a/src/wok/plugins/kimchi/Makefile.am
>> b/src/wok/plugins/kimchi/Makefile.am
>> index 1080005..404c3f4 100644
>> --- a/src/wok/plugins/kimchi/Makefile.am
>> +++ b/src/wok/plugins/kimchi/Makefile.am
>> @@ -99,7 +99,7 @@ config.py: config.py.in Makefile
>>   install-deb: install
>>       cp -R $(top_srcdir)/contrib/DEBIAN $(DESTDIR)/
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi
>> -    touch $(DESTDIR)/$(localstatedir)/lib/wok/objectstore
>> +    touch $(DESTDIR)/$(localstatedir)/lib/kimchi/objectstore
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/vnc-tokens
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/debugreports
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/screenshots
>> @@ -140,7 +140,7 @@ ChangeLog:
>>
>>   install-data-local:
>>       $(MKDIR_P) $(DESTDIR)/$(localstatedir)/lib/kimchi/
>> -    touch $(DESTDIR)/$(localstatedir)/lib/wok/objectstore
>> +    touch $(DESTDIR)/$(localstatedir)/lib/kimchi/objectstore
>>       $(MKDIR_P) $(DESTDIR)$(kimchidir)
>>       $(INSTALL_DATA) API.json $(DESTDIR)$(kimchidir)/API.json
>>       mkdir -p $(DESTDIR)/$(localstatedir)/lib/kimchi/vnc-tokens
>> diff --git a/src/wok/plugins/kimchi/config.py.in
>> b/src/wok/plugins/kimchi/config.py.in
>> index 6ae0ccd..43e94e1 100644
>> --- a/src/wok/plugins/kimchi/config.py.in
>> +++ b/src/wok/plugins/kimchi/config.py.in
>> @@ -42,6 +42,10 @@ def get_debugreports_path():
>>       return os.path.join(PluginPaths('kimchi').state_dir,
>> 'debugreports')
>>
>>
>> +def get_object_store():
>> +    return os.path.join(PluginPaths('kimchi').state_dir, 'objectstore')
>> +
>> +
>>   def get_screenshot_path():
>>       return os.path.join(PluginPaths('kimchi').state_dir, 'screenshots')
>>
>> diff --git a/src/wok/plugins/kimchi/contrib/kimchi.spec.fedora.in
>> b/src/wok/plugins/kimchi/contrib/kimchi.spec.fedora.in
>> index e4b7b6d..41a8a86 100644
>> --- a/src/wok/plugins/kimchi/contrib/kimchi.spec.fedora.in
>> +++ b/src/wok/plugins/kimchi/contrib/kimchi.spec.fedora.in
>> @@ -100,10 +100,10 @@ rm -rf $RPM_BUILD_ROOT
>>   %{_sysconfdir}/kimchi/
>>   %{_sharedstatedir}/kimchi/debugreports/
>>   %{_sharedstatedir}/kimchi/isos/
>> +%{_sharedstatedir}/kimchi/objectstore
>>   %{_sharedstatedir}/kimchi/screenshots/
>>   %{_sharedstatedir}/kimchi/vnc-tokens/
>>   %{_sharedstatedir}/kimchi/
>> -%{_sharedstatedir}/wok/objectstore
>>
>>
>>   %changelog
>> diff --git a/src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in
>> b/src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in
>> index 03dfa7b..6fbdb3e 100644
>> --- a/src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in
>> +++ b/src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in
>> @@ -87,10 +87,10 @@ rm -rf $RPM_BUILD_ROOT
>>   %{_sysconfdir}/kimchi/
>>   %{_var}/lib/kimchi/debugreports/
>>   %{_var}/lib/kimchi/isos/
>> +%{_var}/lib/kimchi/objectstore
>>   %{_var}/lib/kimchi/screenshots/
>>   %{_var}/lib/kimchi/vnc-tokens/
>>   %{_var}/lib/kimchi/
>> -%{_var}/lib/wok/objectstore
>>
>>
>>   %changelog
>> diff --git a/src/wok/plugins/kimchi/mockmodel.py
>> b/src/wok/plugins/kimchi/mockmodel.py
>> index 0832b20..4dd063c 100644
>> --- a/src/wok/plugins/kimchi/mockmodel.py
>> +++ b/src/wok/plugins/kimchi/mockmodel.py
>> @@ -67,7 +67,7 @@ class MockModel(Model):
>>       _undefineDomain = libvirt.virDomain.undefine
>>       _libvirt_get_vol_path = LibvirtVMTemplate._get_volume_path
>>
>> -    def __init__(self, objstore_loc=None):
>> +    def __init__(self, objstore_loc):
>>           # Override osinfo.defaults to ajust the values according to
>>           # test:///default driver
>>           defaults = dict(osinfo.defaults)
>> diff --git a/src/wok/plugins/kimchi/root.py
>> b/src/wok/plugins/kimchi/root.py
>> index 1e2bfc7..21c3131 100644
>> --- a/src/wok/plugins/kimchi/root.py
>> +++ b/src/wok/plugins/kimchi/root.py
>> @@ -33,12 +33,14 @@ from model import model as kimchiModel
>>
>>   class KimchiRoot(WokRoot):
>>       def __init__(self, wok_options):
>> +        objstore_loc = config.get_object_store()
>>           if hasattr(wok_options, "model"):
>>               self.model = wok_options.model
>>           elif wok_options.test:
>> -            self.model = mockmodel.MockModel()
>> +            mockstore = "%s-mock" % os.path.abspath(objstore_loc)
>> +            self.model = mockmodel.MockModel(mockstore)
>>           else:
>> -            self.model = kimchiModel.Model()
>> +            self.model = kimchiModel.Model(objstore_loc=objstore_loc)
>>
>>           dev_env = wok_options.environment != 'production'
>>           super(KimchiRoot, self).__init__(self.model, dev_env)
>> @@ -57,6 +59,7 @@ class KimchiRoot(WokRoot):
>>           self.messages = messages
>>
>>           make_dirs = [
>> +            os.path.dirname(os.path.abspath(config.get_object_store())),
>>               os.path.abspath(config.get_distros_store()),
>>               os.path.abspath(config.get_debugreports_path()),
>>               os.path.abspath(config.get_screenshot_path())
>> diff --git a/src/wok/plugins/kimchi/tests/test_yumparser.py
>> b/src/wok/plugins/kimchi/tests/test_yumparser.py
>> index be5e95c..c69f574 100644
>> --- a/src/wok/plugins/kimchi/tests/test_yumparser.py
>> +++ b/src/wok/plugins/kimchi/tests/test_yumparser.py
>> @@ -23,6 +23,7 @@ import unittest
>>
>>   from wok.rollbackcontext import RollbackContext
>>
>> +from wok.plugins.kimchi import config
>>   from wok.plugins.kimchi.model import model
>>   from wok.plugins.kimchi.yumparser import delete_repo_from_file,
>> get_repo_files
>>   from wok.plugins.kimchi.yumparser import get_yum_packages_list_update
>> @@ -34,7 +35,7 @@ TEMP_REPO_FILE = ''
>>
>>
>>   def _is_yum_distro():
>> -    inst = model.Model('test:///default')
>> +    inst = model.Model('test:///default', config.get_object_store())
>>       repo_type = inst.capabilities_lookup()['repo_mngt_tool']
>>       return repo_type == 'yum'
>>
>> diff --git a/src/wok/server.py b/src/wok/server.py
>> index c6f12dd..071c425 100644
>> --- a/src/wok/server.py
>> +++ b/src/wok/server.py
>> @@ -124,7 +124,7 @@ class Server(object):
>>           if hasattr(options, 'model'):
>>               model_instance = options.model
>>           else:
>> -            model_instance = model.Model()
>> +            model_instance = model.Model(config.get_object_store())
>
> This model_instance is related to WOK! And we are setting the
> objectstore location used by Kimchi!

No, here we use the config.get_object_store() from wok. That method 
already existed before in wok, and this patch creates one for Kimchi. 
But here we use Wok method and get wok location


>
> While loading the plugin we need to get the right objectstore location
> and keep it optional to Wok.
>
> Does that make sense?

Yes, I will try to remove the objectstore from wok.


>
> Regards,
> Aline Manera
>
>>           for ident, node in sub_nodes.items():
>>               if node.url_auth:
>


-- 
Lucio Correia
Software Engineer
IBM LTC Brazil




More information about the Kimchi-devel mailing list