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

Logrotate_combo #584

Closed
wants to merge 16 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@anshprat
Contributor

anshprat commented May 5, 2015

Will be combining more of logrotate for various modules under this.

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 5, 2015

Contributor

please test this

Contributor

anshprat commented May 5, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 5, 2015

Contributor

please test this

Contributor

anshprat commented May 5, 2015

please test this

@bodepd

This comment has been minimized.

Show comment
Hide comment
@bodepd

bodepd May 5, 2015

Contributor

one comment here and I know that @sorenh doesn't always agree with me on this, but for such a simple change-set, it is not required to have so many commits. I like the idea of having as few commits as possible so as to keep history usable.

of coarse, the disadvantage to rebasing is that it's harder to understand how you are making changes once you start getting review. I think a reasonable compromise is to clean up history before you start soliciting feedback, then issue new comments in response to requests to change things.

Contributor

bodepd commented May 5, 2015

one comment here and I know that @sorenh doesn't always agree with me on this, but for such a simple change-set, it is not required to have so many commits. I like the idea of having as few commits as possible so as to keep history usable.

of coarse, the disadvantage to rebasing is that it's harder to understand how you are making changes once you start getting review. I think a reasonable compromise is to clean up history before you start soliciting feedback, then issue new comments in response to requests to change things.

Show outdated Hide outdated Puppetfile
@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 6, 2015

Contributor

please test this

Contributor

anshprat commented May 6, 2015

please test this

@sorenh

This comment has been minimized.

Show comment
Hide comment
@sorenh

sorenh May 6, 2015

Contributor

You changed the mode of keystone.pp to 775. Please change it back.

Contributor

sorenh commented May 6, 2015

You changed the mode of keystone.pp to 775. Please change it back.

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 6, 2015

Contributor

please test this

Contributor

anshprat commented May 6, 2015

please test this

@anshprat anshprat changed the title from Logrotate keystone to Logrotate_combo May 6, 2015

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 7, 2015

Contributor

please test this

Contributor

anshprat commented May 7, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 7, 2015

Contributor

please test this

Contributor

anshprat commented May 7, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 8, 2015

Contributor

please test this

Contributor

anshprat commented May 8, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 11, 2015

Contributor

please test this

Contributor

anshprat commented May 11, 2015

please test this

@@ -70,6 +70,16 @@
headers => $headers,
}
rjil::jiocloud::logrotate { 'glance-api':

This comment has been minimized.

@bodepd

bodepd May 11, 2015

Contributor

why are you wrapping some of the log rotate config in classes, and putting some straight into the service class? It seems a little inconsistent.

@bodepd

bodepd May 11, 2015

Contributor

why are you wrapping some of the log rotate config in classes, and putting some straight into the service class? It seems a little inconsistent.

This comment has been minimized.

@anshprat

anshprat May 11, 2015

Contributor

Yes, this was before our session about resource duplicate handling and my failures to get an array working with a define. Will see if can get it all wrapped up within the define itself.

@anshprat

anshprat May 11, 2015

Contributor

Yes, this was before our session about resource duplicate handling and my failures to get an array working with a define. Will see if can get it all wrapped up within the define itself.

This comment has been minimized.

@anshprat

anshprat May 11, 2015

Contributor

So I now have another problem:

$logsarray = ['test1',
'teste2',
'test3',
]
rjil::jiocloud::logrotate { $logsarray:
logfile => "/tmp/$name"
}

This creates correct service as $name but the logfile as /tmp/main for all the logs. Any ideas how to fix this? passing $logsarray to logfile causes string concatnation of the elements.

@anshprat

anshprat May 11, 2015

Contributor

So I now have another problem:

$logsarray = ['test1',
'teste2',
'test3',
]
rjil::jiocloud::logrotate { $logsarray:
logfile => "/tmp/$name"
}

This creates correct service as $name but the logfile as /tmp/main for all the logs. Any ideas how to fix this? passing $logsarray to logfile causes string concatnation of the elements.

This comment has been minimized.

@bodepd

bodepd May 11, 2015

Contributor

you are calling $name from the wrong scope.

@bodepd

bodepd May 11, 2015

Contributor

you are calling $name from the wrong scope.

This comment has been minimized.

@anshprat

anshprat May 11, 2015

Contributor

ah, good point. But then which variable can I use out here when calling the define with array?

@anshprat

anshprat May 11, 2015

Contributor

ah, good point. But then which variable can I use out here when calling the define with array?

This comment has been minimized.

@bodepd

bodepd May 11, 2015

Contributor

that is one thing that is really hard to do in Puppet pre 4 (without iteration). You can use $name inside the define, but not outside. You could pass in the directory if you want to assume that the filename defaults to $name.log

@bodepd

bodepd May 11, 2015

Contributor

that is one thing that is really hard to do in Puppet pre 4 (without iteration). You can use $name inside the define, but not outside. You could pass in the directory if you want to assume that the filename defaults to $name.log

This comment has been minimized.

@anshprat

anshprat May 11, 2015

Contributor

yeah, that sounds good, thanks.

@anshprat

anshprat May 11, 2015

Contributor

yeah, that sounds good, thanks.

Show outdated Hide outdated manifests/glance.pp
@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 11, 2015

Contributor

please test this

Contributor

anshprat commented May 11, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 11, 2015

Contributor

please test this

Contributor

anshprat commented May 11, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 11, 2015

Contributor

please test this

Contributor

anshprat commented May 11, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 11, 2015

Contributor

please test this

Contributor

anshprat commented May 11, 2015

please test this

@@ -19,4 +19,19 @@
}
include ::contrail
$contrail_logs = [ 'contrail-analytic-api',

This comment has been minimized.

@bodepd

bodepd May 11, 2015

Contributor

where did you get this list of services from? When I look in /var/log/contrail, I see

-rw-r--r--  1 contrail contrail        0 May 11 18:08 vnc_openstack.err.1
-rw-r--r--  1 contrail adm             0 May 11 18:08 ifmap-server-console.log
-rw-r--r--  1 contrail contrail     4991 May 11 18:10 vnc_openstack.tmp
-rw-r--r--  1 contrail contrail     4992 May 11 18:10 vnc_openstack.err
-rw-r--r--  1 contrail contrail 10485938 May 11 19:12 contrail-control.log.5
-rw-r--r--  1 contrail contrail 10485801 May 11 20:16 contrail-control.log.4
-rw-r--r--  1 contrail contrail 10486070 May 11 21:19 contrail-control.log.3
-rw-r--r--  1 contrail contrail 10486020 May 11 22:22 contrail-control.log.2
-rw-r--r--  1 contrail contrail  1048836 May 11 22:43 dns.log.10
-rw-r--r--  1 contrail contrail  1048868 May 11 22:49 dns.log.9
-rw-r--r--  1 contrail contrail  1048692 May 11 22:56 dns.log.8
-rw-r--r--  1 contrail contrail  1048848 May 11 23:02 dns.log.7
-rw-r--r--  1 contrail contrail  1048886 May 11 23:08 dns.log.6
-rw-r--r--  1 contrail contrail  1048826 May 11 23:15 dns.log.5
-rw-r--r--  1 contrail contrail  1048865 May 11 23:21 dns.log.4
-rw-r--r--  1 contrail contrail 10485967 May 11 23:25 contrail-control.log.1
-rw-r--r--  1 contrail contrail  1048844 May 11 23:28 dns.log.3
-rw-r--r--  1 contrail contrail  1048873 May 11 23:34 dns.log.2
-rw-r--r--  1 contrail contrail  1048882 May 11 23:40 dns.log.1
drwxr-x---  2 contrail adm          4096 May 11 23:40 .
-rw-r--r--  1 contrail contrail  4425415 May 11 23:44 api.log
-rw-r--r--  1 contrail contrail  3909645 May 11 23:44 api-0-zk.log
-rw-r--r--  1 contrail contrail   377955 May 11 23:44 schema-zk.log
-rw-r--r--  1 contrail contrail  3060852 May 11 23:44 schema.log
-rw-r--r--  1 contrail contrail  1515973 May 11 23:44 discovery.log
-rw-r--r--  1 contrail adm       4697308 May 11 23:44 ifmap-server.log
-rw-r--r--  1 contrail contrail   605399 May 11 23:44 dns.log
-rw-r--r--  1 contrail contrail  3250388 May 11 23:44 contrail-control.log
@bodepd

bodepd May 11, 2015

Contributor

where did you get this list of services from? When I look in /var/log/contrail, I see

-rw-r--r--  1 contrail contrail        0 May 11 18:08 vnc_openstack.err.1
-rw-r--r--  1 contrail adm             0 May 11 18:08 ifmap-server-console.log
-rw-r--r--  1 contrail contrail     4991 May 11 18:10 vnc_openstack.tmp
-rw-r--r--  1 contrail contrail     4992 May 11 18:10 vnc_openstack.err
-rw-r--r--  1 contrail contrail 10485938 May 11 19:12 contrail-control.log.5
-rw-r--r--  1 contrail contrail 10485801 May 11 20:16 contrail-control.log.4
-rw-r--r--  1 contrail contrail 10486070 May 11 21:19 contrail-control.log.3
-rw-r--r--  1 contrail contrail 10486020 May 11 22:22 contrail-control.log.2
-rw-r--r--  1 contrail contrail  1048836 May 11 22:43 dns.log.10
-rw-r--r--  1 contrail contrail  1048868 May 11 22:49 dns.log.9
-rw-r--r--  1 contrail contrail  1048692 May 11 22:56 dns.log.8
-rw-r--r--  1 contrail contrail  1048848 May 11 23:02 dns.log.7
-rw-r--r--  1 contrail contrail  1048886 May 11 23:08 dns.log.6
-rw-r--r--  1 contrail contrail  1048826 May 11 23:15 dns.log.5
-rw-r--r--  1 contrail contrail  1048865 May 11 23:21 dns.log.4
-rw-r--r--  1 contrail contrail 10485967 May 11 23:25 contrail-control.log.1
-rw-r--r--  1 contrail contrail  1048844 May 11 23:28 dns.log.3
-rw-r--r--  1 contrail contrail  1048873 May 11 23:34 dns.log.2
-rw-r--r--  1 contrail contrail  1048882 May 11 23:40 dns.log.1
drwxr-x---  2 contrail adm          4096 May 11 23:40 .
-rw-r--r--  1 contrail contrail  4425415 May 11 23:44 api.log
-rw-r--r--  1 contrail contrail  3909645 May 11 23:44 api-0-zk.log
-rw-r--r--  1 contrail contrail   377955 May 11 23:44 schema-zk.log
-rw-r--r--  1 contrail contrail  3060852 May 11 23:44 schema.log
-rw-r--r--  1 contrail contrail  1515973 May 11 23:44 discovery.log
-rw-r--r--  1 contrail adm       4697308 May 11 23:44 ifmap-server.log
-rw-r--r--  1 contrail contrail   605399 May 11 23:44 dns.log
-rw-r--r--  1 contrail contrail  3250388 May 11 23:44 contrail-control.log

This comment has been minimized.

@anshprat

anshprat May 12, 2015

Contributor

huh! This is weird. I used the existing logrotate configs as source of truth. I did look up and compare a sample of nodes and services but not all I guess. Will cross check.

@anshprat

anshprat May 12, 2015

Contributor

huh! This is weird. I used the existing logrotate configs as source of truth. I did look up and compare a sample of nodes and services but not all I guess. Will cross check.

This comment has been minimized.

@anshprat

anshprat May 12, 2015

Contributor

Most of contrail logs are managed by the default log4cxx module via the respective package. The defaults are really low (few M size and count 10) - which gives us only a couple of hours of log - for example for contrail-dns.

To fix this correctly, as per my discussion in opencontrail irc, we will need to create respective logger property files and point the respective confs to these properties and then use logrotate to manage the files.

http://paste.openstack.org/show/220587/

This above task is outside the scope of logrotate and hence will do it under another PR.

@anshprat

anshprat May 12, 2015

Contributor

Most of contrail logs are managed by the default log4cxx module via the respective package. The defaults are really low (few M size and count 10) - which gives us only a couple of hours of log - for example for contrail-dns.

To fix this correctly, as per my discussion in opencontrail irc, we will need to create respective logger property files and point the respective confs to these properties and then use logrotate to manage the files.

http://paste.openstack.org/show/220587/

This above task is outside the scope of logrotate and hence will do it under another PR.

Show outdated Hide outdated manifests/glance.pp
@@ -30,6 +30,10 @@
include ::neutron::server
include ::neutron::quota
rjil::jiocloud::logrotate { 'neutron-server':

This comment has been minimized.

@bodepd

bodepd May 11, 2015

Contributor

this looks wrong:

ls /var/log/neutron/
neutron-netns-cleanup.log  server.log
@bodepd

bodepd May 11, 2015

Contributor

this looks wrong:

ls /var/log/neutron/
neutron-netns-cleanup.log  server.log

This comment has been minimized.

@anshprat

anshprat May 12, 2015

Contributor

fixed the server.log. the netns-cleanup.log looks mostly like part of the issue discussed above, so will need to handle it separately.

@anshprat

anshprat May 12, 2015

Contributor

fixed the server.log. the netns-cleanup.log looks mostly like part of the issue discussed above, so will need to handle it separately.

@bodepd

This comment has been minimized.

Show comment
Hide comment
@bodepd

bodepd May 12, 2015

Contributor

one other thing to add. You might want to also look at rotating the upstart logs, or least have a look in /var/log/upstart to see if they are getting out of hand.

Contributor

bodepd commented May 12, 2015

one other thing to add. You might want to also look at rotating the upstart logs, or least have a look in /var/log/upstart to see if they are getting out of hand.

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 12, 2015

Contributor

yes, I had checked upstart, they are being rotated correctly by a *.log upstart logrotate config (I guess from the package). I am first targeting the openstack related logs, will then puppetize the rest of system configs.

Contributor

anshprat commented May 12, 2015

yes, I had checked upstart, they are being rotated correctly by a *.log upstart logrotate config (I guess from the package). I am first targeting the openstack related logs, will then puppetize the rest of system configs.

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 12, 2015

Contributor

please test this

Contributor

anshprat commented May 12, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 13, 2015

Contributor

please test this

Contributor

anshprat commented May 13, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 13, 2015

Contributor

please test this

Contributor

anshprat commented May 13, 2015

please test this

@anshprat

This comment has been minimized.

Show comment
Hide comment
@anshprat

anshprat May 14, 2015

Contributor

please test this

Contributor

anshprat commented May 14, 2015

please test this

$rotate_every = 'daily',
$rotate = 60,
$compress = true,
$delaycompress = true,
$ifempty = false,
) {
if ($logfile == "notset"){

This comment has been minimized.

@bodepd

bodepd May 14, 2015

Contributor

I'm not a huge fan of picking a string value to mean not set, it's more in line with conventions to use false or undef
in which case you can just say:

if $var {

}
@bodepd

bodepd May 14, 2015

Contributor

I'm not a huge fan of picking a string value to mean not set, it's more in line with conventions to use false or undef
in which case you can just say:

if $var {

}
@bodepd

This comment has been minimized.

Show comment
Hide comment
@bodepd

bodepd May 14, 2015

Contributor

resubmitted as #603 with unit tests

Contributor

bodepd commented May 14, 2015

resubmitted as #603 with unit tests

@bodepd bodepd closed this May 14, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment