Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First stable release almost ready #68

Merged
merged 33 commits into from
Jan 20, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
92fed6f
allow passing dry_run to individual tasks
dbarrosop Dec 21, 2017
37e1fa5
added write task
dbarrosop Dec 21, 2017
29f4c56
redundant
dbarrosop Dec 24, 2017
2a926df
log by default to ./brigade.log
dbarrosop Dec 24, 2017
b0ca3e0
raise an error properly if we fail to connect to the device
dbarrosop Dec 24, 2017
8063186
added more devices to the lab
dbarrosop Dec 24, 2017
eadbf11
examples WIP
dbarrosop Dec 27, 2017
a230151
improvements to napalm_configure
dbarrosop Dec 27, 2017
3184632
unnecessary
dbarrosop Dec 27, 2017
027a440
improvements to logging
dbarrosop Dec 27, 2017
5e59282
better error handling
dbarrosop Dec 27, 2017
52f63f9
document new Result attributes
dbarrosop Dec 27, 2017
4972edd
allow overriding raise_on_error per task
dbarrosop Dec 29, 2017
cecc260
added napalm_validate task
dbarrosop Dec 29, 2017
30e8570
render automatically template path
dbarrosop Dec 29, 2017
0b265ce
Added helper functions to make output pretty
dbarrosop Dec 29, 2017
3e89f8c
fix napalm_validate tests
dbarrosop Dec 29, 2017
0e4027d
bugfix
dbarrosop Dec 31, 2017
ca49110
return properly dict_keys and dict_values
dbarrosop Dec 31, 2017
1e06a18
subtasks now return MultiResults which aggregate mutiple results for …
dbarrosop Jan 6, 2018
4041eec
Merge branch 'develop' of github.com:napalm-automation/brigade into r…
dbarrosop Jan 6, 2018
f830286
skip hosts if they fail and raise_on_error is False
dbarrosop Jan 6, 2018
8b8a4d6
added easy_brigade
dbarrosop Jan 6, 2018
a7096c6
added tutorials
dbarrosop Jan 6, 2018
499d11d
Merge branch 'develop' of github.com:napalm-automation/brigade into r…
dbarrosop Jan 6, 2018
308fd67
bugfix
dbarrosop Jan 15, 2018
57853e2
allow telling a task to be run on hosts that should be skipped otherwise
dbarrosop Jan 15, 2018
7824871
for consistency with tasks
dbarrosop Jan 15, 2018
b9a0d09
allow printing multiresult objects
dbarrosop Jan 15, 2018
df26d81
update docstring
dbarrosop Jan 15, 2018
707c9cc
updated examples
dbarrosop Jan 15, 2018
7e1bb53
minor fixes proposed in the comments
dbarrosop Jan 20, 2018
7b9eef3
adding missing deps
dbarrosop Jan 20, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,4 @@ tags
.vars
output/

demo/log

tests.log

.DS_Store
20 changes: 11 additions & 9 deletions brigade/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def __init__(self, inventory, dry_run, config=None, config_file=None,
logging.basicConfig(
level=logging.ERROR,
format=format,
filename="brigade.log",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we shouldn't enable this by default. People can enable it if they want to, but shouldn't default to enabled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recognize this changes in this commit 027a440, but still defaults to logging to brigade.log (level INFO).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#71

)
if available_connections is not None:
self.available_connections = available_connections
Expand All @@ -89,25 +90,25 @@ def filter(self, **kwargs):
b.inventory = self.inventory.filter(**kwargs)
return b

def _run_serial(self, task, **kwargs):
def _run_serial(self, task, dry_run, **kwargs):
t = Task(task, **kwargs)
result = AggregatedResult()
for host in self.inventory.hosts.values():
try:
logger.debug("{}: running task {}".format(host.name, t))
r = t._start(host=host, brigade=self, dry_run=self.dry_run)
r = t._start(host=host, brigade=self, dry_run=dry_run)
result[host.name] = r
except Exception as e:
logger.error("{}: {}".format(host, e))
result.failed_hosts[host.name] = e
result.tracebacks[host.name] = traceback.format_exc()
return result

def _run_parallel(self, task, num_workers, **kwargs):
def _run_parallel(self, task, num_workers, dry_run, **kwargs):
result = AggregatedResult()

pool = Pool(processes=num_workers)
result_pool = [pool.apply_async(run_task, args=(h, self, Task(task, **kwargs)))
result_pool = [pool.apply_async(run_task, args=(h, self, dry_run, Task(task, **kwargs)))
for h in self.inventory.hosts.values()]
pool.close()
pool.join()
Expand All @@ -121,14 +122,15 @@ def _run_parallel(self, task, num_workers, **kwargs):
result[host] = res
return result

def run(self, task, num_workers=None, **kwargs):
def run(self, task, num_workers=None, dry_run=None, **kwargs):
"""
Run task over all the hosts in the inventory.

Arguments:
task (``callable``): function or callable that will be run against each device in
the inventory
num_workers(``int``): Override for how many hosts to run in paralell for this task
dry_run(``bool``): Whether if we are testing the changes or not
**kwargs: additional argument to pass to ``task`` when calling it

Raises:
Expand All @@ -141,19 +143,19 @@ def run(self, task, num_workers=None, **kwargs):
num_workers = num_workers or self.config.num_workers

if num_workers == 1:
result = self._run_serial(task, **kwargs)
result = self._run_serial(task, dry_run, **kwargs)
else:
result = self._run_parallel(task, num_workers, **kwargs)
result = self._run_parallel(task, num_workers, dry_run, **kwargs)

if self.config.raise_on_error:
result.raise_on_error()
return result


def run_task(host, brigade, task):
def run_task(host, brigade, dry_run, task):
try:
logger.debug("{}: running task {}".format(host.name, task))
r = task._start(host=host, brigade=brigade, dry_run=brigade.dry_run)
r = task._start(host=host, brigade=brigade, dry_run=dry_run)
return host.name, r, None, None
except Exception as e:
logger.error("{}: {}".format(host, e))
Expand Down
2 changes: 1 addition & 1 deletion brigade/core/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def __repr__(self):
def _start(self, host, brigade, dry_run):
self.host = host
self.brigade = brigade
self.dry_run = dry_run
self.dry_run = dry_run if dry_run is not None else brigade.dry_run
return self.task(self, **self.params)

def run(self, task, **kwargs):
Expand Down
2 changes: 2 additions & 0 deletions brigade/plugins/tasks/files/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from .sftp import sftp
from .write import write


__all__ = (
"sftp",
"write",
)
48 changes: 48 additions & 0 deletions brigade/plugins/tasks/files/write.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import difflib
import os

from brigade.core.task import Result


def read_file(file):
if not os.path.exists(file):
return []
with open(file, "r") as f:
return f.read().splitlines()


def generate_diff(filename, content, append):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should read_file and generate_file be prefixed with a _?

If we decide to allow multiple tasks within the same file we might want to collect all of the tasks dynamically, then we could just exclude _read_file and _generate_diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, will mark them as private.

original = read_file(filename)
if append:
c = list(original)
c.extend(content.splitlines())
content = c
else:
content = content.splitlines()

diff = difflib.unified_diff(original, content, fromfile=filename, tofile="new")

return "\n".join(diff)


def write(task, filename, content, append=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that write sounds very generic. Could it be write_file instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Changing to write_file

"""
Write contents to a file (locally)

Arguments:
filename (``str``): file you want to write into
conteint (``str``): content you want to write
append (``bool``): whether you want to replace the contents or append to it

Returns:
* changed (``bool``):
* diff (``str``): unified diff
"""
diff = generate_diff(filename, content, append)

if not task.dry_run:
mode = "a+" if append else "w+"
with open(filename, mode=mode) as f:
f.write(content)

return Result(host=task.host, diff=diff, changed=bool(diff))
Binary file modified tests/inventory_data/nsot/nsot.sqlite3
Binary file not shown.
24 changes: 12 additions & 12 deletions tests/plugins/tasks/files/test_sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
class Test(object):

def test_sftp_put(self, brigade):
brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="put",
src="README.md",
dst="/tmp/README.md")
Expand All @@ -19,8 +19,8 @@ def test_sftp_put(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = False
result = brigade.run(files.sftp,
dry_run=False,
action="put",
src="README.md",
dst="/tmp/README.md")
Expand All @@ -29,8 +29,8 @@ def test_sftp_put(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="put",
src="README.md",
dst="/tmp/README.md")
Expand All @@ -41,8 +41,8 @@ def test_sftp_put(self, brigade):

def test_sftp_get(self, brigade):
filename = "/tmp/" + str(uuid.uuid4()) + "-{host}"
brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="get",
src="/etc/hostname",
dst=filename)
Expand All @@ -51,8 +51,8 @@ def test_sftp_get(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = False
result = brigade.run(files.sftp,
dry_run=False,
action="get",
src="/etc/hostname",
dst=filename)
Expand All @@ -61,8 +61,8 @@ def test_sftp_get(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=False,
action="get",
src="/etc/hostname",
dst=filename)
Expand All @@ -72,8 +72,8 @@ def test_sftp_get(self, brigade):
assert not r.changed

def test_sftp_put_directory(self, brigade):
brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="put",
src="./brigade",
dst="/tmp/asd")
Expand All @@ -82,8 +82,8 @@ def test_sftp_put_directory(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = False
result = brigade.run(files.sftp,
dry_run=False,
action="put",
src="./brigade",
dst="/tmp/asd")
Expand All @@ -92,8 +92,8 @@ def test_sftp_put_directory(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="put",
src="./brigade",
dst="/tmp/asd")
Expand All @@ -104,8 +104,8 @@ def test_sftp_put_directory(self, brigade):

def test_sftp_get_directory(self, brigade):
filename = "/tmp/" + str(uuid.uuid4()) + "-{host}"
brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="get",
src="/etc/terminfo/",
dst=filename)
Expand All @@ -114,8 +114,8 @@ def test_sftp_get_directory(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = False
result = brigade.run(files.sftp,
dry_run=False,
action="get",
src="/etc/terminfo/",
dst=filename)
Expand All @@ -124,8 +124,8 @@ def test_sftp_get_directory(self, brigade):
for h, r in result.items():
assert r.changed, r.files_changed

brigade.dry_run = True
result = brigade.run(files.sftp,
dry_run=True,
action="get",
src="/etc/terminfo/",
dst=filename)
Expand Down
Loading