Add additional python scripts to provide mapping validation and automatic scripts for generate_mapping failover and failback #38

maorlipchuk · 2018-03-06T10:59:41Z

Introduce the ovirt-dr script to provide the user a more convinient way to run
disaster recovery actions as a way to avoid using ansible playbooks directly.

ovirt-dr should support several actions which the user can execute:

validate validate the var file mapping which is used for failover and failback
generate Generate the mapping var file based on the primary and secondary setup, to be used for failover and failback
failover Start a failover process to the target setup
failback Start a failback process from the target setup to the source setup

Each of those actions is using a configuration file which its default location is files/dr.conf

Here are few examples how the script can be run:
For mapping file generation:
./ovirt-dr generate
For mapping file validation:
./ovirt-dr validate
For fail-over operation:
./ovirt-dr failover
For fail-back operation:
./ovirt-dr failback

didib · 2018-03-07T08:16:57Z

I was asked to review this PR, but sorry, I can't understand its purpose, how it should be used, etc.

didib

Already discussed in private, some comments inline. Didn't review the indentation/style, not sure what this project uses.

didib · 2018-03-07T07:53:16Z

files/dr.conf

@@ -0,0 +1,6 @@
+[generate_vars]
+site=http://10.35.0.140:8080/ovirt-engine/api


We should probably have two files:

Default - perhaps something like /usr/share/ovirt-ansible-disaster-recovery/dr-defaults.conf

User-controlled - perhaps /etc/ovirt-ansible-disaster-recovery/dr.conf

didib · 2018-03-07T07:54:35Z

files/dr.conf

+[generate_vars]
+site=http://10.35.0.140:8080/ovirt-engine/api
+username=admin@internal
+#password=


The conf file should be protected, as it might contain a password. So make e.g. the directory holding it 0750, owned by root:ovirt, or something like that. Perhaps your own group.

didib · 2018-03-07T10:30:11Z

files/dr.conf

+site=http://10.35.0.140:8080/ovirt-engine/api
+username=admin@internal
+#password=
+ca=/tmp/ca.pem


Can have default point at engine ca, /etc/pki/ovirt-engine/ca.pem

didib · 2018-03-07T10:31:55Z

files/dr.conf

+username=admin@internal
+#password=
+ca=/tmp/ca.pem
+output_file=mapping_vars.yml


Should probably default to /var/lib/ovirt-ansible-disaster-recovery/mapping.vars.yml, especially if you intend to mainly/only allow editing it using a tool/gui and not manually.

Should probably either refuse changing it if exists, or backup before changing.

didib · 2018-03-07T10:32:59Z

files/generate_vars.py

@@ -0,0 +1,115 @@
+#!/usr/bin/python
+import configparser


Does this work in both python2 and 3? 2 has ConfigParser. You can do e.g.:

try:
import configparser
except ImportError:
import ConfigParser as configparser

I will fix that but
Besides that, we have many issues regarding to python compatibility version (For example raw_input vs input)
I've discussed this with Ondra and currently we will support only python 2 and when we port all ansble cpde to python 3 we will support this python script as well.

didib · 2018-03-07T10:34:16Z

files/generate_vars.py

+        site, username, password, ca, output_file = self._init_vars()
+        print("%ssite address: %s \n"
+              "%susername: %s \n"
+              "%spassword: %s \n"


Please don't output the password. You can output "password: " if you want.

didib · 2018-03-07T10:35:45Z

files/generate_vars.py

+                 self.prefix,
+                 output_file))
+
+        ansible_play = "../examples/dr_generate_var.yml"


Will this link work also from rpm install? Where do you put the examples then? Currently in doc. If you actually use them, perhaps move to /usr/share/ovirt-ansible-disaster-recovery/playbooks or something.

So what you are saying is that I will need to add a new folder containing this playbook? which will be located in playbooks?

didib · 2018-03-07T10:37:04Z

files/generate_vars.py

+                        .format(site, username, password, ca, output_file)
+        command = "ansible-playbook {} -t {} -e '{}'" \
+                  .format(ansible_play, dr_tag, external_vars)
+        process = subprocess.Popen('/bin/bash', stdin=subprocess.PIPE,


Perhaps better to run the command directly and not bash.

didib · 2018-03-07T10:38:45Z

files/generate_vars.py

+        site, username, password, ca, output_file = '', '', '', '', ''
+        settings = configparser.ConfigParser()
+        settings._interpolation = configparser.ExtendedInterpolation()
+        settings.read('dr.conf')


Perhaps don't hard-code conf file but make it parametric? Also, as commented above, read both defaults and custom.

didib · 2018-03-07T10:44:12Z

files/generate_vars.py

+        settings._interpolation = configparser.ExtendedInterpolation()
+        settings.read('dr.conf')
+        site = settings.get('generate_vars', 'site',
+                            vars=DefaultOption(settings,


I do not understand why you need DefaultOption. Isn't this enough: settings.get('generate_vars', 'site') ?

If you also supply a defaults file, your defaults should be there, not in the code.

machacekondra · 2018-03-28T14:11:54Z

files/dr.conf

+site=http://localhost:8080/ovirt-engine/api
+username=admin@internal
+password=12
+ca=/etc/pki/ovirt-engine/ca.pem


I would use ca_file instead of ca

machacekondra · 2018-03-28T14:19:34Z

files/generate-vars

+
+class GenerateMappingFile():
+    prefix = "[Generate Mapping File] "
+    ansible_play = "../examples/dr_generate_var.yml"


This won't work in RPM, becuase in RPM the examples are in:

/usr/share/doc/ovirt-ansible-disaster-recover/examples/..

But the path of the role is:

/usr/share/ansible/roles/oVirt.disaster.recovery/tasks/main.yml

Changed to be compatible to the role

machacekondra · 2018-03-28T14:22:08Z

files/generate-vars

+                                                      'generate_vars',
+                                                      output_file=None))
+        if (not site):
+            site = raw_input(self.prefix + "SITE address is not "


I would also fallback to environment variables variables.

But as future feature not a must.

machacekondra · 2018-03-28T14:22:47Z

files/generate-vars

+        site, username, password, ca, output_file = self._init_vars()
+        print("\n%ssite address: %s \n"
+              "%susername: %s \n"
+              "%spassword: \n"


Maybe: ****** instead of empty would be nicer..

indeed, done

machacekondra · 2018-03-28T14:24:20Z

files/generate-vars

+                 self.prefix,
+                 output_file))
+
+        external_vars = "site={} username={} password={} ca={} var_file={}" \


password will be seen on command line, maybe you can create temporary file with extra variables

Sure, although I don't think it is related to this log.
In this log I will simply write '******'

machacekondra · 2018-03-28T14:25:33Z

files/generate-vars

+        call(command, shell=True)
+        print("\n%sFinished generating variable mapping file "
+              "for oVirt ansible disaster recovery" % self.prefix)
+        print("\n%sVar file is: '%s'" % (self.prefix, output_file))


Don't this contain any sensitive information?

It contains all the members in the setup.
Basically the person which run this script should also have privilladges to view this file.
I guess that we better be safe then sorry so I will drop this print.

Now I noticed that I only mention the location of the mapping file and not its content, so I think that this is relevant enough for the user.
I will change the variable name from output_file to var_file_location to make it more clear

machacekondra · 2018-03-28T14:25:59Z

files/generate-vars

+            ans = raw_input(self.prefix + "The output file '" + fname +
+                            "' already exists, "
+                            "would you like to override it (y,n)? ")
+            ans.lower


ans.lower()

and move it under while True

done, thanks

machacekondra · 2018-04-05T15:17:14Z

files/dr.conf

@@ -0,0 +1,10 @@
+[generate_vars]
+site=http://localhost:8080/ovirt-engine/api


I would use engine.example.com as an example

machacekondra · 2018-04-05T15:17:35Z

files/dr.conf

+username=admin@internal
+password=12
+ca_file=/etc/pki/ovirt-engine/ca.pem
+output_file=/var/lib/ovirt-ansible-disaster-recovery/mapping_vars.yml


does this directory exists?

As part of the generate script, I add this folder if it does not exists.
See in generate_vars.py file at GenerateMappingFile_validate_output_file_exists

machacekondra · 2018-04-05T15:18:47Z

files/validator.py

+                                 self.secondary_ca)
+
+    def _connect_sdk(self, url, username, password, ca):
+        connection = sdk.Connection(


You should somewhere close the connection.

machacekondra

Can you please add somewhere to documentation how the user should use this?

maorlipchuk · 2018-04-09T13:48:40Z

I will add a new python script called ovirt_dr
Through this script I will provide a way to initiate the generate_mapping, validator, fail_over and fail_back.
I will add the proper documentation for it.

machacekondra · 2018-04-11T14:58:46Z

README.md

+  failover	Start a failover process to the target setup
+  failback	Start a failback process from the target setup to the source setup
+
+Each of those actions is using a configuration file which its default location is files/dr.conf


files/dr.conf .. please put whole path.

machacekondra · 2018-04-11T14:59:14Z

README.md

@@ -49,6 +49,29 @@ Example Playbook
 Generate var file mapping [demo](https://youtu.be/s1-Hq_Mk1w8)<br/>
 Fail over scenario [demo](https://youtu.be/mEOgH-Tk09c)

+Scripts
+-------
+The ovirt-dr script should provide the user a more convinient way to run


ovirt-dr where is it installed in /usr/bin?

It should be somewhere there..

I'm not sure how to do that.
I prefer to do that on another patch so at least we will have this.
Meanwhile I've added a commit to this PR of spec file

Ok, np, just that user must enter full path, which is different in galaxy and in RPM installation.

Should I add this in the spec as well?

machacekondra · 2018-04-11T15:28:56Z

files/fail_over.py

+
+
+if __name__ == "__main__":
+    FailOver().run('dr.conf', '/var/log/ovirt-dr/ovirt-dr.log')


/var/log/ovirt-dr/ is this directory created by RPM? I think it won't work from galaxy

no it is a directory which I'm creating.
How I should do it then?

In the build.sh create and then in spec file define. Check ovirt-engine.spec.in for example.

I've found ovirt-engine.spec.in and add the log as mention inthere.
It can be reviewed in the new commit

mykaul · 2018-04-30T09:08:07Z

Any updates here?

Also adding log

Rename from dr_generate_var.yml to dr_play.yml so it can be used for failover and failback

Add configuration values for failover and failback Add scripts for failover and failback

Through ovirt-dr the user should be able to execute generate mapping, validate mapping failover and failback

maorlipchuk mentioned this pull request Mar 6, 2018

Separate roles for preparation, failover and failback #17

Closed

maorlipchuk force-pushed the issue17_fix branch from ef492d2 to ac9c0b7 Compare March 7, 2018 07:34

didib reviewed Mar 7, 2018

View reviewed changes

maorlipchuk force-pushed the issue17_fix branch 3 times, most recently from 80d3543 to dc50bd3 Compare March 15, 2018 11:55

maorlipchuk force-pushed the issue17_fix branch from dc50bd3 to 87d7573 Compare March 21, 2018 12:30

maorlipchuk changed the title ~~[WIP] Add python scripts for generate_mapping failover and failback~~ Add python scripts for generate_mapping failover and failback Mar 21, 2018

maorlipchuk changed the title ~~Add python scripts for generate_mapping failover and failback~~ Add additional python scripts to privide mapping validation and automatic scripts for generate_mapping failover and failback Mar 21, 2018

maorlipchuk changed the title ~~Add additional python scripts to privide mapping validation and automatic scripts for generate_mapping failover and failback~~ Add additional python scripts to provide mapping validation and automatic scripts for generate_mapping failover and failback Mar 21, 2018

machacekondra reviewed Mar 28, 2018

View reviewed changes

maorlipchuk force-pushed the issue17_fix branch from 87d7573 to 527649d Compare April 3, 2018 00:08

machacekondra reviewed Apr 5, 2018

View reviewed changes

maorlipchuk force-pushed the issue17_fix branch 6 times, most recently from 6c32613 to 21a4f05 Compare April 11, 2018 03:44

maorlipchuk force-pushed the issue17_fix branch 5 times, most recently from 6955d66 to 61e79f2 Compare April 11, 2018 14:19

machacekondra reviewed Apr 11, 2018

View reviewed changes

maorlipchuk force-pushed the issue17_fix branch 2 times, most recently from 8b2861f to dbfc1f2 Compare April 15, 2018 09:44

maorlipchuk force-pushed the issue17_fix branch from dbfc1f2 to 64bef39 Compare April 24, 2018 13:13

Add bcolors class for better logging

be0d89c

maorlipchuk force-pushed the issue17_fix branch from 64bef39 to e0da097 Compare May 2, 2018 00:36

Maor Lipchuk added 8 commits May 2, 2018 03:38

Add python scripts for generate_mapping

9f26294

Add python validator script for mapping file

81f89d5

Add validation using SDK for existing entites

00edcb0

Also adding log

Rename play to a general name

d01c6a5

Rename from dr_generate_var.yml to dr_play.yml so it can be used for failover and failback

Support failover and failback scripts

752538a

Add configuration values for failover and failback Add scripts for failover and failback

Introduce ovirt-dr

976b72b

Through ovirt-dr the user should be able to execute generate mapping, validate mapping failover and failback

Add documentation for scripts

47c8008

move generate_mapping file from tasks to files

2ff00d5

maorlipchuk force-pushed the issue17_fix branch from e0da097 to 2ff00d5 Compare May 2, 2018 00:38

maorlipchuk merged commit 402c88d into master May 2, 2018

sandrobonazzola deleted the issue17_fix branch February 5, 2020 07:36

		@@ -0,0 +1,6 @@
		[generate_vars]
		site=http://10.35.0.140:8080/ovirt-engine/api

		@@ -0,0 +1,10 @@
		[generate_vars]
		site=http://localhost:8080/ovirt-engine/api



		if __name__ == "__main__":
		FailOver().run('dr.conf', '/var/log/ovirt-dr/ovirt-dr.log')

Add additional python scripts to provide mapping validation and automatic scripts for generate_mapping failover and failback #38

Add additional python scripts to provide mapping validation and automatic scripts for generate_mapping failover and failback #38

Conversation

maorlipchuk commented Mar 6, 2018 • edited

didib commented Mar 7, 2018

didib left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maorlipchuk Apr 9, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machacekondra left a comment

Choose a reason for hiding this comment

maorlipchuk commented Apr 9, 2018

machacekondra Apr 11, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machacekondra Apr 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mykaul commented Apr 30, 2018

maorlipchuk commented Mar 6, 2018 •

edited

maorlipchuk Apr 9, 2018 •

edited

machacekondra Apr 11, 2018 •

edited

machacekondra Apr 12, 2018 •

edited