)]}'
{"specs/approved/no-conductor-to-ipa-communication.rst":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"NOTE: I\u0027d like to adapt the below story to match this spec, is there any"},{"line_number":12,"context_line":"objection?"},{"line_number":13,"context_line":"https://storyboard.openstack.org/#!/story/2007765"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This spec intends to make Ironic conductor to IPA API communication optional,"},{"line_number":16,"context_line":"instead using heartbeats to coordinate communication between the conductor and"}],"source_content_type":"text/x-rst","patch_set":1,"id":"8f91d8d6_c83696c0","line":13,"updated":"2021-02-23 19:04:27.000000000","message":"i think that is reasonable; ping Dmitry about it.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"626bf098d4c4e0728d416baaac0c418c3cbde606","unresolved":true,"context_lines":[{"line_number":35,"context_line":"* A tenant network, which can communicate with the outside world, and other"},{"line_number":36,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"However, this doesn\u0027t scale with medium to mega scale deployers who leverage"},{"line_number":39,"context_line":"layer 3 network topologies. In L3 networks a subnet is constrained to a single"},{"line_number":40,"context_line":"rack. This means that to leverage two networks to image hosts one would need"},{"line_number":41,"context_line":"to provision a second subnet for every single rack."}],"source_content_type":"text/x-rst","patch_set":1,"id":"cf8b85f5_3fc74c90","line":38,"updated":"2021-02-23 19:04:15.000000000","message":"\"medium to mega scale\" implies these terms are defined and meaningful.\n\nI suggest \"large scale\", which is less wordy and matches the original intent.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":false,"context_lines":[{"line_number":35,"context_line":"* A tenant network, which can communicate with the outside world, and other"},{"line_number":36,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"However, this doesn\u0027t scale with medium to mega scale deployers who leverage"},{"line_number":39,"context_line":"layer 3 network topologies. In L3 networks a subnet is constrained to a single"},{"line_number":40,"context_line":"rack. This means that to leverage two networks to image hosts one would need"},{"line_number":41,"context_line":"to provision a second subnet for every single rack."}],"source_content_type":"text/x-rst","patch_set":1,"id":"64d6b447_abe70f01","line":38,"in_reply_to":"cf8b85f5_3fc74c90","updated":"2021-02-25 16:32:58.000000000","message":"Done","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"38d2a874892f7087a4481b7d89474130bf7b5379","unresolved":true,"context_lines":[{"line_number":36,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"However, this doesn\u0027t scale with medium to mega scale deployers who leverage"},{"line_number":39,"context_line":"layer 3 network topologies. In L3 networks a subnet is constrained to a single"},{"line_number":40,"context_line":"rack. This means that to leverage two networks to image hosts one would need"},{"line_number":41,"context_line":"to provision a second subnet for every single rack."},{"line_number":42,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"a5629c18_f51a3da1","line":39,"updated":"2021-02-23 19:18:31.000000000","message":"To be blunt, I think the term \"layer 3 network\" with the meaning implied in this paragraph is a Verizon Media-internal colloquialism/slang. I also don\u0027t think the \"layer 3 networks\" label is useful in this context either. \n\nInstead, you could more properly explain this as \"Ironic may be deployed on a physical network where creating a \u0027provisioning network segment\u0027 is difficult. [..]\"","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":false,"context_lines":[{"line_number":36,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"However, this doesn\u0027t scale with medium to mega scale deployers who leverage"},{"line_number":39,"context_line":"layer 3 network topologies. In L3 networks a subnet is constrained to a single"},{"line_number":40,"context_line":"rack. This means that to leverage two networks to image hosts one would need"},{"line_number":41,"context_line":"to provision a second subnet for every single rack."},{"line_number":42,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"b00160e2_5dfbe196","line":39,"in_reply_to":"a5629c18_f51a3da1","updated":"2021-02-25 16:32:58.000000000","message":"Done","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":49,"context_line":"change proposes changes to the IPA and Ironic conductor communication flow such"},{"line_number":50,"context_line":"that a node can be provisioned and cleaned with IPA without having to permit any"},{"line_number":51,"context_line":"inbound traffic from an Ironic conductor to the target nodes."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"0318a783_b1e49f9e","line":52,"updated":"2021-02-23 19:04:27.000000000","message":"Don\u0027t know if it is worth mentioning (or maybe you have elsewhere) but it seems more efficient to use the heartbeat API to provide more info than just heartbeat. That will reduce the number of API requests going back/forth. (the \u0027unnecessary traffic\u0027 mentioned in the RFE story).","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":false,"context_lines":[{"line_number":49,"context_line":"change proposes changes to the IPA and Ironic conductor communication flow such"},{"line_number":50,"context_line":"that a node can be provisioned and cleaned with IPA without having to permit any"},{"line_number":51,"context_line":"inbound traffic from an Ironic conductor to the target nodes."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"b5437a42_0c9ed1bd","line":52,"in_reply_to":"0318a783_b1e49f9e","updated":"2021-02-25 16:32:58.000000000","message":"Done","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"929d56d521315938affffafa353d165f5a0ad692","unresolved":true,"context_lines":[{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"}],"source_content_type":"text/x-rst","patch_set":1,"id":"39203e41_e9631e0f","line":56,"updated":"2021-02-23 17:50:58.000000000","message":"I think calling this \"disable-ipa-api\" is misleading. Why not an enum like\n\nipa-control-type: heartbeat\nipa-control-type: ipa-api","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"ec1170cc49a7ff72e36444ce3a68b8369f7af4bd","unresolved":true,"context_lines":[{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"}],"source_content_type":"text/x-rst","patch_set":1,"id":"f23f7707_f9e2fed2","line":56,"in_reply_to":"39203e41_e9631e0f","updated":"2021-02-23 19:02:07.000000000","message":"This is a good idea. Then, later if/when we implement no-ipa-to-conductor communication, we could set it to \"poll-only\".","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":true,"context_lines":[{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"}],"source_content_type":"text/x-rst","patch_set":1,"id":"214e6cb9_4d7dba60","line":56,"in_reply_to":"95d39ece_6598354e","updated":"2021-02-25 16:32:58.000000000","message":"I\u0027ve addressed this in my upcoming changeset, I think.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"38bd2dc12d94ac353b04b5805ca67b6e3f34c69d","unresolved":true,"context_lines":[{"line_number":53,"context_line":"Proposed change"},{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"}],"source_content_type":"text/x-rst","patch_set":1,"id":"95d39ece_6598354e","line":56,"in_reply_to":"f23f7707_f9e2fed2","updated":"2021-02-25 14:48:56.000000000","message":"hmm. poll is relative to something but if the description is clear, \u0027poll-only\u0027 might work. Seems like we\u0027d have 3? states: poll-only, push-only, bi-directional? Are we trying to describe how ironic \u0026 ipa communicate?","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"},{"line_number":60,"context_line":"conductor/IPA communication flow with a new unidirectional flow. First, Ironic"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4d835cb1_b1d7345d","line":57,"updated":"2021-02-23 19:04:27.000000000","message":"i prefer to avoid \u0027disable\u0027 due to possible double negatives. How about \u0027ipa_api\u0027 or \u0027ipa_api_enable\u0027. \n\nTo Zachary\u0027s comment. I prefer not to hardcode \u0027heartbeat\u0027 in case we add some other ironic-API that IPA can use in addition to the heartbeat one.\n\neg, we add a \u0027status\u0027 one that might be issued immediately when a deploy/clean step is done or when the entire operation is done, instead of waiting for the next heartbeat to convey that info. (fwiw, i just looked, default for ramdisk_heartbeat_timeout is 300 secs, so 5 min. hmm, i\u0027d like a new ironic API that ipa can use :D)","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"38bd2dc12d94ac353b04b5805ca67b6e3f34c69d","unresolved":true,"context_lines":[{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"},{"line_number":60,"context_line":"conductor/IPA communication flow with a new unidirectional flow. First, Ironic"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5e238d3c_05fe115b","line":57,"in_reply_to":"4d835cb1_b1d7345d","updated":"2021-02-25 14:48:56.000000000","message":"Ugh. Jay explained to me that \u0027heartbeat\u0027 is badly named. That API is used for heartbeat AND for IPA to contact ironic after it has finished executing a command (or clean/deploy step). Probably too late to rename it...","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":false,"context_lines":[{"line_number":54,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"We will add an option, ``[agent]/disable-ipa-api``, to Ironic and the option"},{"line_number":57,"context_line":"``ipa-disable-api`` to IPA."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If ``[agent]/disable-ipa-api`` is enabled, we will replace the typical two-way"},{"line_number":60,"context_line":"conductor/IPA communication flow with a new unidirectional flow. First, Ironic"}],"source_content_type":"text/x-rst","patch_set":1,"id":"32cadbe8_9993410f","line":57,"in_reply_to":"5e238d3c_05fe115b","updated":"2021-02-25 16:32:58.000000000","message":"Done","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":68,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":69,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"},{"line_number":70,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":71,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":72,"context_line":"3. When IPA is started with ``ipa-disable-ipa-api\u003dTrue`` and receives a"},{"line_number":73,"context_line":"   heartbeat response containing a ``next_command``, IPA will begin executing"},{"line_number":74,"context_line":"   the command just as if it was received over the IPA ``PUT /v1/commands``"}],"source_content_type":"text/x-rst","patch_set":1,"id":"e3239f7f_5ebfffd3","line":71,"updated":"2021-02-23 19:04:27.000000000","message":"is this the same or similar info that the conductor would have done if it had made an IPA request (PUT /v1/commands)?","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":true,"context_lines":[{"line_number":68,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":69,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"},{"line_number":70,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":71,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":72,"context_line":"3. When IPA is started with ``ipa-disable-ipa-api\u003dTrue`` and receives a"},{"line_number":73,"context_line":"   heartbeat response containing a ``next_command``, IPA will begin executing"},{"line_number":74,"context_line":"   the command just as if it was received over the IPA ``PUT /v1/commands``"}],"source_content_type":"text/x-rst","patch_set":1,"id":"b6ef187f_fdd1f53b","line":71,"in_reply_to":"e3239f7f_5ebfffd3","updated":"2021-02-25 16:32:58.000000000","message":"My intent is for it to be as identical as possible.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":78,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":79,"context_line":"   API endpoint."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"Alternatives"},{"line_number":82,"context_line":"------------"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"edcd8682_a0480c31","line":80,"updated":"2021-02-23 19:04:27.000000000","message":"this may be a dumb question. And i don\u0027t recall when this might happen, but I know that the conductor has code to check the agent version (that is in the heartbeat request) against the agent version stashed in internal_driver_info.as long as the version is the same or \u003e than the one that has this change, we\u0027re good. if it is lower, do we need to handle that case? Maybe it happens during an ironic upgrade? If there isn\u0027t anything in the heartbeat API request that the ironic conductor can 100% know if ipa thinks that its-api-is-disabled, then maybe we should add something...?","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":78,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":79,"context_line":"   API endpoint."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"Alternatives"},{"line_number":82,"context_line":"------------"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"c3134acf_40b806f2","line":80,"in_reply_to":"edcd8682_a0480c31","updated":"2021-02-25 16:32:58.000000000","message":"I believe I\u0027ve addressed this in the next change.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":85,"context_line":"the ramdisk establish a tunnel to an Ironic conductor\u0027s network before starting"},{"line_number":86,"context_line":"IPA. This is not ideal as it requires additional infrastructure setup and may"},{"line_number":87,"context_line":"have negative security implications."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Data model impact"},{"line_number":90,"context_line":"-----------------"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"e59bbebd_efdee4c9","line":88,"updated":"2021-02-23 19:04:27.000000000","message":"my suggestion of a new ironic API for ipa to convey info to the conductor is probably another alternative (wrt the one-way communication only). Not sure how much this would cut down on traffic but it\u0027ll address the first issue. And the bonus of speeding up the deploy/clean :)","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"38bd2dc12d94ac353b04b5805ca67b6e3f34c69d","unresolved":false,"context_lines":[{"line_number":85,"context_line":"the ramdisk establish a tunnel to an Ironic conductor\u0027s network before starting"},{"line_number":86,"context_line":"IPA. This is not ideal as it requires additional infrastructure setup and may"},{"line_number":87,"context_line":"have negative security implications."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Data model impact"},{"line_number":90,"context_line":"-----------------"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"4bfecd87_44072c60","line":88,"in_reply_to":"e59bbebd_efdee4c9","updated":"2021-02-25 14:48:56.000000000","message":"Welp, shows how much I know about IPA. Forget this.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":100,"context_line":"---------------"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"There are no structural changes to the rest API. However, we will be using"},{"line_number":103,"context_line":"the preexisting ``POST /v1/heartbeat`` endpoint with additional data passed."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"Client (CLI) impact"},{"line_number":106,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"e8df9947_d183770d","line":103,"updated":"2021-02-23 19:04:27.000000000","message":"and the response will be different too. Guess we\u0027ll need a microversion bump. (and an ipa version bump).\n\ndon\u0027t know if we tend to note it here, but we need to remember to add code to conductor to make sure ipa supports this option.i think ipa heartbeats include the agent version?","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"403ff497e82915e38301ce25d3766720e0cb7f78","unresolved":true,"context_lines":[{"line_number":100,"context_line":"---------------"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"There are no structural changes to the rest API. However, we will be using"},{"line_number":103,"context_line":"the preexisting ``POST /v1/heartbeat`` endpoint with additional data passed."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"Client (CLI) impact"},{"line_number":106,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"1741102a_f2c06363","line":103,"in_reply_to":"7a7087cc_8eb94a0e","updated":"2021-02-26 15:08:19.000000000","message":"i think, strictly speaking, if a REST API changes, a microversion bump is needed. So eg, if A uses existing microversion to send a heartbeat request, ironic will respond as it does now cuz the assumption is that the caller expects the current respons.\n\nif B uses new microversion to send heartbeat request, ironic will respond with new command/whatever in the response for the request, and B will expect that additional info cuz it specified the new microversion.\n\nhaving said that -- do we not do this cuz we know / think only IPA issues the heartbeat request to ironic?","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":true,"context_lines":[{"line_number":100,"context_line":"---------------"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"There are no structural changes to the rest API. However, we will be using"},{"line_number":103,"context_line":"the preexisting ``POST /v1/heartbeat`` endpoint with additional data passed."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"Client (CLI) impact"},{"line_number":106,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a7087cc_8eb94a0e","line":103,"in_reply_to":"e8df9947_d183770d","updated":"2021-02-25 16:32:58.000000000","message":"I\u0027m not certain this needs a microversion. For instance, the anaconda spec proposes additions to the request body to /v1/heartbeat without a microversion bump. Admittedly, I\u0027m not completely sure of all the cases we bump it, but for adding a field to both the post and response bodies it seems like overkill.","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"67f7c9ec40b805deb0b63c30c331b3f68ebcd533","unresolved":true,"context_lines":[{"line_number":120,"context_line":"RPC API impact"},{"line_number":121,"context_line":"--------------"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"None"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"Driver API impact"},{"line_number":126,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"ac0d5a51_c1b62d35","line":123,"updated":"2021-02-23 19:04:27.000000000","message":"there will be a version change here for the heartbeat request. Something like https://opendev.org/openstack/ironic/src/commit/6e0682377ce433e1f9e6acf863e2bf73728a75ae/ironic/conductor/rpcapi.py#L933-L935","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"b664c7adf2648ae705e87054bf62c10633f1d489","unresolved":false,"context_lines":[{"line_number":120,"context_line":"RPC API impact"},{"line_number":121,"context_line":"--------------"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"None"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"Driver API impact"},{"line_number":126,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"8233ee72_9e35ae1d","line":123,"in_reply_to":"ac0d5a51_c1b62d35","updated":"2021-02-25 16:32:58.000000000","message":"Done","commit_id":"edc594b97c576b5a50e55469593f3afed99e96cc"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f1bcc2f2210592753c54f4bb2e9383d8066e46a6","unresolved":true,"context_lines":[{"line_number":111,"context_line":"There are no structural changes to the rest API. However, we will be using"},{"line_number":112,"context_line":"the preexisting ``POST /v1/heartbeat`` endpoint with support for an additional"},{"line_number":113,"context_line":"field: ``command_results`` and updating the response body to include"},{"line_number":114,"context_line":"``next-command``."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Client (CLI) impact"},{"line_number":117,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"96bc32cc_8dbd2c61","line":114,"updated":"2021-02-25 22:52:54.000000000","message":"i suspect we ought to bump the microversion...\nhow/what microversion does IPA use when it sends a heartbeat request to ironic?","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"863f6f1f1a692ecf2b61ae8177a490ff0c0ed3fb","unresolved":true,"context_lines":[{"line_number":111,"context_line":"There are no structural changes to the rest API. However, we will be using"},{"line_number":112,"context_line":"the preexisting ``POST /v1/heartbeat`` endpoint with support for an additional"},{"line_number":113,"context_line":"field: ``command_results`` and updating the response body to include"},{"line_number":114,"context_line":"``next-command``."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Client (CLI) impact"},{"line_number":117,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"95fab2ab_cf36de4b","line":114,"in_reply_to":"96bc32cc_8dbd2c61","updated":"2021-02-25 22:57:04.000000000","message":"https://opendev.org/openstack/ironic-python-agent/src/branch/master/ironic_python_agent/ironic_api_client.py#L32\n\nOne look at this makes it clear I should bump microversion, so I can gate support for this config variable on it. I\u0027ll update the spec to reflect that.\n\nIMO; it doesn\u0027t matter that we\u0027re changing the return/post bodies, but IPA needs the microversion bump to detect for support.","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f1bcc2f2210592753c54f4bb2e9383d8066e46a6","unresolved":true,"context_lines":[{"line_number":177,"context_line":"Recap of configuration options added:"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"Ironic adds:"},{"line_number":180,"context_line":" - ``[agent]/communication-style`` (type\u003denumerated, default\u003dFalse):"},{"line_number":181,"context_line":"    * ``bidirectional`` - default, maintains existing behavior"},{"line_number":182,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"2008d63b_e54f9dca","line":180,"updated":"2021-02-25 22:52:54.000000000","message":"isn\u0027t it default\u003dbidirectional ?","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"863f6f1f1a692ecf2b61ae8177a490ff0c0ed3fb","unresolved":false,"context_lines":[{"line_number":177,"context_line":"Recap of configuration options added:"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"Ironic adds:"},{"line_number":180,"context_line":" - ``[agent]/communication-style`` (type\u003denumerated, default\u003dFalse):"},{"line_number":181,"context_line":"    * ``bidirectional`` - default, maintains existing behavior"},{"line_number":182,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"c3ba829e_ff670446","line":180,"in_reply_to":"2008d63b_e54f9dca","updated":"2021-02-25 22:57:04.000000000","message":"Done","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f1bcc2f2210592753c54f4bb2e9383d8066e46a6","unresolved":true,"context_lines":[{"line_number":182,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Ironic Python Agent adds:"},{"line_number":185,"context_line":" - ``[default]/communication-style`` (type\u003denumerated, default\u003dFalse)"},{"line_number":186,"context_line":"    * exposed to kernel command line as ``ipa-communication-style``"},{"line_number":187,"context_line":"    * ``bidirectional`` - default, maintains existing behavior"},{"line_number":188,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"}],"source_content_type":"text/x-rst","patch_set":4,"id":"c5754591_4751297e","line":185,"updated":"2021-02-25 22:52:54.000000000","message":"same question about default\u003dFalse","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"863f6f1f1a692ecf2b61ae8177a490ff0c0ed3fb","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Ironic Python Agent adds:"},{"line_number":185,"context_line":" - ``[default]/communication-style`` (type\u003denumerated, default\u003dFalse)"},{"line_number":186,"context_line":"    * exposed to kernel command line as ``ipa-communication-style``"},{"line_number":187,"context_line":"    * ``bidirectional`` - default, maintains existing behavior"},{"line_number":188,"context_line":"    * ``heartbeat-only`` - enables heartbeat-based unidirectional communication"}],"source_content_type":"text/x-rst","patch_set":4,"id":"c9d5bd17_dd73247e","line":185,"in_reply_to":"c5754591_4751297e","updated":"2021-02-25 22:57:04.000000000","message":"Done","commit_id":"aae40c334a5eda0a887b36157951babdd1955c54"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"630b37d8e239667f9f0bf87e854a7ae18993dc8d","unresolved":true,"context_lines":[{"line_number":33,"context_line":"* A tenant network, which can communicate with the outside world, and other"},{"line_number":34,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Ironic may be deployed on a physical network where creating a dedicated"},{"line_number":37,"context_line":"provisioning network segment is difficult or impossible. Such deployers are put"},{"line_number":38,"context_line":"in an untenable position: they must grant access to the port the IPA API is"},{"line_number":39,"context_line":"available on from the Ironic conductor, which also, after provisioning, would"}],"source_content_type":"text/x-rst","patch_set":5,"id":"6547b43f_a4f60d43","line":36,"range":{"start_line":36,"start_character":51,"end_line":36,"end_character":1},"updated":"2021-03-01 16:03:08.000000000","message":"Nit: this is still a prescription of a solution (\"creating a provisioning network segment\"), and somewhat misses describing the actual problem: the presence of network filtering between Ironic and the target nodes.\n\n(It\u0027s also a bit handwavy/non-implicit, given the Verizon Media context, on how a \"provisioning network segment\" would avoid the same network filtering issues as the network segments that currently exist. This psuedo-solutioning does not positively contribute to a well-formed problem statement.)\n\nI rewriting the first sentence to more directly describe the problem.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"0f65066ddbab371a3f401be372d4b93131482a19","unresolved":true,"context_lines":[{"line_number":33,"context_line":"* A tenant network, which can communicate with the outside world, and other"},{"line_number":34,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Ironic may be deployed on a physical network where creating a dedicated"},{"line_number":37,"context_line":"provisioning network segment is difficult or impossible. Such deployers are put"},{"line_number":38,"context_line":"in an untenable position: they must grant access to the port the IPA API is"},{"line_number":39,"context_line":"available on from the Ironic conductor, which also, after provisioning, would"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a691c918_4861d17a","line":36,"range":{"start_line":36,"start_character":51,"end_line":36,"end_character":1},"in_reply_to":"1f16f740_853df85c","updated":"2021-03-02 17:02:04.000000000","message":"I think I might have addressed your concerns but honestly, overall, I think this description does a good enough job of justifying this change that I\u0027m going to focus on the solution portions.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"9773e8ea5ce7a7fb8c50defb08e22e50d061b282","unresolved":true,"context_lines":[{"line_number":33,"context_line":"* A tenant network, which can communicate with the outside world, and other"},{"line_number":34,"context_line":"  hosts, but cannot contact the Ironic API."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Ironic may be deployed on a physical network where creating a dedicated"},{"line_number":37,"context_line":"provisioning network segment is difficult or impossible. Such deployers are put"},{"line_number":38,"context_line":"in an untenable position: they must grant access to the port the IPA API is"},{"line_number":39,"context_line":"available on from the Ironic conductor, which also, after provisioning, would"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1f16f740_853df85c","line":36,"range":{"start_line":36,"start_character":51,"end_line":36,"end_character":1},"in_reply_to":"6547b43f_a4f60d43","updated":"2021-03-01 16:19:15.000000000","message":"My mistake--not just the first sentence starting on line 36. This comment affects line 24-34 as well.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"1c846cf270ab5561887817d6d8ecdf41ce3a5f36","unresolved":true,"context_lines":[{"line_number":39,"context_line":"available on from the Ironic conductor, which also, after provisioning, would"},{"line_number":40,"context_line":"give the Ironic conductor access to the workload deployed onto the node."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"To remediate this issue, we need to attempt to reduce the attack surface. This"},{"line_number":43,"context_line":"change proposes changes to the IPA and Ironic conductor communication flow such"},{"line_number":44,"context_line":"that a node can be provisioned and cleaned with IPA without having to permit"},{"line_number":45,"context_line":"any inbound traffic from an Ironic conductor to the target nodes."}],"source_content_type":"text/x-rst","patch_set":5,"id":"7d16233c_13141f54","line":42,"range":{"start_line":42,"start_character":25,"end_line":42,"end_character":72},"updated":"2021-03-01 15:46:25.000000000","message":"I think this may give too much emphasis on this being a \"security\"-slanted issue, rather than a  technical limitation.\n\nI suggest removing this entire sentence. The remaining text is a sufficient and more politically neutral explanation.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"0f65066ddbab371a3f401be372d4b93131482a19","unresolved":false,"context_lines":[{"line_number":39,"context_line":"available on from the Ironic conductor, which also, after provisioning, would"},{"line_number":40,"context_line":"give the Ironic conductor access to the workload deployed onto the node."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"To remediate this issue, we need to attempt to reduce the attack surface. This"},{"line_number":43,"context_line":"change proposes changes to the IPA and Ironic conductor communication flow such"},{"line_number":44,"context_line":"that a node can be provisioned and cleaned with IPA without having to permit"},{"line_number":45,"context_line":"any inbound traffic from an Ironic conductor to the target nodes."}],"source_content_type":"text/x-rst","patch_set":5,"id":"543954ee_4b49b4a1","line":42,"range":{"start_line":42,"start_character":25,"end_line":42,"end_character":72},"in_reply_to":"7d16233c_13141f54","updated":"2021-03-02 17:02:04.000000000","message":"Done","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"403ff497e82915e38301ce25d3766720e0cb7f78","unresolved":true,"context_lines":[{"line_number":53,"context_line":"We will add an enumerated config option, ``[agent]/communication-style``, to"},{"line_number":54,"context_line":"Ironic. It will default to ``bidirectional`` which will maintain existing"},{"line_number":55,"context_line":"behavior. If set to ``heartbeat-only``, it will use the new communication"},{"line_number":56,"context_line":"method proposed here. When this spec is approved, the no-ipa-to-conductor"},{"line_number":57,"context_line":"communication spec will be updated to use this same config option, with the"},{"line_number":58,"context_line":"value ``poll-only``."},{"line_number":59,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"fe7d5d15_6e025338","line":56,"updated":"2021-02-26 15:08:19.000000000","message":"not a big deal but maybe add a link to this spec?","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"19a64e9b8d315c2241c663e3f8ea7456857b794f","unresolved":true,"context_lines":[{"line_number":53,"context_line":"We will add an enumerated config option, ``[agent]/communication-style``, to"},{"line_number":54,"context_line":"Ironic. It will default to ``bidirectional`` which will maintain existing"},{"line_number":55,"context_line":"behavior. If set to ``heartbeat-only``, it will use the new communication"},{"line_number":56,"context_line":"method proposed here. When this spec is approved, the no-ipa-to-conductor"},{"line_number":57,"context_line":"communication spec will be updated to use this same config option, with the"},{"line_number":58,"context_line":"value ``poll-only``."},{"line_number":59,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"b9df48c0_a9fbd931","line":56,"in_reply_to":"fe7d5d15_6e025338","updated":"2021-02-26 15:45:07.000000000","message":"My thought was as long as this is accepted, I\u0027ll push a patchset that includes edits to that spec to reflect these changes and stop mentioning them here. :)","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"403ff497e82915e38301ce25d3766720e0cb7f78","unresolved":true,"context_lines":[{"line_number":66,"context_line":"convert to the following:"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"1. Ironic Python Agent, on startup, sees"},{"line_number":69,"context_line":"   ``ipa-communication-style\u003dheartbeat-only``, and as such does not start"},{"line_number":70,"context_line":"   the API listener but still performs a lookup as usual."},{"line_number":71,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":72,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"}],"source_content_type":"text/x-rst","patch_set":5,"id":"306f2802_04107215","line":69,"updated":"2021-02-26 15:08:19.000000000","message":"nit I think. IPA looks at the [default]/communication-style configuration option vs \u0027ipa-communication-style\u0027 from the kernel command line?","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"19a64e9b8d315c2241c663e3f8ea7456857b794f","unresolved":true,"context_lines":[{"line_number":66,"context_line":"convert to the following:"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"1. Ironic Python Agent, on startup, sees"},{"line_number":69,"context_line":"   ``ipa-communication-style\u003dheartbeat-only``, and as such does not start"},{"line_number":70,"context_line":"   the API listener but still performs a lookup as usual."},{"line_number":71,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":72,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d22ceb07_41d52cc7","line":69,"in_reply_to":"306f2802_04107215","updated":"2021-02-26 15:45:07.000000000","message":"This is generally how IPA configs work. \n\nhttps://opendev.org/openstack/ironic-python-agent/src/branch/master/ironic_python_agent/config.py#L31\n\nThe convention is for `config-value` it\u0027ll be exposed on the kernel command line as `ipa-config-value`. So both of these are true: we\u0027ll have it setup as [default]/communication-style in oslo.config, but it\u0027ll be set to look for ipa-communication-style in the kernel command line.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":12640,"name":"Arun S A G","email":"sagarun@gmail.com","username":"sagarun"},"change_message_id":"4a7f9b477b6afcc8347f1e317c668a9765f52c2b","unresolved":true,"context_lines":[{"line_number":70,"context_line":"   the API listener but still performs a lookup as usual."},{"line_number":71,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":72,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"},{"line_number":73,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":74,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":75,"context_line":"3. When IPA is started with ``ipa-communication-style\u003dheartbeat-only`` and"},{"line_number":76,"context_line":"   receives a heartbeat response containing a ``next_command``, IPA will begin"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f7a6f4c_bec76f57","line":73,"updated":"2021-03-01 19:15:36.000000000","message":"v1/heartbeat/uuid is a POST. AFAIK, heartbeat API POST does not really return anything. What you are describing is v1/lookup API.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"0f65066ddbab371a3f401be372d4b93131482a19","unresolved":true,"context_lines":[{"line_number":70,"context_line":"   the API listener but still performs a lookup as usual."},{"line_number":71,"context_line":"2. Ironic Python Agent heartbeats to the conductor. In the response to the"},{"line_number":72,"context_line":"   heartbeat, the Ironic conductor will return, in addition to the config"},{"line_number":73,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":74,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":75,"context_line":"3. When IPA is started with ``ipa-communication-style\u003dheartbeat-only`` and"},{"line_number":76,"context_line":"   receives a heartbeat response containing a ``next_command``, IPA will begin"}],"source_content_type":"text/x-rst","patch_set":5,"id":"79bdaf41_df06812d","line":73,"in_reply_to":"7f7a6f4c_bec76f57","updated":"2021-03-02 17:02:04.000000000","message":"Cleaned this up. Thanks for the comment.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"1c846cf270ab5561887817d6d8ecdf41ce3a5f36","unresolved":true,"context_lines":[{"line_number":73,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":74,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":75,"context_line":"3. When IPA is started with ``ipa-communication-style\u003dheartbeat-only`` and"},{"line_number":76,"context_line":"   receives a heartbeat response containing a ``next_command``, IPA will begin"},{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5334136f_2f84a545","line":76,"updated":"2021-03-01 15:46:25.000000000","message":"What if the conductor has multiple commands it would like IPA to start on the current heartbeat? Does that happen?","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"8a9000123c17c61a106a1767c56d48d3a60f89a5","unresolved":true,"context_lines":[{"line_number":73,"context_line":"   currently returned, a key ``next_command``, with a dict value in the format"},{"line_number":74,"context_line":"   ``agent_command_name: agent_command_params``."},{"line_number":75,"context_line":"3. When IPA is started with ``ipa-communication-style\u003dheartbeat-only`` and"},{"line_number":76,"context_line":"   receives a heartbeat response containing a ``next_command``, IPA will begin"},{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"48e2a801_4637b458","line":76,"in_reply_to":"5334136f_2f84a545","updated":"2021-03-01 16:00:05.000000000","message":"The IPA API theoretically supports multiple commands, but it\u0027s not used. In fact, there\u0027s a hardcoded part of the agent client library that only takes the most recent command result.\n\ntl;dr: it\u0027s theoretically possible with the existing API; but not used in current Ironic code","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":32592,"name":"Zachary Buhman","email":"zachary.buhman@verizonmedia.com"},"change_message_id":"1c846cf270ab5561887817d6d8ecdf41ce3a5f36","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"},{"line_number":80,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":81,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":82,"context_line":"   API endpoint."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"bc49ce9a_49b9abea","line":80,"updated":"2021-03-01 15:46:25.000000000","message":"Should IPA do the next heartbeat as soon as the command result is available, or possibly later, at the next regular heartbeat interval?","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"03e77808fa4b62557473de0bab885ce65fa947f3","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"},{"line_number":80,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":81,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":82,"context_line":"   API endpoint."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"ef675b5b_9a9043cb","line":80,"in_reply_to":"4f613172_126f2a7e","updated":"2021-03-02 16:31:12.000000000","message":"it might be worth adding to the spec (if it isn\u0027t there), a reminder that the \u0027heartbeat\u0027request is issued on a regular basis (\u0027heartbeat\u0027) and after IPA finishes a command. (Cuz zach and I didn\u0027t know that!)","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"8a9000123c17c61a106a1767c56d48d3a60f89a5","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"},{"line_number":80,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":81,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":82,"context_line":"   API endpoint."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"4f613172_126f2a7e","line":80,"in_reply_to":"bc49ce9a_49b9abea","updated":"2021-03-01 16:00:05.000000000","message":"IPA already heartbeats immediately after a command is completed. This design will not change that behavior.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"0f65066ddbab371a3f401be372d4b93131482a19","unresolved":true,"context_lines":[{"line_number":77,"context_line":"   executing the command just as if it was received over the IPA"},{"line_number":78,"context_line":"   ``PUT /v1/commands`` API endpoint."},{"line_number":79,"context_line":"4. Next heartbeat, IPA will post an additional key with its payload:"},{"line_number":80,"context_line":"   ``command_results``. This will return a list of command results in the same"},{"line_number":81,"context_line":"   format they would be returned if received from the IPA ``GET /v1/commands``"},{"line_number":82,"context_line":"   API endpoint."},{"line_number":83,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"c1789dda_70221679","line":80,"in_reply_to":"ef675b5b_9a9043cb","updated":"2021-03-02 17:02:04.000000000","message":"Added a paragraph about existing heartbeat behavior.","commit_id":"3a40c5c839ce57d42e0c0913ee09b5a312123cc8"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"05a1b79411a23e4f5185b8fdac029b801fca2a14","unresolved":true,"context_lines":[{"line_number":76,"context_line":"   the API listener but still performs a lookup as usual."},{"line_number":77,"context_line":"2. Ironic Python Agent heartbeats to the conductor with three additional keys"},{"line_number":78,"context_line":"   in the POST payload:"},{"line_number":79,"context_line":"     - ``clean_steps``, which includes the results of"},{"line_number":80,"context_line":"       ``clean.get_clean_steps`` from IPA"},{"line_number":81,"context_line":"     - ``deploy_steps``, which includes the results of"},{"line_number":82,"context_line":"       ``deploy.get_deploy_steps`` from IPA"}],"source_content_type":"text/x-rst","patch_set":8,"id":"ce757d72_f953a3ce","line":79,"updated":"2021-03-20 00:28:36.000000000","message":"reduce the indentation here and on L81 and L83 by 2 spaces","commit_id":"5407aa15e394f326fc7593a3a45f82b62f094661"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"05a1b79411a23e4f5185b8fdac029b801fca2a14","unresolved":true,"context_lines":[{"line_number":77,"context_line":"2. Ironic Python Agent heartbeats to the conductor with three additional keys"},{"line_number":78,"context_line":"   in the POST payload:"},{"line_number":79,"context_line":"     - ``clean_steps``, which includes the results of"},{"line_number":80,"context_line":"       ``clean.get_clean_steps`` from IPA"},{"line_number":81,"context_line":"     - ``deploy_steps``, which includes the results of"},{"line_number":82,"context_line":"       ``deploy.get_deploy_steps`` from IPA"},{"line_number":83,"context_line":"     - ``commands_results``, which includes the commands results as they would"}],"source_content_type":"text/x-rst","patch_set":8,"id":"c3ca10ea_953be8a0","line":80,"updated":"2021-03-20 00:28:36.000000000","message":"reduce the indentation here and on L82 and L84 by 4","commit_id":"5407aa15e394f326fc7593a3a45f82b62f094661"}]}