Support log for ansible-disaster-recovery #43

maorlipchuk · 2018-04-24T13:12:27Z

Changes in vault and Configparse usages to avoid any missing dependecies in RHEL installation.
dr-play.yml will be copied to "files" folder to be used for the ovirt-dr script.
Introduce the use of log file in ovirt-dr script.
The script default location and level is configured in the dr.conf
Default location is /var/log/ovirt-dr/ovirt-dr.log and default level is "DEBUG"
The user can override it with --log-file and --log-level
If no log file will be specified stdout will be used to print the output.

machacekondra · 2018-04-25T12:03:24Z

Why not do it inside build.sh script? I've created it so it's simpler then using Makefile.

machacekondra · 2018-05-02T08:42:21Z

build.sh

@@ -6,12 +6,14 @@ RPM_RELEASE="0.1.$MILESTONE.$(date -u +%Y%m%d%H%M%S)"

 ROLE_NAME="oVirt.disaster-recovery"
 PACKAGE_NAME="ovirt-ansible-disaster-recovery"
-PREFIX=/usr/local
+LOG_NAME="ovirt-dr"
+PPREFIX=/usr/local


Why it's renamed to PPREFIX? I think it's typo, no?

ohh damn, you are right, will fix

machacekondra · 2018-05-02T08:44:35Z

build.sh


  cp -pR defaults/ $PKG_DATA_DIR
  cp -pR library/ $PKG_DATA_DIR
  cp -pR meta/ $PKG_DATA_DIR
  cp -pR tasks/ $PKG_DATA_DIR
+  cp -pR examples/ $PKG_DATA_DIR


We store examples in /usr/share/doc for other roles, jut FYI, if you want to have it also in /usr/share/ you can, up to you.

I would want to use the ansible play there in the script, that is why I'm copying it.
I will try to copy only the play without the entire example folder.
Let me know if you have a better idea

machacekondra

I think the DR RPM should create directory /var/log/ovirt-dr/. Like follows:

install -d /var/log/ovirt-dr -m 755

and in spec file:

%dir /var/log/ovirt-dr

Then the DR script should always create file disaster-recovery-{timestamp} in /var/log/ovirt-dr directory. Maybe configure logrotate for it? Depends how often it will be used. Or if you think it's better to use single log file and append to it, I am OK with it, but be sure it's simple for users to understand the log, where which part starts/ends.

If you follow your current way don't forget to add %ghost %log_file_path to spec file, and verify it's OK with rpm --verify command.

machacekondra · 2018-05-02T11:47:02Z

@mwperina

mwperina · 2018-05-02T18:48:20Z

build.sh

@@ -6,12 +6,14 @@ RPM_RELEASE="0.1.$MILESTONE.$(date -u +%Y%m%d%H%M%S)"

 ROLE_NAME="oVirt.disaster-recovery"
 PACKAGE_NAME="ovirt-ansible-disaster-recovery"
+LOG_NAME="ovirt-dr"


If you are going to put logs into /var/log/ovirt-dr then please provide logrotate setup similarly as we do in engine:

https://github.com/oVirt/ovirt-engine/blob/master/packaging/sys-etc/logrotate.d/ovirt-engine.in

I've added the file,
please let me know if there is anything more that should be addressed.

You also need to add dependency to logrotate, replace relative paths inside it (please take a look at GENERATED list and its usage in engine Makefile [1]) and install it into /etc/logrotate.d [2].

[1] https://github.com/oVirt/ovirt-engine/blob/master/Makefile#L211
[2] https://github.com/oVirt/ovirt-engine/blob/master/ovirt-engine.spec.in#L990

maorlipchuk · 2018-05-02T19:04:25Z

Why do we need to add %dir /var/log/ovirt-dr in the spec file?

If I add it the build fails, since it refers '/var/log/ovirt-dr' as a relative path, see below:
18:59:53 [check-patch.el7.x86_64] error: Directory not found: /home/jenkins/workspace/oVirt_ovirt-ansible-disaster-recovery_standard-check-pr/ovirt-ansible-disaster-recovery/rpmbuild/BUILDROOT/ovirt-ansible-disaster-recovery-0.4-0.1.master.20180502185951.el7.centos.x86_64/var/log/ovirt-dr

mwperina · 2018-05-03T06:34:15Z

logrotate.d/ovirt-dr.in

@@ -0,0 +1,8 @@
+"@PKG_LOG_DIR/LOG_NAME@/ovirt-dr.log" {


Where are you replacing @PKG_LOG_DIR/LOG_NAME@ with /var/log/ovirt-dr? Also why is there only single log file? Where are you going to log multiple concurrent invocations of a role from single machine?

That is what I've defined in the spec, I will try to do that with '/var/log/ovirt-dr'
I've tried to follow how ovirt-engine works, should I do it like this:
"/var/log/ovirt-dr"/*.log

Is it ok if the file is located here: logrotate.d/ovirt-dr.in

mwperina · 2018-05-03T06:35:23Z

ovirt-ansible-disaster-recovery.spec.in

@@ -26,10 +26,12 @@ This Ansible role provide funtionality to perform disaster recovery for oVirt en
 export PKG_DATA_DIR_ORIG=%{_datadir}/%{ansible_roles_dir}/%{roleprefix}%{rolename}
 export PKG_DATA_DIR=%{buildroot}$PKG_DATA_DIR_ORIG
 export PKG_DOC_DIR=%{buildroot}%{_pkgdocdir}
+install -dm 755 "$PKG_LOG_DIR/$LOG_NAME/$LOG_NAME.log"


Please add Requires: logrotate spec file

Should I add it like this
Requires: logrotate.d/ovirt-dr.in
or is that enough:
Requires: logrotate

You need to use Requires: logrotate, logrotate is a package name

machacekondra

Thinking of it, I think the best approach for DR would be to have --log-file and --log-level command line options, instead of persistent log directory maybe it will be even more simple. But I will leave this decision on you.

maorlipchuk · 2018-05-10T12:48:54Z

Changed the log to be compatible with your suggestion:
/tmp/ovirt-dr-{timestamp}.log

I haven't understood how would you want me to use the callback plugins.
I've tried to change the ansible.cfg to work with the following configuration (see [1]), but stdout has not been changed.

[1]
# Use the YAML callback plugin.
stdout_callback = yaml
# Use the stdout_callback when running ad-hoc commands.
bin_ansible_callbacks = True

machacekondra · 2018-05-15T14:09:53Z

build.sh

@@ -44,6 +44,7 @@ install() {
  cp -pR library/ $PKG_DATA_DIR
  cp -pR meta/ $PKG_DATA_DIR
  cp -pR tasks/ $PKG_DATA_DIR
+  cp examples/dr_play.yml $PKG_DATA_DIR


We have this in doc directory already, isn't it enought? Or why is it needed also in usr/share?

I wanted the user to be able to also change this.
It is good that this will be in the doc directory so the user will be able to see that as an example.
For the user to use that as part of the scripts I thought it will be more appropriate to be copied to the DATA_DIR directory so the user will be able to manipulate it.
If you think it is redundant I can remove it

It's in /usr/share so his changes will be overriten by update anyway. So the correct use case is to copy the file from doc to some of his directory or /etc...

I think that to keep it simple we can also use the dr_play from the examples.
So I will just change the dr.conf to point to the example dr_play which is in the doc directory

machacekondra · 2018-05-15T14:10:20Z

examples/dr_play.yml

@@ -3,4 +3,4 @@
  hosts: localhost
  connection: local
  roles:
-     - oVirt.disaster-recovery
+      - ovirt-ansible-disaster-recovery


Please use oVirt.disaster-recovery

machacekondra · 2018-05-15T14:10:46Z

files/dr.conf

 [generate_vars]
-site=http://engine.example.com/ovirt-engine/api
+site=http://localhost:8080/ovirt-engine/api


please use engine.example.com

machacekondra · 2018-05-15T14:17:12Z

files/fail_over.py

-    FailOver().run('dr.conf', '/var/log/ovirt-dr/ovirt-dr.log')
+    level = logging.getLevelName("DEBUG")
+    conf = 'dr.conf'
+    log = '/var/log/ovirt-dr/ovirt-dr.log'


This directory don't exist so it should be here.

This will only be used for testing or if the user will try to run that script directly instead through the ovirt-dr script (which initialize the log file)
I can change this to be /tmp if you think it is neccessery.

It's not, but anyone other testing it, will fail, because he won't have that directory.

The code in fail-over should create that directory if it doesn't exists.
but to keep it simple I will change it to /tmp

mwperina

Please rebase before merging

Change dr-play location in dr.conf and change vault location

Using attribute 'ExtendedInterpolation' is not compatible for configparser. If it is being used from RHEL 7.4 the following exception is being thrown: Traceback (most recent call last): File "./ovirt-dr", line 145, in <module> main(sys.argv[1:]) File "./ovirt-dr", line 28, in main action, conf_file, log_file, log_level = _init_vars(argv) File "./ovirt-dr", line 88, in _init_vars log_file, log_level = _get_log_conf(conf_file, log_file, log_level) File "./ovirt-dr", line 101, in _get_log_conf settings._interpolation = configparser.ExtendedInterpolation() AttributeError: 'module' object has no attribute 'ExtendedInterpolation' Therefore to make the code compatible for RHEL as well, instead of using configparser, we use SafeConfigParser from ConfigParser.

Configure the report file name to be passed as an argument when running the ovirt_dr script on failover and failback.

maorlipchuk requested a review from machacekondra April 24, 2018 13:12

maorlipchuk force-pushed the makefile_additions branch from c0de7a2 to 9ddb99a Compare April 24, 2018 13:58

maorlipchuk force-pushed the makefile_additions branch 2 times, most recently from 0c72857 to 13591f1 Compare May 2, 2018 02:16

maorlipchuk changed the title ~~Support log file for ansible-disaster-recovery in Makefile~~ Support scripts and log file for ansible-disaster-recovery in build.sh May 2, 2018

machacekondra reviewed May 2, 2018

View reviewed changes

maorlipchuk force-pushed the makefile_additions branch 3 times, most recently from 34c8c50 to ff101d5 Compare May 2, 2018 09:53

machacekondra reviewed May 2, 2018

View reviewed changes

maorlipchuk force-pushed the makefile_additions branch 6 times, most recently from 50630b7 to 9af6fc3 Compare May 2, 2018 18:35

mwperina suggested changes May 2, 2018

View reviewed changes

maorlipchuk force-pushed the makefile_additions branch 2 times, most recently from 3e0b864 to 8ba4e48 Compare May 2, 2018 18:58

maorlipchuk force-pushed the makefile_additions branch 5 times, most recently from 6800434 to c0b737c Compare May 3, 2018 05:09

mwperina suggested changes May 3, 2018

View reviewed changes

machacekondra reviewed May 3, 2018

View reviewed changes

maorlipchuk force-pushed the makefile_additions branch from e28823e to 3ea00d7 Compare May 10, 2018 08:11

maorlipchuk force-pushed the makefile_additions branch from 712bb4f to 9b0f64b Compare May 14, 2018 12:38

machacekondra reviewed May 15, 2018

View reviewed changes

maorlipchuk force-pushed the makefile_additions branch 5 times, most recently from 9379082 to 317eb02 Compare May 17, 2018 00:43

machacekondra approved these changes May 17, 2018

View reviewed changes

mwperina approved these changes May 17, 2018

View reviewed changes

Maor Lipchuk added 15 commits May 17, 2018 11:03

initialize log file and log level in ovirt-dr

19c2098

Change default value in dr.conf

70eabeb

Change dr-play location in dr.conf and change vault location

Use logging in generate vars

1e77afb

Remove unused log file in validator

4e635e4

add logging support for fail over

26a5044

add logging support for fail back

010eff3

Add missing file of vault secret to be used for fail_back.py

a8801a9

Use vault password as input for fail_over

d31f07b

Fix redundant var in generate_vars

80e9efc

Print failure message for failover and failback

4147686

Configure report file name on failover and failback

5f43a01

Configure the report file name to be passed as an argument when running the ovirt_dr script on failover and failback.

Add stdout callback plugin to print appropriate output

9574887

Rename tasks of adding storage domains

12a68c2

Declare all facts in a unified way

6b27501

maorlipchuk force-pushed the makefile_additions branch from 317eb02 to 6b27501 Compare May 17, 2018 08:12

Remove redundant minus mark in include_tasks

4222c49

maorlipchuk merged commit d4b6fc3 into master May 17, 2018

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

Support log for ansible-disaster-recovery #43

Support log for ansible-disaster-recovery #43

maorlipchuk commented Apr 24, 2018 •

edited

machacekondra commented Apr 25, 2018

machacekondra May 2, 2018

maorlipchuk May 2, 2018

machacekondra May 2, 2018

maorlipchuk May 2, 2018 •

edited

machacekondra left a comment

machacekondra commented May 2, 2018

mwperina May 2, 2018

maorlipchuk May 2, 2018

mwperina May 2, 2018

maorlipchuk commented May 2, 2018

mwperina May 3, 2018

maorlipchuk May 3, 2018

maorlipchuk May 3, 2018

mwperina May 3, 2018

maorlipchuk May 3, 2018 •

edited

mwperina May 3, 2018

machacekondra left a comment

maorlipchuk commented May 10, 2018 •

edited

machacekondra May 15, 2018

maorlipchuk May 16, 2018

machacekondra May 16, 2018

maorlipchuk May 16, 2018

machacekondra May 15, 2018

machacekondra May 15, 2018

machacekondra May 15, 2018

maorlipchuk May 16, 2018

machacekondra May 16, 2018

maorlipchuk May 16, 2018

mwperina left a comment

Support log for ansible-disaster-recovery #43

Support log for ansible-disaster-recovery #43

Conversation

maorlipchuk commented Apr 24, 2018 • edited

machacekondra commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maorlipchuk May 2, 2018 • edited

Choose a reason for hiding this comment

machacekondra left a comment

Choose a reason for hiding this comment

machacekondra commented May 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maorlipchuk commented May 2, 2018

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 May 3, 2018 • edited

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 May 10, 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

Choose a reason for hiding this comment

mwperina left a comment

Choose a reason for hiding this comment

maorlipchuk commented Apr 24, 2018 •

edited

maorlipchuk May 2, 2018 •

edited

maorlipchuk May 3, 2018 •

edited

maorlipchuk commented May 10, 2018 •

edited