-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add script which compares two minions #1
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
utils/compare_minions.py
Outdated
Fore = Back = Style = ColorFallback() | ||
|
||
def color_diff(diff): | ||
'''Colorize lines in the diff generator object''' |
There was a problem hiding this comment.
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 """ ... """
utils/compare_minions.py
Outdated
|
||
diffs = {} | ||
for key in comparable_properties: | ||
diffs[key] = color_diff ( |
There was a problem hiding this comment.
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
) | ||
|
||
result = "\n".join( | ||
[ |
There was a problem hiding this comment.
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
) | ||
return result | ||
|
||
|
There was a problem hiding this comment.
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
utils/compare_minions.py
Outdated
|
||
@property | ||
def pillar_items(self): | ||
return json.loads(json.dumps(self.salt['pillar.items'](saltenv='javaci'))) |
There was a problem hiding this comment.
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
utils/compare_minions.py
Outdated
indent=4 | ||
) | ||
|
||
def minions_diff(minionA, minionB): |
There was a problem hiding this comment.
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
utils/compare_minions.py
Outdated
"\n".join( [key.upper(), "\n".join(value)] ) for key, value in diffs.items() | ||
] | ||
) | ||
return result |
There was a problem hiding this comment.
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
) | ||
) | ||
|
||
result = "\n".join( |
There was a problem hiding this comment.
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
utils/compare_minions.py
Outdated
) | ||
|
||
def minions_diff(minionA, minionB): | ||
'''Generate multiline string from colorized diffs''' |
There was a problem hiding this comment.
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.
utils/compare_minions.py
Outdated
|
||
default_config = '/opt/{name}/minion' | ||
|
||
def __init__(self, name=None, config=None): |
There was a problem hiding this comment.
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)
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:
This way you'll get the difference given by the salt states code changes applied to the same minion (i.e. grains input)