)]}'
{"manifests/logging.pp":[{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"a835737cf78071b5d829a80d6ac5b3366815567a","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"  $log_dir_owner \u003d pick($::nova::nova_user, \u0027nova\u0027)"},{"line_number":127,"context_line":"  $log_dir_group \u003d pick($::nova::nova_log_group, \u0027nova\u0027)"},{"line_number":128,"context_line":"  file { $log_dir_real :"},{"line_number":129,"context_line":"    ensure    \u003d\u003e directory,"},{"line_number":130,"context_line":"    owner     \u003d\u003e $log_dir_owner,"},{"line_number":131,"context_line":"    group     \u003d\u003e $log_dir_group,"}],"source_content_type":"text/x-puppet","patch_set":4,"id":"df87a7cf_849d3fbd","line":128,"range":{"start_line":128,"start_character":2,"end_line":128,"end_character":24},"updated":"2017-12-12 00:42:21.000000000","message":"TODO: test","commit_id":"338a9ee86de89d0aff859b197504f986df15f7cf"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"48f8abfdd50fd7366d87ca8d28de0a63b9918eb6","unresolved":false,"context_lines":[{"line_number":136,"context_line":"      recurselimit \u003d\u003e 1,"},{"line_number":137,"context_line":"      require      \u003d\u003e Anchor[\u0027nova::install::end\u0027]"},{"line_number":138,"context_line":"    }"},{"line_number":139,"context_line":"    # Setgid to ensure logs get the correct group"},{"line_number":140,"context_line":"    exec { \u0027setgid_nova_log_dir\u0027:"},{"line_number":141,"context_line":"      command \u003d\u003e \"chmod 2750 ${log_dir_real}\","},{"line_number":142,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":143,"context_line":"      onlyif  \u003d\u003e \"test \\$(stat -c \u0027%a\u0027 ${log_dir_real}) !\u003d 2750\","},{"line_number":144,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":145,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":146,"context_line":"    }"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # This should force an update of the gid and selinux role. They will be incorrect if"},{"line_number":149,"context_line":"    # the log was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":23,"id":"df87a7cf_2393d251","line":146,"range":{"start_line":139,"start_character":4,"end_line":146,"end_character":5},"updated":"2017-12-13 20:24:44.000000000","message":"TODO: push this up to the rpm/deb.","commit_id":"021af54cf843aa0ccadd8c9049b1f0ffe7ab213a"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"eaa687740082a84b1d019c03ca989e34f9aa3994","unresolved":false,"context_lines":[{"line_number":136,"context_line":"      recurselimit \u003d\u003e 1,"},{"line_number":137,"context_line":"      require      \u003d\u003e Anchor[\u0027nova::install::end\u0027]"},{"line_number":138,"context_line":"    }"},{"line_number":139,"context_line":"    # Setgid to ensure logs get the correct group"},{"line_number":140,"context_line":"    exec { \u0027setgid_nova_log_dir\u0027:"},{"line_number":141,"context_line":"      command \u003d\u003e \"chmod 2750 ${log_dir_real}\","},{"line_number":142,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":143,"context_line":"      onlyif  \u003d\u003e \"test \\$(stat -c \u0027%a\u0027 ${log_dir_real}) !\u003d 2750\","},{"line_number":144,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":145,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":146,"context_line":"    }"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # This should force an update of the gid and selinux role. They will be incorrect if"},{"line_number":149,"context_line":"    # the log was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":23,"id":"df87a7cf_6093fa50","line":146,"range":{"start_line":139,"start_character":4,"end_line":146,"end_character":5},"in_reply_to":"df87a7cf_2393d251","updated":"2017-12-14 14:54:16.000000000","message":"Will remove it and use the exec group param to ensure the gid is correct","commit_id":"021af54cf843aa0ccadd8c9049b1f0ffe7ab213a"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"2b627df03dd42f3d67dec3e3011a9bc36e728424","unresolved":false,"context_lines":[{"line_number":136,"context_line":"      recurselimit \u003d\u003e 1,"},{"line_number":137,"context_line":"      require      \u003d\u003e Anchor[\u0027nova::install::end\u0027]"},{"line_number":138,"context_line":"    }"},{"line_number":139,"context_line":"    # Setgid to ensure logs get the correct group"},{"line_number":140,"context_line":"    exec { \u0027setgid_nova_log_dir\u0027:"},{"line_number":141,"context_line":"      command \u003d\u003e \"chmod 2750 ${log_dir_real}\","},{"line_number":142,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":143,"context_line":"      onlyif  \u003d\u003e \"test \\$(stat -c \u0027%a\u0027 ${log_dir_real}) !\u003d 2750\","},{"line_number":144,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":145,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":146,"context_line":"    }"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # This should force an update of the gid and selinux role. They will be incorrect if"},{"line_number":149,"context_line":"    # the log was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":23,"id":"df87a7cf_6dabee74","line":146,"range":{"start_line":139,"start_character":4,"end_line":146,"end_character":5},"in_reply_to":"df87a7cf_6093fa50","updated":"2017-12-14 16:56:57.000000000","message":"hrmm, not idempotent on ubuntu without this","commit_id":"021af54cf843aa0ccadd8c9049b1f0ffe7ab213a"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"fadefcf2e3f74ed4d68d2a538b461a01f47c3df2","unresolved":false,"context_lines":[{"line_number":150,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":151,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":152,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":153,"context_line":"    }"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    # This should force an update the selinux role if the logfile exists."},{"line_number":156,"context_line":"    # It will be incorrect if the file was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":25,"id":"df87a7cf_68bce505","line":153,"updated":"2017-12-18 20:55:56.000000000","message":"Would recurse \u003d\u003e true on the file { $log_dir_real: ... } not suffice?  I tested what is mentioned in the comment and it seems to work just fine. This seems to possibly introduce idempotency issues if you use a different user\n\n\n1:49 PM ☁  ~  ➜ mkdir /tmp/foo\n1:49 PM ☁  ~  ➜ touch /tmp/foo/bar\n1:49 PM ☁  ~  ➜ chown root:root /tmp/foo/bar \n1:51 PM ☁  ~  ➜ cat perms.pp \nfile { \u0027/tmp/foo\u0027:\n  ensure  \u003d\u003e directory,\n  owner   \u003d\u003e \u0027aschultz\u0027,\n  group   \u003d\u003e \u0027aschultz\u0027,\n  recurse \u003d\u003e true\n}\n\nfile { \u0027/tmp/foo/baz\u0027:\n  ensure \u003d\u003e file,\n  owner  \u003d\u003e \u0027root\u0027,\n  group  \u003d\u003e \u0027root\u0027,\n}\n1:51 PM ☁  ~  ➜ sudo puppet apply perms.pp\nNotice: Compiled catalog for aschultz.localdomain in environment production in 0.15 seconds\nNotice: /Stage[main]/Main/File[/tmp/foo/bar]/owner: owner changed \u0027root\u0027 to \u0027aschultz\u0027\nNotice: /Stage[main]/Main/File[/tmp/foo/bar]/group: group changed \u0027root\u0027 to \u0027aschultz\u0027\nNotice: /Stage[main]/Main/File[/tmp/foo/baz]/ensure: created\nNotice: Finished catalog run in 0.07 seconds","commit_id":"05803222923ddfba2342b1f95aab1aa34c9350ad"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"053c5b27bf67a0beab3c15671adf7eab494f01c9","unresolved":false,"context_lines":[{"line_number":150,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":151,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":152,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":153,"context_line":"    }"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    # This should force an update the selinux role if the logfile exists."},{"line_number":156,"context_line":"    # It will be incorrect if the file was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":25,"id":"df87a7cf_8714a6a1","line":153,"in_reply_to":"df87a7cf_39f63165","updated":"2017-12-19 14:01:21.000000000","message":"Well the group thing could be accounted for in params.  I think we should just switch it to use recurse","commit_id":"05803222923ddfba2342b1f95aab1aa34c9350ad"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"1dd8079614518e468703a31848c2c57352e1ddfd","unresolved":false,"context_lines":[{"line_number":150,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":151,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":152,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":153,"context_line":"    }"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    # This should force an update the selinux role if the logfile exists."},{"line_number":156,"context_line":"    # It will be incorrect if the file was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":25,"id":"df87a7cf_39f63165","line":153,"in_reply_to":"df87a7cf_68bce505","updated":"2017-12-19 01:00:48.000000000","message":"Could use a recursive file resource with nova:nova (would be idempotent I think) but it contradicts the rpm/deb scripts.\n\nrpm sets /var/log/nova to nova:root, deb sets it to nova:adm, but the log files (created on first use by oslo) are nova:nova.","commit_id":"05803222923ddfba2342b1f95aab1aa34c9350ad"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"74b13295819fb0e0113a7a03f952f8fab9e659c2","unresolved":false,"context_lines":[{"line_number":150,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":151,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":152,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":153,"context_line":"    }"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    # This should force an update the selinux role if the logfile exists."},{"line_number":156,"context_line":"    # It will be incorrect if the file was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":25,"id":"df87a7cf_ca794ec5","line":153,"in_reply_to":"df87a7cf_82bed462","updated":"2017-12-19 18:29:28.000000000","message":"Might be worth keeping it as there are reports of a similar issue with nova-placement.log. Don\u0027t have a root cause for that one though, probably not directly related to puppet-nova.","commit_id":"05803222923ddfba2342b1f95aab1aa34c9350ad"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"05c0dc5390fb564ea689eb2eddbd3989fe21010e","unresolved":false,"context_lines":[{"line_number":150,"context_line":"      path    \u003d\u003e [\u0027/usr/bin\u0027, \u0027/bin\u0027],"},{"line_number":151,"context_line":"      require \u003d\u003e File[$log_dir_real],"},{"line_number":152,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"},{"line_number":153,"context_line":"    }"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    # This should force an update the selinux role if the logfile exists."},{"line_number":156,"context_line":"    # It will be incorrect if the file was created by the dbsync exec resources."}],"source_content_type":"text/x-puppet","patch_set":25,"id":"df87a7cf_82bed462","line":153,"in_reply_to":"df87a7cf_8714a6a1","updated":"2017-12-19 14:56:59.000000000","message":"Ok so I think recurse is not idempotent unless we define all the log files. Maybe we should put a todo to remove this next cycle as this is really only for the upgrade case where the commands were run as root. Once they get switched over I don\u0027t think we need this stuff anymore.","commit_id":"05803222923ddfba2342b1f95aab1aa34c9350ad"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"dc126fa500e03daeb0bbcf753582094cf68b8f2c","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    file { $log_dir_real:"},{"line_number":133,"context_line":"      ensure  \u003d\u003e directory,"},{"line_number":134,"context_line":"      owner   \u003d\u003e $log_file_owner,"},{"line_number":135,"context_line":"      group   \u003d\u003e $log_file_group,"},{"line_number":136,"context_line":"      recurse \u003d\u003e true,"},{"line_number":137,"context_line":"      require \u003d\u003e Anchor[\u0027nova::install::end\u0027],"},{"line_number":138,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"}],"source_content_type":"text/x-puppet","patch_set":26,"id":"df87a7cf_a5bbdd0c","line":135,"range":{"start_line":134,"start_character":6,"end_line":135,"end_character":33},"updated":"2017-12-19 18:52:45.000000000","message":"@James @Corey interested in your opinions on this ...\n\nCurrently the .deb creates /var/log/nova with uid/gid nova:adm (nova:root for the rpm).\nLog files are created dynamically by oslo.log, typically with uid/gid nova:nova.\n\nTo keep things simple this just recursively chowns /var/log/nova to nova:nova.\n\nI can\u0027t think of anything that would break with an rpm install but I\u0027m not 100% sure about the .deb install. Is there something significant about the adm group here?\n\nThanks,\nOllie","commit_id":"6d4a3a707a70d988070ec33598838e83ab9d3198"},{"author":{"_account_id":23811,"name":"Oliver Walsh","email":"owalsh@redhat.com","username":"owalsh"},"change_message_id":"4209f5d70432cca587b5ec215ec550392f28fab9","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    file { $log_dir_real:"},{"line_number":133,"context_line":"      ensure  \u003d\u003e directory,"},{"line_number":134,"context_line":"      owner   \u003d\u003e $log_file_owner,"},{"line_number":135,"context_line":"      group   \u003d\u003e $log_file_group,"},{"line_number":136,"context_line":"      recurse \u003d\u003e true,"},{"line_number":137,"context_line":"      require \u003d\u003e Anchor[\u0027nova::install::end\u0027],"},{"line_number":138,"context_line":"      before  \u003d\u003e Anchor[\u0027nova::config::begin\u0027]"}],"source_content_type":"text/x-puppet","patch_set":26,"id":"df87a7cf_604dc370","line":135,"range":{"start_line":134,"start_character":6,"end_line":135,"end_character":33},"in_reply_to":"df87a7cf_a5bbdd0c","updated":"2017-12-19 19:27:05.000000000","message":"AFAICT the adm group is significant for rsyslog. Also failing *-ubuntu check jobs as it\u0027s not idempotent. Reverting back to PS25 which preserves the rpm/deb ownership on /var/log/nova, although it\u0027s a bit more convoluted","commit_id":"6d4a3a707a70d988070ec33598838e83ab9d3198"}]}