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(a)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(a)linux.vnet.ibm.com> 1.6
> +* Fri Jun 19 2015 Lucio Correia <luciojhc(a)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(a)linux.vnet.ibm.com> 1.6
> +* Fri Jun 19 2015 Lucio Correia <luciojhc(a)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