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

Add script which compares two minions #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bookwar
Copy link
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

@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.

Fore = Back = Style = ColorFallback()

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

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 """ ... """


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

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(
[
Copy link

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


Copy link

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


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

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

indent=4
)

def minions_diff(minionA, minionB):
Copy link

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

"\n".join( [key.upper(), "\n".join(value)] ) for key, value in diffs.items()
]
)
return result
Copy link

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(
Copy link

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

)

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

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.


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

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

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