Skip to content

Add script which compares two minions#1

Open
bookwar wants to merge 1 commit into
masterfrom
compare-minions
Open

Add script which compares two minions#1
bookwar wants to merge 1 commit into
masterfrom
compare-minions

Conversation

@bookwar
Copy link
Copy Markdown
Contributor

@bookwar bookwar commented Jun 22, 2017

Script can be used to compare pillars, highstates and grains for two minions,
or for two different snapshots of the salt states code.

To use it for pull request verification one can:

  1. checkout destination branch to dirA,
  2. checkout pull-request branch to dirB,
  3. setup minion config /opt/minionA/minion to use states and pillars from dirA
  4. setup minion config in /opt/minionB/minion to use states and pillars from dirB
  5. put the same grains file into /opt/minionA/grains and /opt/minionB/grains
  6. run the script

This way you'll get the difference given by the salt states code changes applied to the same minion (i.e. grains input)

Copy link
Copy Markdown

@akaRem akaRem left a comment

Choose a reason for hiding this comment

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

Please check your code with pylint and add some more annotations. In general it should work. But output is surprise for everyone. If you are going to use it as executable, then it's a very good idea to add simple CLI wrapper with help and brief description.

Comment thread utils/compare_minions.py Outdated
Fore = Back = Style = ColorFallback()

def color_diff(diff):
'''Colorize lines in the diff generator object'''
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PEP257 docstring should be enclosed in """ ... """

Comment thread utils/compare_minions.py Outdated

diffs = {}
for key in comparable_properties:
diffs[key] = color_diff (
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

style: extra space betw. func name and parens

Comment thread utils/compare_minions.py
)

result = "\n".join(
[
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you may use ( .. ) instead of [ .. ] here

Comment thread utils/compare_minions.py
)
return result


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if __name__ == '__main__': ... otherwise this module will execute following lines on import

Comment thread utils/compare_minions.py Outdated

@property
def pillar_items(self):
return json.loads(json.dumps(self.salt['pillar.items'](saltenv='javaci')))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why to dump and load?? If this is some kind of workaround, please add annotations

Comment thread utils/compare_minions.py Outdated
indent=4
)

def minions_diff(minionA, minionB):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be more reusable in form of class

Comment thread utils/compare_minions.py
"\n".join( [key.upper(), "\n".join(value)] ) for key, value in diffs.items()
]
)
return result
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

result lives for 2 lines. You can throw it away

Comment thread utils/compare_minions.py
)
)

result = "\n".join(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please add output example as this templating is very complex and hard to trace

Comment thread utils/compare_minions.py Outdated
)

def minions_diff(minionA, minionB):
'''Generate multiline string from colorized diffs'''
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actually not. this generates multiline colorized diff from 2 minions. And you don't have any functionality for jyst getting colorized or non-colorized diff.

Comment thread utils/compare_minions.py Outdated

default_config = '/opt/{name}/minion'

def __init__(self, name=None, config=None):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

config is not used and could be removed from params, so name is one and only required param (as far as I see from this script)

Script can be used to compare pillars, highstates and grains for two minions,
or for two different snapshots of the salt states code.

To use it for pull request verification one can:

1) checkout destination branch to dirA,
2) checkout pull-request branch to dirB,
3) setup minion config /opt/minionA/minion to use states and pillars from dirA
4) setup minion config in /opt/minionB/minion to use states and pillars from dirB
5) put the same grains file into /opt/minionA/grains and /opt/minionB/grains
6) run the script

This way you'll get the difference given by the salt states code changes applied to the same minion (i.e. grains input)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants