retr0h / ansible-logrotate

MIT License
8 stars 9 forks source link

need some fixes #12

Closed ozbillwang closed 8 years ago

ozbillwang commented 8 years ago

This ansible role makes the task easier to define different logrotate configuration for different applications. The idea to make it run as ansible module is better than the other hottest ansible logrotate galaxy role (https://github.com/nickhammond/ansible-logrotate)

But when I implement the vars file, I got two problems, in fact three. Here is my task file.

$ cat roles/common/tasks/logrotate.yml
- name: Define logrotate config
  logrotate: name=syslog path='/var/log/cron /var/log/maillog /var/log/messages /var/log/secure /var/log/spooler'
  args:
    options:
      - compress
      - copytruncate
      - missingok
      - sharedscripts
      - weekly
      - rotate 12
      - postrotate
      - '  /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true'
      - endscript
  tags:
    - logrotate

So I have to:

put path into one line: path='/var/log/cron /var/log/maillog /var/log/messages /var/log/secure /var/log/spooler'

can we have something as:

- name: Define logrotate config
  logrotate: name=syslog 
  path:
     - /var/log/cron 
     - /var/log/maillog 
     - /var/log/messages 
     - /var/log/secure 
     - /var/log/spooler
  args:
    ...

put spaces before exec script.

  - '  /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true'

the { is in new line, as below.

/var/log/cron /var/log/maillog /var/log/messages /var/log/secure /var/log/spooler
{
  compress
  copytruncate
  missingok
  sharedscripts
  weekly
  rotate 12
  postrotate
    /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true
  endscript
}

I know it should work, but will be better to update as

/var/log/cron /var/log/maillog /var/log/messages /var/log/secure /var/log/spooler {
  ...
}
retr0h commented 8 years ago
  • name: Define logrotate config logrotate: name=syslog path:
    • /var/log/cron
    • /var/log/maillog
    • /var/log/messages
    • /var/log/secure
    • /var/log/spooler' args: ...

Hey @SydOps, I implemented more or less what you wanted in #15 around passing a list to path. As to the syntax of the logrotate conf files, they are rendered on new lines. I'm also not inclined to change the placement of the brackets. However, the functionality you requested is now implemented. Let me know how this works out for you.

/var/log/log1.log
/var/log/log2.log
{
  daily
  missingok
  rotate 8
  compress
  delaycompress
  copytruncate
  notifempty
}
ozbillwang commented 8 years ago

Thanks @retr0h

I think the format is wrong. Can we still have all path on the same line?

$ cat /etc/logrotate.d/syslog
/var/log/cron
/var/log/maillog
/var/log/messages
/var/log/secure
/var/log/spooler
{
  compress
  copytruncate
  missingok
  sharedscripts
  weekly
  rotate 12
  postrotate
    /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true
  endscript
}
ozbillwang commented 8 years ago

Here is the final syslog logrotate I want to have:

/var/log/cron /var/log/maillog /var/log/messages /var/log/secure /var/log/spooler {
  compress
  copytruncate
  missingok
  sharedscripts
  weekly
  rotate 12
  postrotate
    /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true
  endscript
}
ozbillwang commented 8 years ago

See the sample of http://www.linuxcommand.org/man_pages/logrotate8.html with two log path :


       "/var/log/httpd/access.log" /var/log/httpd/error.log {
           rotate 5
           mail www@my.org
           size=100k
           sharedscripts
           postrotate
                                     /sbin/killall -HUP httpd
           endscript
       }
retr0h commented 8 years ago

I think the format is wrong. Can we still have all path on the same line?

Sure, I guess I don't care all that much. Implemented in #16.

ozbillwang commented 8 years ago

Perfect. I got all I need. 👍

We should release it now, create a release on it: v1.0

retr0h commented 8 years ago

We should release it now, create a release on it: v1.0

done

ozbillwang commented 8 years ago

My final playbook task yaml file for reference by others.

- name: Define logrotate config
  logrotate:
    name: syslog
    path:
       - /var/log/cron
       - /var/log/maillog
       - /var/log/messages
       - /var/log/secure
       - /var/log/spooler
  args:
    options:
      - compress
      - copytruncate
      - missingok
      - sharedscripts
      - weekly
      - rotate 12
      - postrotate
      - '  /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true'
      - endscript
  tags:
    - logrotate
ozbillwang commented 8 years ago

@retr0h

I try ansible-galaxy with the release 1.0 , get this error:

ERROR! Unexpected Exception: execv() arg 2 must contain only strings

can you change the release to v1.0? Seems this will fix this issue.

 # logrotate role
 - src: https://github.com/retr0h/ansible-logrotate
-  version: 65c8a4e35f7e59ff250c5cd5526ff752a2905cac
+  version: 1.0
   name: logrotate

Wait, seems it is fine with 1.0 , I see this sample from ansible website:

http://docs.ansible.com/ansible/galaxy.html

# from GitLab or other git-based scm
 - src: git@gitlab.company.com:mygroup/ansible-base.git
   scm: git
   version: 0.1.0

Looks like a bug in ansible galaxy command. Let me report to ansible

ozbillwang commented 8 years ago

reported the issue to https://github.com/ansible/ansible/issues/15962

ozbillwang commented 8 years ago

@retr0h

Seems it is something wrong in this repo setting and release.

Here is the error:

$ cat requirements.yml
- src: retr0h.logrotate
  version: 2.0
  name: logrotate

- downloading role 'logrotate', owned by retr0h
 [WARNING]: - logrotate was NOT installed successfully: - the specified version (2.0) of logrotate was not found in the list of available versions ([{u'name':
u'2.0', u'created': u'2016-05-23T20:51:04.489Z', u'url': u'', u'release_date': u'2016-05-23T10:51:31Z', u'modified': u'2016-05-23T20:51:04.877Z', u'related': {},
u'id': 14736, u'active': True, u'summary_fields': {u'role': {u'id': 1131, u'name': u'logrotate'}}}, {u'name': u'1.0', u'created': u'2016-05-23T07:02:56.805Z',
u'url': u'', u'release_date': u'2016-05-23T10:51:31Z', u'modified': u'2016-05-23T20:51:05.231Z', u'related': {}, u'id': 14719, u'active': True, u'summary_fields':
{u'role': {u'id': 1131, u'name': u'logrotate'}}}]).

ERROR! - you can use --ignore-errors to skip failed roles and finish processing the list.

and no problem with commits

$ cat requirements.yml
- src: https://github.com/retr0h/ansible-logrotate
  version: f9b87cef8934e5d02848b3b47839ef184aad8ac0
  name: logrotate

and I don't have issue to install other galaxy roles.

retr0h commented 8 years ago

Yeah, I'm not sure, I don't generally download from galaxy, I usually point to github in my requirement files. This following appears to work, but I know thats not the answer you were looking for.

[jodewey:~] 1 % ansible-galaxy install retr0h.logrotate
- downloading role 'logrotate', owned by retr0h
- downloading role from https://github.com/retr0h/ansible-logrotate/archive/2.0.tar.gz
- extracting retr0h.logrotate to /etc/ansible/roles/retr0h.logrotate