)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":36238,"name":"Hollie Hutchinson","display_name":"Hollie Hutchinson","email":"hollie@stackhpc.com","username":"assumptions"},"change_message_id":"a763f9d9a4032e6bfb9c2b1a0c1c7628564b4df9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"75a67ffe_85fef890","updated":"2024-02-01 15:06:04.000000000","message":"Precheck disabled as it\u0027s not finding any listening NTP servers.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"}],"playbooks/roles/bifrost-ironic-install/defaults/main.yml":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ae905fc328a639d2d5439642a558cdebaab47d76","unresolved":false,"context_lines":[{"line_number":403,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":404,"context_line":"ironic_enable_ntp_precheck: true"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"pxe_kernel_params_defaults:"},{"line_number":407,"context_line":"  - \"nofb\""},{"line_number":408,"context_line":"  - \"nomodeset\""},{"line_number":409,"context_line":"  - \"systemd.journald.forward_to_console\u003dyes\""}],"source_content_type":"text/x-yaml","patch_set":5,"id":"d4523e60_03ed2f56","line":406,"updated":"2023-12-15 11:08:01.000000000","message":"Unrelated change (that already caused a discussion on a different patch, so I\u0027d rather not repeat it here)","commit_id":"8fe075e5e174d57f20699afba87297e76d6425d3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"104fe67925ca9b4bd5f2deb2b2aecf27577697ad","unresolved":false,"context_lines":[{"line_number":403,"context_line":"# sensor data should be collected from undeployed nodes"},{"line_number":404,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"6ce16440_718973fb","line":406,"updated":"2024-04-15 15:58:59.000000000","message":"This is a wrong section to add user-facing variables.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"464e151ed76e55a8496529cb55f1f5187e7e857b","unresolved":true,"context_lines":[{"line_number":403,"context_line":"# sensor data should be collected from undeployed nodes"},{"line_number":404,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"a50afb09_6a81bbd3","line":406,"in_reply_to":"6ce16440_718973fb","updated":"2024-05-15 16:34:00.000000000","message":"When you say user-facing, are you referring to internal_ip?","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"7a1795eef862e250de8f687c81c48a5e899d63b3","unresolved":true,"context_lines":[{"line_number":403,"context_line":"# sensor data should be collected from undeployed nodes"},{"line_number":404,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"766718fe_512e4fd5","line":406,"in_reply_to":"7b28ebbd_9f5925cc","updated":"2024-05-16 12:40:19.000000000","message":"Ah, I missed that you were commenting on the \"Internal variables\" comment. In which case yes, I agree.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b9e2e6d955a4eb3f4c0ebb566135154f41f172fb","unresolved":true,"context_lines":[{"line_number":403,"context_line":"# sensor data should be collected from undeployed nodes"},{"line_number":404,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"7b28ebbd_9f5925cc","line":406,"in_reply_to":"a50afb09_6a81bbd3","updated":"2024-05-16 10:50:45.000000000","message":"I mean, isn\u0027t ironic_ntp_server a variable the operators want to override? If yes, why put it under \"Internal variables\"?","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"104fe67925ca9b4bd5f2deb2b2aecf27577697ad","unresolved":false,"context_lines":[{"line_number":404,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"pxe_kernel_params: \u003e-"}],"source_content_type":"text/x-yaml","patch_set":15,"id":"00ca3831_4a534b46","line":407,"updated":"2024-04-15 15:58:59.000000000","message":"This is not quite right for the most users? I\u0027d expect this value to be empty by default.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"104fe67925ca9b4bd5f2deb2b2aecf27577697ad","unresolved":false,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"pxe_kernel_params: \u003e-"},{"line_number":411,"context_line":"  nofb nomodeset systemd.journald.forward_to_console\u003dyes"}],"source_content_type":"text/x-yaml","patch_set":15,"id":"b22df64b_98f05c23","line":408,"updated":"2024-04-15 15:58:59.000000000","message":"This is not necessary a wrong feature, but it feels like it\u0027s addressing an edge case (NTP server on the same machine), and I think Bifrost already has too many options that are not used by most users. I\u0027d prefer this dropped and moved to your higher level playbooks.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"7a1795eef862e250de8f687c81c48a5e899d63b3","unresolved":true,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"pxe_kernel_params: \u003e-"},{"line_number":411,"context_line":"  nofb nomodeset systemd.journald.forward_to_console\u003dyes"}],"source_content_type":"text/x-yaml","patch_set":15,"id":"4b583470_de164825","line":408,"in_reply_to":"94c57a13_ef7931e1","updated":"2024-05-16 12:40:19.000000000","message":"Sure, and we should probably provide a flag to opt-out of the check. But in my experience not using an NTP server is likely to cause problems.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"464e151ed76e55a8496529cb55f1f5187e7e857b","unresolved":true,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"pxe_kernel_params: \u003e-"},{"line_number":411,"context_line":"  nofb nomodeset systemd.journald.forward_to_console\u003dyes"}],"source_content_type":"text/x-yaml","patch_set":15,"id":"c8e57d09_2c43d96a","line":408,"in_reply_to":"b22df64b_98f05c23","updated":"2024-05-15 16:34:00.000000000","message":"If we remove the default for ironic_ntp_server, should we add a check that it has been set? It\u0027s easy to forget and you\u0027re likely to hit issues without it.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b9e2e6d955a4eb3f4c0ebb566135154f41f172fb","unresolved":true,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"# Internal variables"},{"line_number":407,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":408,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"pxe_kernel_params: \u003e-"},{"line_number":411,"context_line":"  nofb nomodeset systemd.journald.forward_to_console\u003dyes"}],"source_content_type":"text/x-yaml","patch_set":15,"id":"94c57a13_ef7931e1","line":408,"in_reply_to":"c8e57d09_2c43d96a","updated":"2024-05-16 10:50:45.000000000","message":"Not all environments have an NTP server. May lab one probably does not.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"104fe67925ca9b4bd5f2deb2b2aecf27577697ad","unresolved":false,"context_lines":[{"line_number":415,"context_line":"  sshkey\u003d\"{{ ssh_public_key }}\""},{"line_number":416,"context_line":"  {% endif %}"},{"line_number":417,"context_line":"  {{ extra_kernel_options | default(\u0027\u0027) }}"},{"line_number":418,"context_line":"  {% if ironic_ntp_server | length %}ipa-ntp-server\u003d{{ ironic_ntp_server }}{% endif %}"},{"line_number":419,"context_line":"redfish_kernel_params: \"{{ pxe_kernel_params }}\""},{"line_number":420,"context_line":"ilo_kernel_params: \"{{ pxe_kernel_params }}\""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"cbe3cd97_869af00a","line":418,"updated":"2024-04-15 15:58:59.000000000","message":"The check is correct here, but the value is basically never empty.","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"b9e2e6d955a4eb3f4c0ebb566135154f41f172fb","unresolved":true,"context_lines":[{"line_number":415,"context_line":"  sshkey\u003d\"{{ ssh_public_key }}\""},{"line_number":416,"context_line":"  {% endif %}"},{"line_number":417,"context_line":"  {{ extra_kernel_options | default(\u0027\u0027) }}"},{"line_number":418,"context_line":"  {% if ironic_ntp_server | length %}ipa-ntp-server\u003d{{ ironic_ntp_server }}{% endif %}"},{"line_number":419,"context_line":"redfish_kernel_params: \"{{ pxe_kernel_params }}\""},{"line_number":420,"context_line":"ilo_kernel_params: \"{{ pxe_kernel_params }}\""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"1c2ed047_7a6225dc","line":418,"in_reply_to":"56b96797_949474e6","updated":"2024-05-16 10:50:45.000000000","message":"Correct","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"464e151ed76e55a8496529cb55f1f5187e7e857b","unresolved":true,"context_lines":[{"line_number":415,"context_line":"  sshkey\u003d\"{{ ssh_public_key }}\""},{"line_number":416,"context_line":"  {% endif %}"},{"line_number":417,"context_line":"  {{ extra_kernel_options | default(\u0027\u0027) }}"},{"line_number":418,"context_line":"  {% if ironic_ntp_server | length %}ipa-ntp-server\u003d{{ ironic_ntp_server }}{% endif %}"},{"line_number":419,"context_line":"redfish_kernel_params: \"{{ pxe_kernel_params }}\""},{"line_number":420,"context_line":"ilo_kernel_params: \"{{ pxe_kernel_params }}\""}],"source_content_type":"text/x-yaml","patch_set":15,"id":"56b96797_949474e6","line":418,"in_reply_to":"cbe3cd97_869af00a","updated":"2024-05-15 16:34:00.000000000","message":"Although it would be if the default for ironic_ntp_server were empty?","commit_id":"b3621f6dc9a7e83de880c50fe179889db9272976"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2166a1a1e7ab5f4ef4208cbaedd550aab0ff5b05","unresolved":false,"context_lines":[{"line_number":446,"context_line":"sensor_data_undeployed_nodes: false"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"# Specify NTP server for IPA"},{"line_number":449,"context_line":"ironic_ntp_server: \"{{ internal_ip }}\""},{"line_number":450,"context_line":"# Enable checking for a listening NTP server during Ironic install"},{"line_number":451,"context_line":"ironic_enable_ntp_precheck: false"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":16,"id":"10b8bd97_fb2def33","line":449,"updated":"2024-11-05 12:09:26.000000000","message":"Do we expect all users to have an NTP server locally already? If no, this is a breaking change and must at least be documented as such (both in the install docs and the release note). Or should we leave this empty by default?","commit_id":"a9386870a22b3cc937c9d994219b6b9825da8633"}],"playbooks/roles/bifrost-ironic-install/tasks/start.yml":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"18a94550667ae685ed4d5038b81e9b161de60c84","unresolved":true,"context_lines":[{"line_number":53,"context_line":"    - ironic-api"},{"line_number":54,"context_line":"    - ironic-conductor"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"- name: Checking active port for Ironic NTP server"},{"line_number":57,"context_line":"  wait_for:"},{"line_number":58,"context_line":"    host: \"{{ ironic_ntp_server }}\""},{"line_number":59,"context_line":"    port: \"123\""}],"source_content_type":"text/x-yaml","patch_set":5,"id":"887669d7_f0d1adc2","line":56,"updated":"2023-09-26 16:22:03.000000000","message":"Same issue as in kolla-ansible: NTP is UDP based.","commit_id":"8fe075e5e174d57f20699afba87297e76d6425d3"},{"author":{"_account_id":36238,"name":"Hollie Hutchinson","display_name":"Hollie Hutchinson","email":"hollie@stackhpc.com","username":"assumptions"},"change_message_id":"86ba6937f86e48de5732ba8526236e4840aff5fb","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    - ironic-api"},{"line_number":54,"context_line":"    - ironic-conductor"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"- name: Checking active port for Ironic NTP server"},{"line_number":57,"context_line":"  wait_for:"},{"line_number":58,"context_line":"    host: \"{{ ironic_ntp_server }}\""},{"line_number":59,"context_line":"    port: \"123\""}],"source_content_type":"text/x-yaml","patch_set":5,"id":"17125589_441f5bde","line":56,"in_reply_to":"887669d7_f0d1adc2","updated":"2023-12-22 14:34:42.000000000","message":"Removed wait_for","commit_id":"8fe075e5e174d57f20699afba87297e76d6425d3"}],"releasenotes/notes/bifrost-ntp-b9ff464ca13ad959.yaml":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"2166a1a1e7ab5f4ef4208cbaedd550aab0ff5b05","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    NTP configuration for the Ironic Python Agent (IPA) image added in this"},{"line_number":5,"context_line":"    patch. The NTP server can be set using the `ironic_ntp_server` variable"},{"line_number":6,"context_line":"    and NTP precheck can be disabled by setting `ironic_ntp_enable_precheck`"},{"line_number":7,"context_line":"    to false."}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7badc26a_8d335857","line":5,"updated":"2024-11-05 12:09:26.000000000","message":"\"can be set\" does not represent the whole truth: you also set it to internal_ip and expect the server to be there","commit_id":"a9386870a22b3cc937c9d994219b6b9825da8633"}]}