)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"3f562b6935ab43dfdbcd6971c606b35ad2a9ce47","unresolved":true,"context_lines":[{"line_number":15,"context_line":"\u003e The use of `ignore_errors` is generally frowned upon due to the terrible"},{"line_number":16,"context_line":"  UX is carries with it, however, in this case I believe the terrible UX"},{"line_number":17,"context_line":"  makes sense. Generally we\u0027d want the error handling to be silent so the"},{"line_number":18,"context_line":"  user isn\u0027t bothered by the experience but in this case users should be"},{"line_number":19,"context_line":"  informed that selinux is disabled, and be presented with the module"},{"line_number":20,"context_line":"  output in the hopes that the user will change selinux enablement later."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ifd31adcf27902a8a77de9c68482306ec9da6d250"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5d3313ed_854fe70a","line":19,"range":{"start_line":18,"start_character":44,"end_line":19,"end_character":34},"updated":"2022-02-08 09:25:47.000000000","message":"We probably should do a check at the very beginning, and ask for a confirmation. That\u0027s the proper way to inform a user.","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"},{"author":{"_account_id":7353,"name":"Kevin Carter","email":"kevin@cloudnull.com","username":"cloudnull"},"change_message_id":"b0da0aaaa19f776bcea229a5f2ec3a7057ba1835","unresolved":true,"context_lines":[{"line_number":15,"context_line":"\u003e The use of `ignore_errors` is generally frowned upon due to the terrible"},{"line_number":16,"context_line":"  UX is carries with it, however, in this case I believe the terrible UX"},{"line_number":17,"context_line":"  makes sense. Generally we\u0027d want the error handling to be silent so the"},{"line_number":18,"context_line":"  user isn\u0027t bothered by the experience but in this case users should be"},{"line_number":19,"context_line":"  informed that selinux is disabled, and be presented with the module"},{"line_number":20,"context_line":"  output in the hopes that the user will change selinux enablement later."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ifd31adcf27902a8a77de9c68482306ec9da6d250"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c6615ad6_0556f36d","line":19,"range":{"start_line":18,"start_character":44,"end_line":19,"end_character":34},"in_reply_to":"5d3313ed_854fe70a","updated":"2022-02-08 13:48:04.000000000","message":"No, a prompt is failure; we\u0027d be checking the remote machines for the selinux status. If anything I could see the validations folks doing a check and returning the status but from an automation point of view halting a run and returning a prompt should be a non-starter.","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"e4fae64051f70359073fe9a9621137440606a4e3","unresolved":true,"context_lines":[{"line_number":15,"context_line":"\u003e The use of `ignore_errors` is generally frowned upon due to the terrible"},{"line_number":16,"context_line":"  UX is carries with it, however, in this case I believe the terrible UX"},{"line_number":17,"context_line":"  makes sense. Generally we\u0027d want the error handling to be silent so the"},{"line_number":18,"context_line":"  user isn\u0027t bothered by the experience but in this case users should be"},{"line_number":19,"context_line":"  informed that selinux is disabled, and be presented with the module"},{"line_number":20,"context_line":"  output in the hopes that the user will change selinux enablement later."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ifd31adcf27902a8a77de9c68482306ec9da6d250"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3184af15_8e0d22ea","line":19,"range":{"start_line":18,"start_character":44,"end_line":19,"end_character":34},"in_reply_to":"c6615ad6_0556f36d","updated":"2022-02-08 15:56:10.000000000","message":"the \"-y|--yes\" might be a solution.","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"e341eddd0b41c4dfcb594058444e7bfb4ddcf794","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3e29b50b_f51237b8","updated":"2022-02-08 09:12:38.000000000","message":"Sorry - but no.\n\nThe right way is:\nwhen:\n  - ansible_facts.selinux is undefined\n  - ansible_facts.selinux.status \u003d\u003d \"enabled\"\n\nMasking the issue like you\u0027re proposing is terrible, since it would mask issues caused by, say, failed openstack-selinux package install or the like (and YES, it happens).","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"},{"author":{"_account_id":7353,"name":"Kevin Carter","email":"kevin@cloudnull.com","username":"cloudnull"},"change_message_id":"b0da0aaaa19f776bcea229a5f2ec3a7057ba1835","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a4fd8451_07cdbc7a","in_reply_to":"3e29b50b_f51237b8","updated":"2022-02-08 13:48:04.000000000","message":"This would be a way, but it is not the right way.\n\nIn this case with `ignore_errors`, which I generally dislike, would inform the user would by producing a failed but ignored notice at the end of the run and a big red notification during the run that SELinux was disabled; in my opinion, not being able to set an seboolean shouldn\u0027t be the cause of automation failure.\n\nI can add the multiple condition but that will not raise any notification that SELinux is disabled; which I believe is a mistake, additionally, it would be a mistake to use a block/extra task to print a debug notice.\n\nWhile I\u0027m sure there are issues that have happened with `openstack-selinux` not being installed or in some failed state, this change would not have anything to do with that; every use of `seboolean` throughout THT is for the same basic context.","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"e4fae64051f70359073fe9a9621137440606a4e3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"29738e71_723e4b0c","in_reply_to":"a4fd8451_07cdbc7a","updated":"2022-02-08 15:56:10.000000000","message":"Still - I\u0027d rather see a plain failure that\u0027s actually preventing things to move forward if selinux is enabled, and the sebool fails for #reason.\nFor instance, there are 2 booleans:\n- virt_sandbox_use_netlink, coming from container-selinux\n- os_enable_vtpm coming from openstack-selinux\n\nCurrently, there\u0027s a situation where container-selinux doesn\u0027t properly install, leading to a failure for openstack-selinux; so those booleans don\u0027t exist on the system.\nIf we\u0027re passing through without actually failing the deploy, it will lead to some really weird issues, and the operator will lose their mind, hairs and patience looking for the actual reason.\n\nNote that, hopefully, the container-selinux will be updated accordingly in order to not fail anymore, but still, it\u0027s something that may happen, depending on what\u0027s going on with other packages.\n\n\"Silently\" ignoring isn\u0027t good imho. But, in order to not completely block this review, I\u0027ll just push a \"-1\". And see how it goes forward with this proposal.","commit_id":"e5bfab4ec41b7f7e7387e80c05845e23679e721a"},{"author":{"_account_id":14287,"name":"Waldemar Znoinski","email":"wznoinsk@redhat.com","username":"wznoinsk"},"change_message_id":"ea2920ebc1920bd9efdc784a33611c405df4665c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d00a303c_4eb8c677","updated":"2022-02-14 22:48:56.000000000","message":"check-rdo","commit_id":"7e8d88afa507df89e0a6848e1992021f0fb10f03"},{"author":{"_account_id":14287,"name":"Waldemar Znoinski","email":"wznoinsk@redhat.com","username":"wznoinsk"},"change_message_id":"9504431d0fb0ff46c476bfb6c1975daf4d713236","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0943ee6b_50ab20a3","updated":"2022-02-14 12:59:01.000000000","message":"recheck tripleo-ci-centos-8-undercloud-upgrade","commit_id":"7e8d88afa507df89e0a6848e1992021f0fb10f03"}]}
