pedro/cdist
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'order-dep-fix' into 'master'

Browse source 
Redefine/reimplement CDIST_ORDER_DEPENDENCY

See merge request ungleich-public/cdist!815
This commit is contained in:
poljakowski 2019-11-29 13:55:43 +01:00
commit 131c736d22
9 changed files with 283 additions and 128 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
cdist/config.py
View file

@ -124,6 +124,7 @@ class Config(object):
"""Remove files and directories for the run"""
if self.remove_remote_files_dirs:
self._remove_remote_files_dirs()
self.manifest.cleanup()
@staticmethod
def hosts(source):
@ -787,6 +788,9 @@ class Config(object):
self.explorer.run_type_explorers(cdist_object, transfer_type_explorers)
try:
self.manifest.run_type_manifest(cdist_object)
self.log.trace("[ORDER_DEP] Removing order dep files for %s",
cdist_object)
cdist_object.cleanup()
cdist_object.state = core.CdistObject.STATE_PREPARED
except cdist.Error as e:
raise cdist.CdistObjectError(cdist_object, e)

10
cdist/core/cdist_object.py
View file

@ -243,6 +243,16 @@ class CdistObject(object):
lambda obj: os.path.join(obj.base_path, obj.code_local_path))
code_remote = fsproperty.FileStringProperty(
lambda obj: os.path.join(obj.base_path, obj.code_remote_path))
typeorder = fsproperty.FileListProperty(
lambda obj: os.path.join(obj.absolute_path, 'typeorder'))
typeorder_dep = fsproperty.FileListProperty(
lambda obj: os.path.join(obj.absolute_path, 'typeorder_dep'))
def cleanup(self):
try:
os.remove(os.path.join(self.absolute_path, 'typeorder_dep'))
except FileNotFoundError:
pass
@property
def exists(self):

14
cdist/core/manifest.py
View file

@ -96,6 +96,10 @@ class Manifest(object):
"""Executes cdist manifests.
"""
ORDER_DEP_STATE_NAME = 'order_dep_state'
TYPEORDER_DEP_NAME = 'typeorder_dep'
def __init__(self, target_host, local, dry_run=False):
self.target_host = target_host
self.local = local
@ -212,3 +216,13 @@ class Manifest(object):
type_manifest,
env=self.env_type_manifest(cdist_object),
message_prefix=message_prefix)
def cleanup(self):
def _rm_file(fname):
try:
self.log.trace("[ORDER_DEP] Removing %s", fname)
os.remove(os.path.join(self.local.base_path, fname))
except FileNotFoundError:
pass
_rm_file(Manifest.ORDER_DEP_STATE_NAME)
_rm_file(Manifest.TYPEORDER_DEP_NAME)

124
cdist/emulator.py
View file

@ -29,6 +29,7 @@ import sys
import cdist
from cdist import core
from cdist import flock
from cdist.core.manifest import Manifest
class MissingRequiredEnvironmentVariableError(cdist.Error):
@ -82,6 +83,11 @@ class Emulator(object):
self.object_base_path = os.path.join(self.global_path, "object")
self.typeorder_path = os.path.join(self.global_path, "typeorder")
self.typeorder_dep_path = os.path.join(self.global_path,
Manifest.TYPEORDER_DEP_NAME)
self.order_dep_state_path = os.path.join(self.global_path,
Manifest.ORDER_DEP_STATE_NAME)
self.type_name = os.path.basename(argv[0])
self.cdist_type = core.CdistType(self.type_base_path, self.type_name)
@ -206,6 +212,14 @@ class Emulator(object):
return params
def setup_object(self):
# CDIST_ORDER_DEPENDENCY state
order_dep_on = self._order_dep_on()
order_dep_defined = "CDIST_ORDER_DEPENDENCY" in self.env
if not order_dep_defined and order_dep_on:
self._set_order_dep_state_off()
if order_dep_defined and not order_dep_on:
self._set_order_dep_state_on()
# Create object with given parameters
self.parameters = {}
for key, value in vars(self.args).items():
@ -237,6 +251,20 @@ class Emulator(object):
# record the created object in typeorder file
with open(self.typeorder_path, 'a') as typeorderfile:
print(self.cdist_object.name, file=typeorderfile)
# record the created object in parent object typeorder file
__object_name = self.env.get('__object_name', None)
depname = self.cdist_object.name
if __object_name:
parent = self.cdist_object.object_from_name(__object_name)
parent.typeorder.append(self.cdist_object.name)
if self._order_dep_on():
self.log.trace(('[ORDER_DEP] Adding %s to typeorder dep'
' for %s'), depname, parent.name)
parent.typeorder_dep.append(depname)
elif self._order_dep_on():
self.log.trace('[ORDER_DEP] Adding %s to global typeorder dep',
depname)
self._add_typeorder_dep(depname)
# Record / Append source
self.cdist_object.source.append(self.object_source)
@ -293,45 +321,73 @@ class Emulator(object):
return cdist_object.name
def _order_dep_on(self):
return os.path.exists(self.order_dep_state_path)
def _set_order_dep_state_on(self):
self.log.trace('[ORDER_DEP] Setting order dep state on')
with open(self.order_dep_state_path, 'w'):
pass
def _set_order_dep_state_off(self):
self.log.trace('[ORDER_DEP] Setting order dep state off')
# remove order dep state file
try:
os.remove(self.order_dep_state_path)
except FileNotFoundError:
pass
# remove typeorder dep file
try:
os.remove(self.typeorder_dep_path)
except FileNotFoundError:
pass
def _add_typeorder_dep(self, name):
with open(self.typeorder_dep_path, 'a') as f:
print(name, file=f)
def _read_typeorder_dep(self):
try:
with open(self.typeorder_dep_path, 'r') as f:
return f.readlines()
except FileNotFoundError:
return []
def record_requirements(self):
"""Record requirements."""
order_dep_on = self._order_dep_on()
# Inject the predecessor, but not if its an override
# (this would leed to an circular dependency)
if ("CDIST_ORDER_DEPENDENCY" in self.env and
'CDIST_OVERRIDE' not in self.env):
# load object name created befor this one from typeorder file ...
with open(self.typeorder_path, 'r') as typecreationfile:
typecreationorder = typecreationfile.readlines()
# get the type created before this one ...
try:
lastcreatedtype = typecreationorder[-2].strip()
# __object_name is the name of the object whose type
# manifest is currently executed
__object_name = self.env.get('__object_name', None)
if lastcreatedtype == __object_name:
self.log.debug(("Not injecting require for "
"CDIST_ORDER_DEPENDENCY: %s for %s,"
" %s's type manifest is currently"
" being executed"),
lastcreatedtype,
self.cdist_object.name,
lastcreatedtype)
else:
if 'require' in self.env:
appendix = " " + lastcreatedtype
if appendix not in self.env['require']:
self.env['require'] += appendix
else:
self.env['require'] = lastcreatedtype
self.log.debug(("Injecting require for "
"CDIST_ORDER_DEPENDENCY: %s for %s"),
lastcreatedtype,
self.cdist_object.name)
except IndexError:
# if no second last line, we are on the first type,
# so do not set a requirement
pass
if (order_dep_on and 'CDIST_OVERRIDE' not in self.env):
try:
# __object_name is the name of the object whose type
# manifest is currently executed
__object_name = self.env.get('__object_name', None)
# load object name created befor this one from typeorder
# dep file
if __object_name:
parent = self.cdist_object.object_from_name(
__object_name)
typeorder = parent.typeorder_dep
else:
typeorder = self._read_typeorder_dep()
# get the type created before this one
lastcreatedtype = typeorder[-2].strip()
if 'require' in self.env:
if lastcreatedtype not in self.env['require']:
self.env['require'] += " " + lastcreatedtype
else:
self.env['require'] = lastcreatedtype
self.log.debug(("Injecting require for "
"CDIST_ORDER_DEPENDENCY: %s for %s"),
lastcreatedtype,
self.cdist_object.name)
except IndexError:
# if no second last line, we are on the first type,
# so do not set a requirement
pass
reqs = set()
if "require" in self.env:

43
cdist/test/emulator/__init__.py
View file

@ -24,8 +24,6 @@
import io
import os
import shutil
import string
import filecmp
import random
import logging
@ -34,7 +32,6 @@ from cdist import test
from cdist.exec import local
from cdist import emulator
from cdist import core
from cdist import config
import os.path as op
my_dir = op.abspath(op.dirname(__file__))
@ -115,7 +112,7 @@ class EmulatorTestCase(test.CdistTestCase):
def test_requirement_pattern(self):
argv = ['__file', '/tmp/foobar']
self.env['require'] = '__file/etc/*'
emu = emulator.Emulator(argv, env=self.env)
emulator.Emulator(argv, env=self.env)
# if we get here all is fine
def test_loglevel(self):
@ -172,6 +169,44 @@ class EmulatorTestCase(test.CdistTestCase):
self.assertEqual(list(file_object.requirements), ['__planet/mars'])
# if we get here all is fine
def test_order_dependency_context(self):
test_seq = ('A', True, 'B', 'C', 'D', False, 'E', 'F', True, 'G',
'H', False, 'I', )
expected_requirements = {
'C': set(('__planet/B', )),
'D': set(('__planet/C', )),
'H': set(('__planet/G', )),
}
# Ensure env var is not in env
if 'CDIST_ORDER_DEPENDENCY' in self.env:
del self.env['CDIST_ORDER_DEPENDENCY']
for x in test_seq:
if isinstance(x, str):
# Clear because of order dep injection
# In real world, this is not shared over instances
if 'require' in self.env:
del self.env['require']
argv = ['__planet', x]
emu = emulator.Emulator(argv, env=self.env)
emu.run()
elif isinstance(x, bool):
if x:
self.env['CDIST_ORDER_DEPENDENCY'] = 'on'
elif 'CDIST_ORDER_DEPENDENCY' in self.env:
del self.env['CDIST_ORDER_DEPENDENCY']
cdist_type = core.CdistType(self.local.type_path, '__planet')
for x in test_seq:
if isinstance(x, str):
obj = core.CdistObject(cdist_type, self.local.object_path,
self.local.object_marker_name, x)
reqs = set(obj.requirements)
if x in expected_requirements:
self.assertEqual(reqs, expected_requirements[x])
else:
self.assertTrue(len(reqs) == 0)
# if we get here all is fine
class EmulatorConflictingRequirementsTestCase(test.CdistTestCase):

91
docs/src/cdist-best-practice.rst
View file

@ -226,8 +226,8 @@ and also to store all important files in one
repository.
Perils of CDIST_ORDER_DEPENDENCY
--------------------------------
Notes on CDIST_ORDER_DEPENDENCY
-------------------------------
With CDIST_ORDER_DEPENDENCY all types are executed in the order in which they
are created in the manifest. The current created object automatically depends
on the previously created object.
@ -235,96 +235,11 @@ on the previously created object.
It essentially helps you to build up blocks of code that build upon each other
(like first creating the directory xyz than the file below the directory).
This can be helpful, but it can also be the source of *evil*.
CDIST_ORDER_DEPENDENCY easily causes unobvious dependency cycles
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Let's see an example. Suppose you have special init manifest where among other
things you are assuring that remote host has packages `sudo` and `curl`
installed.
**init1**
.. code-block:: sh
CDIST_ORDER_DEPENDENCY=1
export CDIST_ORDER_DEPENDENCY
for p in sudo curl
do
__package "${p}"
done
Then you have some other special init manifest where among other things you are
assuring `sudo` package is installed.
**init2**
.. code-block:: sh
CDIST_ORDER_DEPENDENCY=1
export CDIST_ORDER_DEPENDENCY
__package sudo
Then you have third init manifest where you combine those two init manifests,
by including them:
**init**
.. code-block:: sh
sh -e "$__manifest/init1"
sh -e "$__manifest/init2"
The resulting init manifest is then equal to:
.. code-block:: sh
CDIST_ORDER_DEPENDENCY=1
export CDIST_ORDER_DEPENDENCY
for p in sudo curl
do
__package "${p}"
done
CDIST_ORDER_DEPENDENCY=1
export CDIST_ORDER_DEPENDENCY
__package sudo
In the end you get the following dependencies:
* `__package/curl` depends on `__package/sudo`
* `__package/sudo` depends on `__package/curl`
And here you have a circular dependency!
In the real world manifest can be quite complex, dependencies can become
complicated and circual dependencies are not so obvious. Resolving it can
become cumbersome.
**Practical solution?**
Instead of managing complex init manifests you can write custom types.
Each custom type can do one thing, it has well defined dependencies that will
not leak into init manifest. In custom type you can also add special explorers
and gencode.
Then, in init manifest you combine your complex types. It is:
* cleaner
* easier to follow
* easier to maintain
* easier to debug.
This can be helpful, but one must be aware of its side effects.
CDIST_ORDER_DEPENDENCY kills parallelization
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suppose you have defined CDIST_ORDER_DEPENDENCY and then, among other things,
you specify creation of three, by nature independent, files.

121
docs/src/cdist-manifest.rst
View file

@ -163,7 +163,126 @@ automatically depends on the previously created object.
It essentially helps you to build up blocks of code that build upon each other
(like first creating the directory xyz than the file below the directory).
Read also about `perils of CDIST_ORDER_DEPENDENCY <cdist-best-practice.html#perils-of-cdist-order-dependency>`_.
Read also about `notes on CDIST_ORDER_DEPENDENCY <cdist-best-practice.html#notes-on-cdist-order-dependency>`_.
In version 6.2.0 semantic CDIST_ORDER_DEPENDENCY is finally fixed and well defined.
CDIST_ORDER_DEPENDENCY defines type order dependency context. Order dependency context
starts when CDIST_ORDER_DEPENDENCY is set, and ends when it is unset. After each
manifest execution finishes, any existing order dependency context is automatically
unset. This ensures that CDIST_ORDER_DEPENDENCY is valid within the manifest where it
is used. When order dependency context is defined then cdist executes types in the
order in which they are created in the manifest inside order dependency context.
Sometimes the best way to see how something works is to see examples.
Suppose you have defined **initial manifest**:
.. code-block:: sh
__cycle1 cycle1
export CDIST_ORDER_DEPENDENCY=1
__cycle2 cycle2
__cycle3 cycle3
with types **__cycle1**:
.. code-block:: sh
export CDIST_ORDER_DEPENDENCY=1
__file /tmp/cycle11
__file /tmp/cycle12
__file /tmp/cycle13
**__cycle2**:
.. code-block:: sh
__file /tmp/cycle21
export CDIST_ORDER_DEPENDENCY=1
__file /tmp/cycle22
__file /tmp/cycle23
unset CDIST_ORDER_DEPENDENCY
__file /tmp/cycle24
**__cycle3**:
.. code-block:: sh
__file /tmp/cycle31
__file /tmp/cycle32
export CDIST_ORDER_DEPENDENCY=1
__file /tmp/cycle33
__file /tmp/cycle34
For the above config, cdist results in the following expected *dependency graph*
(type *__cycleX* is shown as *cX*, *__file/tmp/cycleXY* is shown as *fcXY*):
::
c1---->fc11
| /\
| |
+----->fc12
| /\
| |
+----->fc13
c2--+--->fc21
/\ |
| |
| +----->fc22
| | /\
| | |
| +----->fc23
| |
| |
| +----->fc24
|
|
c3---->fc31
|
|
+----->fc32
|
|
+----->fc33
| /\
| |
+----->fc34
Before version 6.2.0 the above configuration would result in cycle:
::
ERROR: 185.203.112.26: Cycle detected in object dependencies:
__file/tmp/cycle11 -> __cycle3/cycle3 -> __cycle2/cycle2 -> __cycle1/cycle1 -> __file/tmp/cycle11!
The following manifest shows an example for order dependency contexts:
.. code-block:: sh
__file /tmp/fileA
export CDIST_ORDER_DEPENDENCY=1
__file /tmp/fileB
__file /tmp/fileC
__file /tmp/fileD
unset CDIST_ORDER_DEPENDENCY
__file /tmp/fileE
__file /tmp/fileF
export CDIST_ORDER_DEPENDENCY=1
__file /tmp/fileG
__file /tmp/fileH
unset CDIST_ORDER_DEPENDENCY
__file /tmp/fileI
This means:
* C depends on B
* D depends on C
* H depends on G
and there are no other dependencies from this manifest.
Overrides

2
docs/src/cdist-reference.rst.sh
View file

@ -330,7 +330,7 @@ CDIST_OVERRIDE
CDIST_ORDER_DEPENDENCY
Create dependencies based on the execution order (see \`cdist manifest <cdist-manifest.html>\`_).
Read also about \`perils of CDIST_ORDER_DEPENDENCY <cdist-best-practice.html#perils-of-cdist-order-dependency>\`_.
Note that in version 6.2.0 semantic of this processing mode is finally fixed and well defined.
CDIST_REMOTE_EXEC
Use this command for remote execution (should behave like ssh).

2
docs/src/man1/cdist.rst
View file

@ -827,6 +827,8 @@ CDIST_OVERRIDE
CDIST_ORDER_DEPENDENCY
Create dependencies based on the execution order.
Note that in version 6.2.0 semantic of this processing mode is
finally fixed and well defined.
CDIST_REMOTE_EXEC
Use this command for remote execution (should behave like ssh).