)]}'
{"functions":[{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"0e3038ac8d5b2100a99fe61c92c549618535b772","unresolved":false,"context_lines":[{"line_number":10,"context_line":"run_playbook() {"},{"line_number":11,"context_line":"  local playbook\u003d$1"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"  pip install ansible"},{"line_number":14,"context_line":"  pushd ${SCRIPT_DIR}"},{"line_number":15,"context_line":"  ansible-playbook -i \"localhost,\" -c local --extra-vars\u003d\"zuul_work_dir\u003d${SCRIPT_DIR}\" playbooks/${playbook}.yaml"},{"line_number":16,"context_line":"  popd"}],"source_content_type":"application/x-shellscript","patch_set":7,"id":"5f4e5783_ace0decd","line":13,"range":{"start_line":13,"start_character":2,"end_line":13,"end_character":21},"updated":"2017-10-11 21:25:10.000000000","message":"If you\u0027re going to do this, it should be at the very least in a virtual environment so that it does not pollute the OpenStack components (especially mixing deb/rpm and pip installations).\n\nYou\u0027re also probably not going to be installing ansible every time you\u0027re going to run a playbook.\n\nMaybe we can do something similar to what devstack-gate [1] (and then use $ANSIBLE_PLAYBOOK) and openstack-ansible [2] are doing (and then use /opt/ansible-runtime/bin/ansible-playbook).\n\n[1]: https://github.com/openstack-infra/devstack-gate/blob/e473fdec13445191d1acf1c77d19c87bbda512c8/devstack-vm-gate-wrap.sh#L473-L484\n[2]: https://github.com/openstack/openstack-ansible/blob/36d3860008f98b96754170b4f487ee983b1eeba7/scripts/bootstrap-ansible.sh#L146-L192","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"}],"playbooks/roles/r10k/defaults/main.yaml":[{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"0e3038ac8d5b2100a99fe61c92c549618535b772","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"zuul_work_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_6c44261c","line":4,"range":{"start_line":4,"start_character":16,"end_line":4,"end_character":42},"updated":"2017-10-11 21:25:10.000000000","message":"I think this needs to be \"{{ zuul.project.src_dir | default(\u0027something\u0027) }}\" in order to allow running outside of a Zuul v3 environment.","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"0e3038ac8d5b2100a99fe61c92c549618535b772","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"zuul_work_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_ccbc3ade","line":4,"range":{"start_line":4,"start_character":0,"end_line":4,"end_character":13},"updated":"2017-10-11 21:25:10.000000000","message":"Should we name this something else that is more generic ? We\u0027re naming this SCRIPT_DIR in functions.sh, ultimately it\u0027s the directory where the puppetfile(s) are located ?\n\nWe could name it like r10k_puppetfile_dir ? or something ?","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"5fa040697fd53c7e118c4dc6ed697de57697bcaa","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"zuul_work_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_02c2f6e9","line":4,"range":{"start_line":4,"start_character":0,"end_line":4,"end_character":13},"in_reply_to":"5f4e5783_ccbc3ade","updated":"2017-10-13 02:57:01.000000000","message":"This is done, but we\u0027re going to use a transitionary \u0027zuul_work_dir\u0027 variable to manage things around till we\u0027re fully ansible-izied","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"6c3c7b24e8dd9e393f161501f3f62124e785841f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"r10k_puppetfile_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f4e5783_899959c5","line":4,"updated":"2017-10-13 14:53:43.000000000","message":"Same comment as earlier patchset, if I try to run this outside of zuul v3 it wouldn\u0027t work because \"zuul.project.src_dir\" won\u0027t be set.\n\nIt should default to a sane value but perhaps it should be handled inside the playbook(s). I\u0027ve done something like this before... For example, a block that handles if r10k_puppetfile_dir is empty/undefined:\n\n\n    - name: Retrieve the source directory\n      shell: \"cd {{ playbook_dir }}/..; pwd\"\n      delegate_to: localhost\n      register: path\n    \n    - name: Set the source directory\n      set_fact:\n        source_directory: \"{{ path.stdout }}\"","commit_id":"00b8a2697e17cc4f5f0fe20af77e8de18676ff47"},{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"90f51f4528cb7f1d7d4c623395f580565546306a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"r10k_puppetfile_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f4e5783_52542521","line":4,"in_reply_to":"5f4e5783_29994578","updated":"2017-10-13 20:16:32.000000000","message":"That leaves an \u0027exercise for the reader\u0027 for something that could be provided as a reasonably safe default. I don\u0027t know.\n\nWe could do here:\n\n    r10k_puppetfile_dir: \"{{ zuul.project.src_dir | default(false) }}\"\n\nAnd then in the main.yml:\n\n    - when: not r10k_puppetfile_dir\n      block:\n        - name: Retrieve the project directory\n          shell: \"cd {{ playbook_dir }}/..; pwd\"\n          register: path\n        \n        - name: Set the source directory\n          set_fact:\n            source_directory: \"{{ path.stdout }}\"\n\n        - name: Warning about running outside of a Zuul environment\n          debug:\n            msg: |\n              The required Zuul variables for setting up the project root could\n              not be found, we have defaulted it to \"{{ source_directory }}\".\n              If you need to override this, provide a value for the\n              \u0027r10k_puppetfile_dir\u0027 variable.","commit_id":"00b8a2697e17cc4f5f0fe20af77e8de18676ff47"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"94e828826ec14299e215c6d369cfb61b6fdefa5d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"r10k_puppetfile: Puppetfile"},{"line_number":3,"context_line":"r10k_extra_args: -v DEBUG"},{"line_number":4,"context_line":"r10k_puppetfile_dir: \"{{ zuul.project.src_dir }}\""}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f4e5783_29994578","line":4,"in_reply_to":"5f4e5783_899959c5","updated":"2017-10-13 14:59:29.000000000","message":"What about simply dropping it and leave it required for the playbook that consumes this roll to provide this?","commit_id":"00b8a2697e17cc4f5f0fe20af77e8de18676ff47"}],"playbooks/roles/r10k/tasks/main.yaml":[{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"0e3038ac8d5b2100a99fe61c92c549618535b772","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Ensure r10k is installed"},{"line_number":3,"context_line":"  gem:"},{"line_number":4,"context_line":"    name: r10k"},{"line_number":5,"context_line":"    state: present"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"- name: Install Puppet modules"},{"line_number":8,"context_line":"  command: \"timeout 300 r10k {{ r10k_extra_args }} puppetfile install\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_79874a11","line":5,"range":{"start_line":2,"start_character":0,"end_line":5,"end_character":18},"updated":"2017-10-11 21:25:10.000000000","message":"Sort of unrelated and could be in a future review but I think it would be a good idea to ensure that the necessary dependencies for \u0027gem\u0027 (rubygems) and r10k (ruby-devel, libxml2-devel, libxslt-devel?) are installed before running this ?\n\nThere\u0027s a bit of an overlap with bindep now, I guess ?\nThere\u0027s a role for bindep which uses bindep.txt.. but things should ideally be self-contained in roles. /me shrugs","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"5fa040697fd53c7e118c4dc6ed697de57697bcaa","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- name: Ensure r10k is installed"},{"line_number":3,"context_line":"  gem:"},{"line_number":4,"context_line":"    name: r10k"},{"line_number":5,"context_line":"    state: present"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"- name: Install Puppet modules"},{"line_number":8,"context_line":"  command: \"timeout 300 r10k {{ r10k_extra_args }} puppetfile install\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_e2ea3a73","line":5,"range":{"start_line":2,"start_character":0,"end_line":5,"end_character":18},"in_reply_to":"5f4e5783_79874a11","updated":"2017-10-13 02:57:01.000000000","message":"I\u0027m working on refactoring this so that all of the necessary components are installed in bindep, and the gem is installed without sudo.","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":9061,"name":"David Moreau Simard","email":"moi@dmsimard.com","username":"dmsimard"},"change_message_id":"0e3038ac8d5b2100a99fe61c92c549618535b772","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"- name: Install Puppet modules"},{"line_number":8,"context_line":"  command: \"timeout 300 r10k {{ r10k_extra_args }} puppetfile install\""},{"line_number":9,"context_line":"  retries: 5"},{"line_number":10,"context_line":"  delay: 10"},{"line_number":11,"context_line":"  args:"},{"line_number":12,"context_line":"    chdir: \"{{ zuul_work_dir }}\""},{"line_number":13,"context_line":"  environment:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_acd57e45","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":11},"updated":"2017-10-11 21:25:10.000000000","message":"I don\u0027t think the usage of retries/delay will do what you\u0027re expecting it to do here.\n\nAccording to docs [1], if an \u0027until\u0027 is not defined, the \u0027retries\u0027 is forced to 1.\n\nIn any case, if I run this locally:\n\n    - command: /bin/false\n      retries: 5\n      delay: 10\n\nIt fails once and doesn\u0027t retry.\n\nIf I run this instead:\n\n    - command: /bin/false\n      retries: 5\n      delay: 10\n      until: result.rc \u003d\u003d 0\n      register: result\n\nIt\u0027ll retry 5 times with 10 seconds in between.\n\n\u0027timeout\u0027 (or r10k) will appropriately exit 1 if they fail so we probably want register and use the rc inside an until.\n\n[1]: http://docs.ansible.com/ansible/latest/playbooks_loops.html#do-until-loops","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"5fa040697fd53c7e118c4dc6ed697de57697bcaa","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"- name: Install Puppet modules"},{"line_number":8,"context_line":"  command: \"timeout 300 r10k {{ r10k_extra_args }} puppetfile install\""},{"line_number":9,"context_line":"  retries: 5"},{"line_number":10,"context_line":"  delay: 10"},{"line_number":11,"context_line":"  args:"},{"line_number":12,"context_line":"    chdir: \"{{ zuul_work_dir }}\""},{"line_number":13,"context_line":"  environment:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"5f4e5783_c2edfe8b","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":11},"in_reply_to":"5f4e5783_acd57e45","updated":"2017-10-13 02:57:01.000000000","message":"Done","commit_id":"94e1a140435ef870486e6bb994e4c14fa73b1001"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"64f2cc200e92f6287b70f95f95bcb373ff041fdc","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    state: present"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"- name: Install Puppet modules"},{"line_number":8,"context_line":"  command: \"timeout 300 r10k {{ r10k_extra_args }} puppetfile install\""},{"line_number":9,"context_line":"  retries: 5"},{"line_number":10,"context_line":"  delay: 10"},{"line_number":11,"context_line":"  register: result"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"1f9dbf25_21039d11","line":8,"updated":"2018-03-04 00:40:19.000000000","message":"Not really related to the change to Ansible but the r10k call here should be changed to use the --moduledir and --puppetfile parameters instead of environments variables since it\u0027s deprecated since several years[1] and will be removed very soon when they cleanup in a new release.\n\nSee my previous take on changing that [2]\n\n[1] https://github.com/puppetlabs/r10k/pull/479\n[2] https://review.openstack.org/#/c/548285/","commit_id":"00b8a2697e17cc4f5f0fe20af77e8de18676ff47"}]}