)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"8b3967f9c9f637069e4a383647949c4086482491","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c2ff8740_cdb31185","updated":"2021-12-03 16:05:04.000000000","message":"-1 for the nodes thing. The other comment is more to start a discussion as I can be convinced that isn\u0027t necessary.","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"f11a4e28957adf149d02090690329d105579cfd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"db015835_f6605916","updated":"2021-12-03 16:05:54.000000000","message":"Oh one more thought. We might want to change the bootstrap bridge job without changing all of the other jobs. Then we can confirm the bootstrap bridge job is always running first and configuring things the way we want before we start relying on that state update in each subsequent job run.","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"eb7aa55df20627df9b80a76639b6494838a0ec54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3fa233d1_dac84064","in_reply_to":"db015835_f6605916","updated":"2021-12-07 05:45:41.000000000","message":"Done","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"1d1dc8e97dec3814e6db24dd74c993c53a9a31e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d4b60152_68fd645f","updated":"2021-12-07 22:50:07.000000000","message":"-1 for the brainstorm comment that calls out a few issues with running from a zuul context that we will need to address.\n\nThinking out loud here should we maybe separate the bootstrapping problem out into a separate set of changes that we can work through? I am slightly concerned that debugging bootstrapping changes will be as fiddly as debugging the parallel work.","commit_id":"70a5f52cf8b2a0758a84d8e0e4166908246f96f3"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"543721c602c5c83e39651f999168feb97a732357","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"190ce3a2_21ae1c0e","updated":"2021-12-08 00:08:28.000000000","message":"This looks much better thanks. It might be worthwhile trying to see if corvus and or mordred can take a look at this one just to be sure we haven\u0027t missed anything in our brainstorm.","commit_id":"a6ac79ca40f0c18cbd04bb373fa165300b979218"}],"playbooks/bootstrap-bridge.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"2100edb4f6f6c8f9b48418a7c410444aa69732a6","unresolved":true,"context_lines":[{"line_number":35,"context_line":"      # production bridge.o.o"},{"line_number":36,"context_line":"      install_ansible_ara_name: \u0027{{ bridge_ara_name | default(\"ara[server]\") }}\u0027"},{"line_number":37,"context_line":"      install_ansible_ara_version: \u0027{{ bridge_ara_version | default(\"latest\") }}\u0027"},{"line_number":38,"context_line":"    - root-keys"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"bf7eba80_4cff7772","line":38,"updated":"2021-12-08 21:57:14.000000000","message":"This role runs and expects the private host/group vars to be present to define root_rsa_key. When we run from a zuul executor context elsewhere we do:\n\n  root_rsa_key: \"{{ lookup(\u0027file\u0027, zuul.executor.work_root + \u0027/\u0027 + zuul.build + \u0027_id_rsa\u0027, rstrip\u003dFalse) }}\"\n\nBut that would be wrong in this case. This complicates the boostrapping from scratch a bit more.","commit_id":"a6ac79ca40f0c18cbd04bb373fa165300b979218"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"9a195941d51bf333e1eaa56b1b99491a195b1735","unresolved":false,"context_lines":[{"line_number":35,"context_line":"      # production bridge.o.o"},{"line_number":36,"context_line":"      install_ansible_ara_name: \u0027{{ bridge_ara_name | default(\"ara[server]\") }}\u0027"},{"line_number":37,"context_line":"      install_ansible_ara_version: \u0027{{ bridge_ara_version | default(\"latest\") }}\u0027"},{"line_number":38,"context_line":"    - root-keys"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"c10d9a19_3b8a0e82","line":38,"in_reply_to":"bf7eba80_4cff7772","updated":"2022-11-03 21:14:49.000000000","message":"I believe this is fixed by https://review.opendev.org/c/opendev/system-config/+/861138.\n\nI need to re-merge this, but I think we are in a better position to deal with this now","commit_id":"a6ac79ca40f0c18cbd04bb373fa165300b979218"}],"zuul.d/infra-prod.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"8b3967f9c9f637069e4a383647949c4086482491","unresolved":true,"context_lines":[{"line_number":45,"context_line":"        bridge in it\u0027s role as the bastion host actaully runs that"},{"line_number":46,"context_line":"        against itself; it includes things not strictly needed to make"},{"line_number":47,"context_line":"        the host able to deploy system-config."},{"line_number":48,"context_line":"    run: playbooks/bootstrap-bridge.yaml"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- job:"},{"line_number":51,"context_line":"    name: infra-prod-base"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"0cfc300e_f836276a","line":48,"updated":"2021-12-03 16:05:04.000000000","message":"We still need to set nodes: [] on this base job.","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"eb7aa55df20627df9b80a76639b6494838a0ec54","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        bridge in it\u0027s role as the bastion host actaully runs that"},{"line_number":46,"context_line":"        against itself; it includes things not strictly needed to make"},{"line_number":47,"context_line":"        the host able to deploy system-config."},{"line_number":48,"context_line":"    run: playbooks/bootstrap-bridge.yaml"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- job:"},{"line_number":51,"context_line":"    name: infra-prod-base"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"5f6020bd_1609e2aa","line":48,"in_reply_to":"0cfc300e_f836276a","updated":"2021-12-07 05:45:41.000000000","message":"Done","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"1d1dc8e97dec3814e6db24dd74c993c53a9a31e7","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        bridge in it\u0027s role as the bastion host actaully runs that"},{"line_number":48,"context_line":"        against itself; it includes things not strictly needed to make"},{"line_number":49,"context_line":"        the host able to deploy system-config."},{"line_number":50,"context_line":"    run: playbooks/bootstrap-bridge.yaml"},{"line_number":51,"context_line":"    # NOTE(ianw): this job sets up its own inventory pointing at the"},{"line_number":52,"context_line":"    # bastion host bridge.o.o, so doesn\u0027t need ephemeral nodes."},{"line_number":53,"context_line":"    nodeset:"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"f46a14bd_535aa678","line":50,"updated":"2021-12-07 22:50:07.000000000","message":"This updates the job to run bootstrap-bridge.yaml from a zuul executor context rather than the bastion\u0027s context. This is intentional, but I wanted to brainstorm through what this might impact.\n\nThe playbook operates on bridge.openstack.org which needs to be in the inventory. Since we\u0027ve set nodes: [] below we\u0027ll have an empty inventory. Then our pre-run playbook(s) will add bridge.openstack.org to our inventory. Unfortunately because this playbook is a separate process invocation we won\u0027t have that inventory update any longer. I think that we need to ensure that we\u0027re updating the inventory again in this playbook.\n\nThe playbook also uses !disabled in the hosts list which is a bastion context group thing that checks the emergency file and the committed disabled group. We should probably consider dropping that entry to reduce confusion. Though I think we check DISABLE-ANSIBLE in the pre-run so we should never get here if we\u0027ve disabled ansible at least. But the two disable methods are sort of orthogonal to each other. One is host specific the other attempts to be global.\n\nThe playbook sets become: true. I think this isn\u0027t really a change from previous behavior. Zuul has access and is privileged. But calling it out in case I\u0027ve missed something.\n\nAll roles run from bootstrap-bridge.yaml need to be available in the zuul job context. I believe they are because the roles in that playbook are in the playbooks/roles/ dir adjacent to this playbook.\n\nFinally, the thing that concerns me most is the execution of the install-ansible role. We pass a number of parameters to it with defaults. Since we are in a zuul executor context here those values would need to be set at a Zuul level and not a bastion level. We should double check that we aren\u0027t overriding any of these defaults because if we are running this playbook from Zuul will suddenly undo them.","commit_id":"70a5f52cf8b2a0758a84d8e0e4166908246f96f3"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"4375649aab036e0cb7737a6790c221ff92fbe205","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        bridge in it\u0027s role as the bastion host actaully runs that"},{"line_number":48,"context_line":"        against itself; it includes things not strictly needed to make"},{"line_number":49,"context_line":"        the host able to deploy system-config."},{"line_number":50,"context_line":"    run: playbooks/bootstrap-bridge.yaml"},{"line_number":51,"context_line":"    # NOTE(ianw): this job sets up its own inventory pointing at the"},{"line_number":52,"context_line":"    # bastion host bridge.o.o, so doesn\u0027t need ephemeral nodes."},{"line_number":53,"context_line":"    nodeset:"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"ef767054_d5afb0d6","line":50,"in_reply_to":"f46a14bd_535aa678","updated":"2021-12-07 23:28:32.000000000","message":"\u003e The playbook operates on bridge.openstack.org which needs to be in the inventory. Since we\u0027ve set nodes: [] below we\u0027ll have an empty inventory. Then our pre-run playbook(s) will add bridge.openstack.org to our inventory. Unfortunately because this playbook is a separate process invocation we won\u0027t have that inventory update any longer. I think that we need to ensure that we\u0027re updating the inventory again in this playbook.\n\n++ -- I have added this with PS4\n \n\u003e The playbook also uses !disabled in the hosts list which is a bastion context group thing that checks the emergency file and the committed disabled group. We should probably consider dropping that entry to reduce confusion. Though I think we check DISABLE-ANSIBLE in the pre-run so we should never get here if we\u0027ve disabled ansible at least. But the two disable methods are sort of orthogonal to each other. One is host specific the other attempts to be global.\n\nI have removed the !disabled in playbooks/bootstrap-bridge.yaml -- I agree this is running outside a context where the \"disabled\" group is a thing (by design, now).   I think that this makes sense, as it is bootstrapping the inventory.\n\n\u003e All roles run from bootstrap-bridge.yaml need to be available in the zuul job context. I believe they are because the roles in that playbook are in the playbooks/roles/ dir adjacent to this playbook.\n\nThis is my theory too -- although running it will tell.\n \n\u003e Finally, the thing that concerns me most is the execution of the install-ansible role. We pass a number of parameters to it with defaults. Since we are in a zuul executor context here those values would need to be set at a Zuul level and not a bastion level. We should double check that we aren\u0027t overriding any of these defaults because if we are running this playbook from Zuul will suddenly undo them.\n\nWe are not setting any of the variables that influence this in /etc/ansible on the bastion host.  We shouldn\u0027t be either -- that would mean that we are possibly installing a different ansible on the production host than we test with.  These version variables should only be defined in the source (with provision for overriding by the -devel job)","commit_id":"70a5f52cf8b2a0758a84d8e0e4166908246f96f3"}],"zuul.d/project.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"8b3967f9c9f637069e4a383647949c4086482491","unresolved":true,"context_lines":[{"line_number":362,"context_line":"        - infra-prod-base: \u0026infra-prod-base"},{"line_number":363,"context_line":"            dependencies:"},{"line_number":364,"context_line":"              - name: infra-prod-bootstrap-bridge"},{"line_number":365,"context_line":"                soft: true"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        # Legacy puppet hosts"},{"line_number":368,"context_line":"        - infra-prod-remote-puppet-else: \u0026infra-prod-remote-puppet-else"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"36752ca8_16cdbd11","line":365,"range":{"start_line":365,"start_character":16,"end_line":365,"end_character":26},"updated":"2021-12-03 16:05:04.000000000","message":"I think that we need to make this a hard dependency. This is less important now that we\u0027ve removed the file matchers and I can probably be convinced to test it with soft deps as a result. But I think the reality is it is a hard dep and it would be better to capture it that way?","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"eb7aa55df20627df9b80a76639b6494838a0ec54","unresolved":false,"context_lines":[{"line_number":362,"context_line":"        - infra-prod-base: \u0026infra-prod-base"},{"line_number":363,"context_line":"            dependencies:"},{"line_number":364,"context_line":"              - name: infra-prod-bootstrap-bridge"},{"line_number":365,"context_line":"                soft: true"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        # Legacy puppet hosts"},{"line_number":368,"context_line":"        - infra-prod-remote-puppet-else: \u0026infra-prod-remote-puppet-else"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"563ea227_c4113a2b","line":365,"range":{"start_line":365,"start_character":16,"end_line":365,"end_character":26},"in_reply_to":"36752ca8_16cdbd11","updated":"2021-12-07 05:45:41.000000000","message":"Done","commit_id":"342e95c58b0cc9377e4bae8f9f9aa5620d3dee13"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"1d1dc8e97dec3814e6db24dd74c993c53a9a31e7","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        - infra-prod-remote-puppet-else: \u0026infra-prod-remote-puppet-else"},{"line_number":370,"context_line":"            dependencies:"},{"line_number":371,"context_line":"              - name: infra-prod-base"},{"line_number":372,"context_line":"                soft: true"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        #"},{"line_number":375,"context_line":"        # Only depends on base, or amongst themselves."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9df4e4be_e3a136cc","line":372,"updated":"2021-12-07 22:50:07.000000000","message":"Does the transitive soft -\u003e hard dependency here do what we want if infra-prod-base doesn\u0027t run? I half suspect it doesn\u0027t but I guess we can figure that out after this change lands.","commit_id":"70a5f52cf8b2a0758a84d8e0e4166908246f96f3"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"1a8db12dfb825f1baf2457c1a9811901fb41e1f2","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        - infra-prod-remote-puppet-else: \u0026infra-prod-remote-puppet-else"},{"line_number":370,"context_line":"            dependencies:"},{"line_number":371,"context_line":"              - name: infra-prod-base"},{"line_number":372,"context_line":"                soft: true"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"        #"},{"line_number":375,"context_line":"        # Only depends on base, or amongst themselves."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"da1115f4_bdbf0864","line":372,"in_reply_to":"9df4e4be_e3a136cc","updated":"2021-12-07 23:31:27.000000000","message":"I feel like it should.  Another thing to keep an eye on","commit_id":"70a5f52cf8b2a0758a84d8e0e4166908246f96f3"}]}