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

Disaster Recovery fixes after added to collections #160

Merged
merged 10 commits into from
Nov 2, 2020

Conversation

barpavel
Copy link
Member

@barpavel barpavel commented Oct 26, 2020

The DR project was added to the "ovirt-ansible-collection" repository and merged in
#134
This PR fixes a few small issues with it.

Signed-off-by: Pavel Bar [email protected]

- The "Example Script" paths were not 100% correct:
"files" sub-folder was missing.
- Also the scripts should be executed from the scripts directory.
The issues is with relative paths.
Probably can be fixed, but better don't touch right now.

Signed-off-by: Pavel Bar <[email protected]>
@barpavel barpavel marked this pull request as draft October 26, 2020 10:37
@barpavel
Copy link
Member Author

Also for @shenitzky

Updating the default paths to be consistent
with other paths in the same files:
  - "dr.conf" configuration file.
  - "validator.py" hardcoded path to the yaml file.

Signed-off-by: Pavel Bar <[email protected]>
This is also a preparation step for the following patches.

Signed-off-by: Pavel Bar <[email protected]>
@barpavel barpavel force-pushed the dr_file_paths_fix branch 2 times, most recently from d16c14f to e2a5eeb Compare October 27, 2020 15:52
Some user messages (asking to enter missing data) were either:
  - Incomplete
  - Confusing
  - Contained copy-paste errors

This is also a preparation step for the following patches.

Signed-off-by: Pavel Bar <[email protected]>
- When the Ansible play file taken from the configuration file
was missing, and user just pressed ENTER thus choosing
to use the suggested default location for the Ansible play file,
an empty string was used instead.

Signed-off-by: Pavel Bar <[email protected]>
- Unite all the validator configuration parameters'
initializations under the same designated method.
- Remove duplicate code related to the var file initialization.
Originally there were 2 sections that tried to initialize
the same var file one after another.

Signed-off-by: Pavel Bar <[email protected]>
@barpavel barpavel force-pushed the dr_file_paths_fix branch 3 times, most recently from 6cf1c8f to e5573d3 Compare October 28, 2020 08:49
There was no "~" support for files received from the user
either via the configuration file "./files/dr.conf" or
via input from the CLI.
The problem existed previously, but became more apparent
due to moving to collections and execution from
"~/.ansible/collections/...." directory.

This commit fixes all the 4 flows:
a) generate
b) validate
c) failover
d) failback

Signed-off-by: Pavel Bar <[email protected]>
"vault" configuration parameter is not used during the validation phase.
Removing it from both "files/dr.conf" & "validator.py".

Signed-off-by: Pavel Bar <[email protected]>
@barpavel barpavel marked this pull request as ready for review October 28, 2020 13:34
@barpavel
Copy link
Member Author

Verified today, October 28th.

The "failover" & "failback" flows were missing a default var file
parameter, while there was a default playbook parameter in both flows
and a default var file parameter in the validation flow.
Adding the missing default for consistency and a better user experience.

Signed-off-by: Pavel Bar <[email protected]>
@mnecas
Copy link
Member

mnecas commented Oct 29, 2020

Please add the fragment.

@barpavel
Copy link
Member Author

Please add the fragment.

Done.

Copy link
Member

@mnecas mnecas left a comment

Choose a reason for hiding this comment

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

LGTM

@mwperina mwperina merged commit ceeb534 into oVirt:master Nov 2, 2020
@barpavel barpavel deleted the dr_file_paths_fix branch November 18, 2020 14:35
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.

4 participants