The check_commit_msg.py complains that the commis message should be of the form :

'tendrl-bug-id:<tendrl_repo>/issue_id'

It should probably say :

tendrl-bug-id:Tendrl/<tendrl_repo_name>#<issue_id>

Source: https://github.com/Tendrl/commons/pull/175/files#r103904583

Summary: "If any fields are empty in class attributes at that time read will never happen for that attribute.
for example we take volume class with attributes vol_id, Vol_name_ vol_size (just for example), There is two machines node A and B. If node A pushing vol_id and vol_name into etcd using volume class. so vol_size is None for node A class. Now node B updating same volume path in etcd with vol_id, vol_name and vol_size (all). Now in etcd volume path contains all fields are populated. If node A try to read volume details from etcd using volume class, then it can be read vol_id and vol_name only, it can't read vol_size. (but vol_size is available in etcd). Problem is some where class attribute values are maintained. so vol_size attribute is None so etcd request for that attribute is not happening.

How this case is affecting jobs means, job is pushed from api, so all the job class attributes are initially empty. so it can't read any value.

"

Unfortunately, it looks like the bridge_common is missing config file that wasn't added. we have to fix this issue.

Node-agent travis log: https://travis-ci.org/Tendrl/node-agent/builds/192361021

tendrl.commons.utils.service_status.ServiceStatus _execute_service_command fails because no "exec_path" provided to Command instance

While execution of flows, we should not compulsorily looks for pre/post runs for the flows.

Version specified in setup.py file is 0.0.1, while the latest git tag (and releases page provided for these tags by github) is v1.1.

Details

$ grep version setup.py
    version="0.0.1",
$ git tag
v1.0
v1.1

Related Upstream Guidelines

https://packaging.python.org/distributing/#choosing-a-versioning-scheme
https://packaging.python.org/single_source_version/#single-sourcing-the-version

Test environment py26 of unit test fails with the following error:

Collecting gevent>=1.0 (from tendrl-commons==1.2.1)
  Downloading gevent-1.2.1.tar.gz (2.8MB)
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-build-A756VU/gevent/setup.py", line 39, in <module>
        raise Exception("Please install gevent 1.1 for Python 2.6")
    Exception: Please install gevent 1.1 for Python 2.6

See full log in Travis CI job.

We run unit tests in py26 test environment to check if tendrl-commons would work on CentOS 6.

Gevent rpm package is not part of CentOS 6 distribution, but it is part of EPEL 6 repository (which contains gevent 0.13.8-3.el6). See python-gevent report from fedoraproject.org.

This means that we need to decide which version we will use on CentOS 6 - either 0.13.8-3.el6 from EPEL 6 or maintain different version ourselves (as right now, the minimal version of gevent we require is 1.0 - as specified in install_requires in setup.py). When we figure out which version we will use on CentOS 6, we would just specify this version in configuration of our test environment like this:

$ git diff
diff --git a/tox.ini b/tox.ini
index 75187a1..8214098 100644
--- a/tox.ini
+++ b/tox.ini
@@ -20,6 +20,7 @@ deps =
     mock
     coverage
     pytest-cov
+    py26: gevent==0.13.8
     cover: codecov
 commands =
     python -m pytest --ignore=tendrl/commons/tests/fixme --cov=tendrl tendrl/commons/tests

So that when py26 test run is executed, correct version of gevent will be installed into test environment.

Repository names need to be updated to follow lowercase, hyphenated convention.

As part of re-factoring, the unit test coverage is decreased. More unit tests should be written for better coverage.

I noticed that most of example/default config files in tendrl-common are present in all other python tendrl projects.

This state has additional maintenance cost and it's error prone when changes needs to be introduced in those files. In other words, it is be hard to maintain in the long term.

Related Work/Issues

The root cause of this is the same as in few other already reported issues:

• Tendrl/node-agent#99
• Tendrl/ceph-integration#57
• Tendrl/gluster-integration#86

This particular issue is created so that we consider this side of the problem as well during redesign of Tendrl configuration.

Details

There are these example config files in tendrl-common:

$ find common/etc/ -type f
common/etc/tendrl/tendrl.conf.sample
common/etc/samples/logging.yaml.syslog.sample
common/etc/samples/tendrl_definitions_generic-0.3.sample.yaml
common/etc/samples/tendrl_api_job.sample.json
common/etc/samples/logging.yaml.timedrotation.sample
common/etc/samples/logging.yaml.journald.sample

And 4 of them (out of 6) are present in all other python tendlr components:

• tendrl.conf.sample
• logging.yaml.journald.sample
• logging.yaml.timedrotation.sample
• logging.yaml.syslog.sample

Changes in each components look like this:

$ diff common/etc/tendrl/tendrl.conf.sample node_agent/etc/tendrl/tendrl.conf.sample 
10d9
< log_level = DEBUG
11a11,22
> log_level = DEBUG
> 
> 
> [node_agent]
> # Path to log file
> log_level = DEBUG
> tendrl_exe_file_prefix = /tmp/.tendrl_runner
> log_cfg_path = /etc/tendrl/node_agent_logging.yaml
> 
> # installation_source_type decides how tendrl will install a software
> # the possible values are pip, rpm, deb.
> installation_source_type = rpm

$ diff common/etc/tendrl/tendrl.conf.sample ceph_bridge/etc/tendrl/tendrl.conf.sample 
10d9
< log_level = DEBUG
11a11,23
> log_level = DEBUG
> 
> 
> [ceph_integration]
> # Path to log file
> log_cfg_path = /etc/tendrl/logging.yaml
> log_level = DEBUG
> crush_host_type = host
> crush_osd_type = osd
> favorite_timeout_factor = 3
> server_timeout_factor = 3
> cluster_contact_threshold = 60
> cluster_map_retention = 3600

The common codebase needs to behave as a library and not as an application. Since it cannot be used on it's own, and only be used as part of another codebase such as the node agent, all the configuration file samples and configuration file locations should be shipped as part of these applications. The common codebase should provide only the code that can take configurable parameters that point to the correct path of the configuration files.

Tied to #62.

With latest commons:master, while starting the ceph-integration it shows the below stack trace

Traceback (most recent call last):
  File "/usr/bin/tendrl-ceph-integration", line 11, in <module>
    load_entry_point('tendrl-ceph-integration==1.2', 'console_scripts', 'tendrl-ceph-integration')()
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 560, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2648, in load_entry_point
    return ep.load()
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2302, in load
    return self.resolve()
  File "/usr/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2308, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/usr/lib/python2.7/site-packages/tendrl_ceph_integration-1.2-py2.7.egg/tendrl/ceph_integration/__init__.py", line 45, in <module>
    CephIntegrationNS()
  File "/usr/lib/python2.7/site-packages/tendrl_ceph_integration-1.2-py2.7.egg/tendrl/ceph_integration/__init__.py", line 43, in __init__
    super(CephIntegrationNS, self).__init__()
  File "/usr/lib/python2.7/site-packages/tendrl_commons-1.2-py2.7.egg/tendrl/commons/__init__.py", line 27, in __init__
    self.register_subclasses_to_ns()
  File "/usr/lib/python2.7/site-packages/tendrl_commons-1.2-py2.7.egg/tendrl/commons/__init__.py", line 144, in register_subclasses_to_ns
    self.add_obj_flow(flow_cls[1].obj.__name__,
AttributeError: type object 'CephIntegrationBaseFlow' has no attribute 'obj'

Seems to be some small change required in BaseFlow definition, but not sure what.

Two tests from py27 test run fails:

tendrl/commons/tests/test_base_flow.py::TestBaseFlow::test_run ERROR
tendrl/commons/tests/test_base_flow.py::TestBaseFlow::test_extract_atom_details ERROR

Errors collected by pytest show importing modules which doesn't exist, eg:

ImportError while importing test module '/home/travis/build/Tendrl/commons/tendrl/commons/tests/test_commons.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tendrl/commons/tests/test_commons.py:12: in <module>
    from tendrl.commons.etcdobj import etcdobj
E   ImportError: cannot import name etcdobj

See full log from Travis CI.

It should easier and less error prone to have just one etcd_url directive in lieu of the current etcd_port and etcd_connection directives. etcd_url should accept http urls, including the port eg. http://localhost:2379.

Furthermore, since the specification of the port is optional in an http url, the value, if not specified, should default to 2379.

The current implementation brings up issues around the handling of the directives eg. should the etcd_connection be specified without a port but etcd_port be missing etc. There are a few such permutations that would need to be handled, specifically regarding the population of default values, if the directives are kept separate. All of these can be avoided by using a single directive and assuming a default value of the port to be 2379.

Package tendrl-common-0.0.1-1.el7.centos.noarch from https://copr-be.cloud.fedoraproject.org/results/tendrl/tendrl/epel-7-x86_64/ copr doesn't mark configuration files as configuration.

This breaks Fedora packaging policy and will have inpact on user changes in
affected configuration files during package upgrades.

Reproducer

# rpm -ql tendrl-common | grep ^/etc
/etc/tendrl
/etc/tendrl/common_logging.yaml
# rpm -qc tendrl-common 
#

While expected result would be:

# rpm -ql tendrl-common | grep ^/etc
/etc/tendrl
/etc/tendrl/common_logging.yaml
# rpm -qc tendrl-common 
/etc/tendrl/common_logging.yaml
#

Additional information

Fedora wikipage on rpm packaging states:

Configuration files should be placed in /etc and are normally specified like
this (which makes sure user changes aren't overwritten on update):

%config(noreplace) %{_sysconfdir}/foo.conf

If the update uses a non-backwards-compatible configuration format, then
specify them like this:

%config %{_sysconfdir}/foo.conf

You can also see additional description of rpm and configuration files.

This issue suggest to remove requirements.txt file and move items which this file contains into install_requires setuptools keyword in setup.py file.

Reasoning

Based on explanation of difference between install_requires keyword and requirements.txt file from upstream Python Packaging User Guide, I understand that:

• python project should provide minimal abstract list of projects which are needed to run it correctly in install_requires
• requirements.txt files are used to define the requirements for a complete python environment

Since this project is a single Tendrl component, which is expected to be installed with other Tendrl components together, using requirement file seems to be inappropriate. Listing dependencies of single reusable python library/tool/daemon, which is our case here, is not mentioned among 4 common uses of Requirements files as listed in description of the feature in packaging guide. Moreover, it creates additional issues:

• using requirements.txt file makes us to install the project in non-production way, which means that we will find problems with listing requirements later than we should, see eg. issue https://github.com/Tendrl/performance_monitoring/issues/17 - this is very blatant issue, but was not noticed by any developer, unit tests or CI - since all of those are using installation method which doesn't resemble what an admin using this project would do
• keeping requirements.txt file while specifying install_requires in setup.py requires to maintain function to read requirements.txt file in setup.py, which is unnecessary maintenance burden
• feature set of requirements.txt is higher and includes features which should not be done in production setup

PEP8 code style is not followed which makes pep8 check fail during unit test execution, see latest report from Travis CI.

There are few serious issues among errors, such as:

tendrl/commons/message.py:50:28: F821 undefined name 'tendrl_ns'
            self.node_id = tendrl_ns.node_context.node_id

Note: to run pep8 check yourself locally, run tox -e pep8 in current master branch, the environment and the check would be the same as run by Travis CI.

Tests from py34 test run fails because of importing ansible modules, eg:

____________ ERROR collecting tendrl/commons/tests/test_command.py _____________
ImportError while importing test module '/home/travis/build/Tendrl/commons/tendrl/commons/tests/test_command.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tendrl/commons/tests/test_command.py:9: in <module>
    from tendrl.commons.utils import cmd_utils
tendrl/commons/utils/cmd_utils.py:4: in <module>
    from ansible_module_runner import AnsibleExecutableGenerationFailed
E   ImportError: No module named 'ansible_module_runner'

See full log from Travis CI.

This seems to happen (correct me if I'm wrong) because of Ansible Python 3 support, which has been introduced only recently as a tech preview, and as such is not production ready.

We may consider either dropping py34-cover test run from Travis CI configuration, or skipping test cases which uses ansible modules under the hood via pytest.mark.skipif or flagging the test cases via xfail when executed with python3 - this depends on how important is for the tendrl code to be python3 compatible.

Both the state and the operations definition files should allow the visibility of all the entities (objects, atoms, attributes and flows) via the API, to be controlled. This enables a workflow where the supporting backend implementation would first have to implement the support for a certain entity. Once the implementation is complete, that entity would then be marked as available for manipulation via the API.

To do this, the definition file needs to support:

• a global default visibility flag, and
• per entity visibility flags that overrides the global default.

Salt package is not a requirement for bridge common. So it has to be removed from the requirements file.

• There is a need to define all valid objects and valid actions on each object in an SDS system.
• This yaml SDS description will serve as a way to identify/validate/process incoming user requests via Tendrl REST API.

There should be document which describes:

what software should be installed and how should be configured before installing Tendrl
Tendrl installation process
Tendrl post installation steps like configuration, service starting and so on

[was originally at https://tendrl.atlassian.net/browse/TEN-36]

The base classes for atoms and flows should be defined in common module and other modules should extend from these

Please refer: https://travis-ci.org/Tendrl/bridge_common/builds/161572084

In the build logs, it is observed that pep8 command passed and code coverage command failed at the end.

We should make sure that pep8 and code coverage commands both have passed and only then return a OK

Package python-gevent is used in gluster-integration, node-agent, performance-monitoring and node-monitoring. We can move this dependency into commons and can remove from other tendrl packages whichever uses commons as a dependency.

1. Fix Directory not found /etc/tendrl error during rpm build
2. new python module "namspaces" is used in this package.
   Add an appropriate build dependency.
3. Add spec file to build "namespaces" python module from source
   The package is not available in epel or any other reliable repo.

The common module should contain the base classes for manager, persister and job queue handler and other modules should extend/use them.

Add a way to decorate a class as java equivalent singleton so that it is created only once thereby helping to avoid sometimes unnecessary redundant initialization.

Look for the parameter node_ids in job and if current node's id falls under this list then only pick and process the job.

requirements.txt currently contains python-dateutil==2.2

Would you please loosen this version requirement, or drop the number altogether?

RHEL/CentOS 7 base has python-dateutil-1.5, and Ubuntu Xenial has python-dateutil 2.4.2.

"commons" is not the proper word for this repo.
"common" would be more to the intended purpose wouldn't it?

python-namespaces 0 library is deprecated in favor of python-maps 1, Please package it as it is required by the "develop" branch

There are multiple issues with current setuptools, tox and travis configuration, see Tendrl/notifier#24 for full list.

Expected state

Running tox in the root directory of the project, executes all unit tests and other checks, assuming I have all dev packages installed, both on Fedora/CentOS and Debian/Ubuntu. This would encourage developers to actually write unit tests, because there would be an easy and direct way to execute them.

Travis CI job would execute unit tests during it's run via tox, along other checks it does right now.

Message framework needs an exception message class for handle and format the exception. the exception message class is the subclass of message class, and it should able to create exception message object with well formatted traceback information.

Tox: https://tox.readthedocs.io/en/latest/

Tox is a generic virtualenv management and test command line tool you can use for:
checking your package installs correctly with different Python versions and interpreters
running your tests in each of the environments, configuring your test tool of choice
acting as a frontend to Continuous Integration servers, greatly reducing boilerplate and merging CI and shell-based testing.

Moving on, all git commits should contain mandatory mention on newline of
"tendrl-bug-id: <tendrl_repo>/<github-issue-id>" and optional mention of "tendrl-spec: <tendrl_spec_name>" in the commit msg.

eg:
"""
Packaging python-etcd

Added Makefile and spec file to create python-etcd package
Now one can use "make rpm" to pull the latest source from github
and build the source rpm
and using the Makefile one can also build rpm using "make rpm"
which will use mock to build the rpm using srpm.

tendrl-bug-id: #18

tendrl-spec: refactor_gluster_integration_get_state_dump
"""

We need to fix pep8 errors from latest builds

How to detect pep8 errors:

• After cloning bridge_common
• run "tox -epep8" from the root of the repository and fix the failures.

Current build: https://travis-ci.org/Tendrl/bridge_common

We need to collectively start improving the unit test code coverage for this project.

Current coverage: https://coveralls.io/github/Tendrl/bridge_common

Unit tests framework: http://docs.openstack.org/developer/oslotest/

How to write unit tests: http://docs.openstack.org/developer/horizon/topics/testing.html

Contribute patches here: https://github.com/Tendrl/bridge_common/tree/master/bridge_common/tests

Lets work on this issue together to get code coverage up to 90%, please share doubts/inputs on this issue itself

The flow extractor is not generic as of now, it fails while parsing flows that are tied to objects. It works only if flows is in global name-space. This has to be fixed by making it more generic

The job acceptor has to be modified to accept jobs only if the "targeted_tags" in the job matches with the node agent tags.

For CI and integration testing it is important to install all Tendrl components from packages.

Logging configuration and output is the domain of an application such as the node agent or bridges that use common as a library. The common codebase should ship with modules that allow a common logging code to be reused, but common should never require any logging configuration or file by itself. The applications should ship with sample configuration files custom tuned for each application. The common code should allow these configuration files and the actual logging to be used appropriately in the applications, without having to be re-implemented.

Tied to #60.

Multiple tendrl components using tendrl-commons module have the same problem with design of configuration processing, eg:

• Tendrl/node-agent#245
• Tendrl/ceph-integration#146

We should investigate if the problem is not just in all those affected components, but also in tendrl-commons itself.

If the problem turns out to be in tendrl-commons, it would be a serious one as it would be blocking proper unit testing of all other tendrl python components using tendrl-commons.

Add all general purpose utilities like command-executor, package-installer, service-manager etc to common. So that all the components can make use of this.

Place all these under utils.
This issue is linked to specification issue: Tendrl/specifications#72

The common module should define specific exception classes for flows, atoms etc in corresponding python modules. Other modules should use these exceptions.

Common should read configuration details from etcd and able to pass particular configuration details to the modules

Developer installation documentation (admin/user centric documentation focusing on binary packages was not available when this issue was created) explains tendrl-common configuration as follows:

# cp etc/tendrl/tendrl.conf.sample /etc/tendrl/tendrl.conf
# cp etc/sample/logging.yaml.timedrotation.sample /etc/tendrl/common_logging.conf
# vim /etc/tendrl/tendrl.conf # ... configure value of etcd_connection ...

While in the tendrl-common-0.0.1-1.el7.centos.noarch package, there is just common_logging.conf file packaged in /etc/tendrl directory:

# rpm -ql tendrl-common | grep ^/etc
/etc/tendrl
/etc/tendrl/common_logging.yaml

What is the reason for this? I'm just curious.