)]}'
{"specs/approved/node-traits.rst":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"An example use case is dedicating a pool of ironic nodes for a specific"},{"line_number":30,"context_line":"project. This can be done by marking all nodes for general use with a trait"},{"line_number":31,"context_line":"`GENERAL_USE` and other nodes tagged with `RESERVED_PROJECT_B`. All the public"},{"line_number":32,"context_line":"flavors are updated to require the `GENERAL_USE` trait and a new flavor is"},{"line_number":33,"context_line":"created for another tenant that is marked with `RESERVED_PROJECT_B`. With all"},{"line_number":34,"context_line":"this setup the appropriate ironic nodes will be picked when a server is booted"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_e46cb909","line":31,"range":{"start_line":31,"start_character":1,"end_line":31,"end_character":12},"updated":"2017-09-25 12:23:38.000000000","message":"CUSTOM_ prefix?  (Also see comment L150)","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":54,"context_line":"some well known traits to see if they are supported. In addition it may be"},{"line_number":55,"context_line":"that some drivers automatically report some traits. This has all been left"},{"line_number":56,"context_line":"till a future date."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Data model impact"},{"line_number":59,"context_line":"-----------------"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_041fd56c","line":57,"updated":"2017-09-25 12:23:38.000000000","message":"Seems like an obvious alternative would be *not* attaching new semantics/data to the node object itself, and instead driving all of this directly through the placement API via the resource class being reported by the node.\n\nSee comments below tagged with [ALT].","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b8e9a23e4b97819b5a86bfa883a95555bc5233bc","unresolved":false,"context_lines":[{"line_number":54,"context_line":"some well known traits to see if they are supported. In addition it may be"},{"line_number":55,"context_line":"that some drivers automatically report some traits. This has all been left"},{"line_number":56,"context_line":"till a future date."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Data model impact"},{"line_number":59,"context_line":"-----------------"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_0ba416a5","line":57,"in_reply_to":"7f515b1d_041fd56c","updated":"2017-09-28 09:52:25.000000000","message":"I need to add details around the inspector story, that would mean inspector needs to talk to placement.\n\nI cover this placement alternative in the Nova spec, just been working through all that myself, and I am totally warming to it.","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":65,"context_line":"This change superseeds the idea of tags, so the above tables should be removed"},{"line_number":66,"context_line":"and replaced with something like::"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    CREATE TABLE node_traits ("},{"line_number":69,"context_line":"        node_id INT(11) NOT NULL,"},{"line_number":70,"context_line":"        trait VARCHAR(255) CHARACTER SET utf8 NOT NULL,"},{"line_number":71,"context_line":"        PRIMARY KEY (node_id, trait),"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_e4139965","line":68,"updated":"2017-09-25 12:23:38.000000000","message":"[ALT] This would go away","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    )"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"A new ironic.objects.tags.NodeTraitList object will be added to the object"},{"line_number":79,"context_line":"model. Thtage ironic.objects.tags.NodeTraitList field in the python object model"},{"line_number":80,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"A tag should be defined in a way that matches the placement API definition,"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_24f09167","line":79,"range":{"start_line":79,"start_character":7,"end_line":79,"end_character":13},"updated":"2017-09-25 12:23:38.000000000","message":"kfleugh","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":104,"context_line":"traits associated with each node by extending the existing API in the following"},{"line_number":105,"context_line":"way ::"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    GET /v1/nodes/detail"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    {"},{"line_number":110,"context_line":"      \"nodes\": ["}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_6e22e7a6","line":107,"updated":"2017-09-25 12:23:38.000000000","message":"[ALT] This could exist, or not.  If not, an API consumer could just interact with the placement API, identifying the node\u0027s resource class, to GET/PUT/DELETE the node\u0027s traits.  If it does exist, it would be a thin passthrough to the placement API.","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":121,"context_line":"the placement API and API WG tags spec:"},{"line_number":122,"context_line":"https://developer.openstack.org/api-ref/placement/#resource-provider-traits"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"To start with there iwll be a new traits resource that follows the above"},{"line_number":125,"context_line":"patterns to get all and set all the traits::"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    GET /nodes/{node_ident}/traits"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_c4ee5d87","line":124,"range":{"start_line":124,"start_character":20,"end_line":124,"end_character":24},"updated":"2017-09-25 12:23:38.000000000","message":"will","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Filtering the node list by tags should work as expected::"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    GET /servers?traits\u003dred,blue"},{"line_number":151,"context_line":"    GET /servers?traits-any\u003dred,blue"},{"line_number":152,"context_line":"    GET /servers?not-traits\u003dred,blue"},{"line_number":153,"context_line":"    GET /servers?not-traits-any\u003dred,blue"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_049a3513","line":150,"range":{"start_line":150,"start_character":24,"end_line":150,"end_character":32},"updated":"2017-09-25 12:23:38.000000000","message":"implies case insensitive and implicit CUSTOM_ prefix.  Is that the case?","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":161,"context_line":"No changes, it is deprecated."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"\"openstack baremetal\" CLI"},{"line_number":164,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"* openstack baremetal node trait list"},{"line_number":167,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_6e70a7a6","line":164,"updated":"2017-09-25 12:23:38.000000000","message":"The below omit any identifier for the node; I assume that\u0027s because it\u0027s supposed to be really obvious how that\u0027s done?","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"fcbcf24c5c6a32bf654e9cc3658ccf1690ec5f71","unresolved":false,"context_lines":[{"line_number":163,"context_line":"\"openstack baremetal\" CLI"},{"line_number":164,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"* openstack baremetal node trait list"},{"line_number":167,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":168,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"RPC API impact"},{"line_number":171,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7f515b1d_2e418f3a","line":168,"range":{"start_line":166,"start_character":0,"end_line":168,"end_character":52},"updated":"2017-09-25 12:23:38.000000000","message":"[ALT] Right, so these would talk directly to the placement API (after first looking up the node\u0027s resource class ID, if necessary).","commit_id":"764212521b535fd21a8f63b0449e6c9d0ed62a22"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24abee04a119f56b3e0b15c0cbff5d6383ba7843","unresolved":false,"context_lines":[{"line_number":20,"context_line":"there are many use cases that need more granular control."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"An example use case is dedicating a pool of ironic nodes for a specific"},{"line_number":23,"context_line":"project. Another is allow flavors that best fit to the available hardware,"},{"line_number":24,"context_line":"you might want smaller flavors to build on larger machines if you are out of"},{"line_number":25,"context_line":"larger machines."},{"line_number":26,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_f53cd493","line":23,"range":{"start_line":23,"start_character":20,"end_line":23,"end_character":25},"updated":"2017-09-29 19:20:02.000000000","message":"allowing","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24abee04a119f56b3e0b15c0cbff5d6383ba7843","unresolved":false,"context_lines":[{"line_number":22,"context_line":"An example use case is dedicating a pool of ironic nodes for a specific"},{"line_number":23,"context_line":"project. Another is allow flavors that best fit to the available hardware,"},{"line_number":24,"context_line":"you might want smaller flavors to build on larger machines if you are out of"},{"line_number":25,"context_line":"larger machines."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"In a similar way, you might have a pool of hardware that are all the same"},{"line_number":28,"context_line":"except for the chosen RAID configuration. Longer term, when you provision the"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_b542dc0d","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":6},"updated":"2017-09-29 19:20:02.000000000","message":"smaller","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":59,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"},{"line_number":60,"context_line":"will be picked when a server is booted in Nova."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"TODO - ironic inspector needs to detect the traits, ideally."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"TODO - talk about ironic standalone and its use of traits."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_423eda29","line":62,"range":{"start_line":62,"start_character":0,"end_line":62,"end_character":60},"updated":"2017-10-05 20:30:46.000000000","message":"\"ironic inspector\" is software, not wetware?  I don\u0027t see how software is going to be able to designate things like \"project B\".  Unless you\u0027re talking about putting in yet *another* layer of abstraction where the operator tells *inspector* about the traits.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"},{"line_number":60,"context_line":"will be picked when a server is booted in Nova."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"TODO - ironic inspector needs to detect the traits, ideally."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"TODO - talk about ironic standalone and its use of traits."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_4192384d","line":62,"updated":"2017-10-06 10:30:14.000000000","message":"Well, we have such level of abstractions already :) However, that\u0027s not about traits for projects, but rather things like CPU flags. Inspector can detect them already. And Inspector can have operator-defined rules to set capabilities (in the future - traits).","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":59,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"},{"line_number":60,"context_line":"will be picked when a server is booted in Nova."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"TODO - ironic inspector needs to detect the traits, ideally."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"TODO - talk about ironic standalone and its use of traits."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_738f1ccd","line":62,"in_reply_to":"7f515b1d_4192384d","updated":"2017-10-09 09:14:52.000000000","message":"+1 what you are saying Dmitry, I just want to include that bit of the story in here.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24abee04a119f56b3e0b15c0cbff5d6383ba7843","unresolved":false,"context_lines":[{"line_number":75,"context_line":"Note when provisioning a node, Nova will send flavor extra_specs to Ironic and"},{"line_number":76,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":77,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":78,"context_line":"also include `/instance_info/traits` in a vary similar way. For completeness"},{"line_number":79,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":80,"context_line":"will be included in the Nova spec."},{"line_number":81,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_150890d7","line":78,"range":{"start_line":78,"start_character":42,"end_line":78,"end_character":46},"updated":"2017-09-29 19:20:02.000000000","message":"very","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":76,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":77,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":78,"context_line":"also include `/instance_info/traits` in a vary similar way. For completeness"},{"line_number":79,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":80,"context_line":"will be included in the Nova spec."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"TODO - how do we deal with merging traits? If the operator sets them in"},{"line_number":83,"context_line":"placement, should nova should overwrite them, otherwise how would it know when"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_cbcad744","line":80,"range":{"start_line":79,"start_character":0,"end_line":80,"end_character":34},"updated":"2017-10-05 20:30:46.000000000","message":"Not sure this is in the scope of the referenced nova spec.  I think it\u0027s on the topic of \"sending allocations to virt\", which may become the subject of a separate spec.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":79,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":80,"context_line":"will be included in the Nova spec."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"TODO - how do we deal with merging traits? If the operator sets them in"},{"line_number":83,"context_line":"placement, should nova should overwrite them, otherwise how would it know when"},{"line_number":84,"context_line":"to delete a trait when it is deleted in Ironic? Maybe we need to consider"},{"line_number":85,"context_line":"operators directly setting them in placement?"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"Alternatives"},{"line_number":88,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_8bedffb8","line":85,"range":{"start_line":82,"start_character":0,"end_line":85,"end_character":45},"updated":"2017-10-05 20:30:46.000000000","message":"Recent revs of the nova spec have declared Ironic to be the source of truth for the traits.  This question now becomes: how do you merge traits between what ironic inspector sets and what the operator sets?  Simplest is just not to do the former.  (I don\u0027t think not doing the latter is an option, as noted above.)","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":83,"context_line":"placement, should nova should overwrite them, otherwise how would it know when"},{"line_number":84,"context_line":"to delete a trait when it is deleted in Ironic? Maybe we need to consider"},{"line_number":85,"context_line":"operators directly setting them in placement?"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"Alternatives"},{"line_number":88,"context_line":"------------"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_6b9403e9","line":86,"updated":"2017-10-05 20:30:46.000000000","message":"Here\u0027s another thing to consider:\n\nWhen the operator (and/or inspector) adds a trait, that trait will have to be validated and added to placement if not already present.  You could do this on the Ironic side or on the nova virt side.  If the latter, Ironic doesn\u0027t need to talk directly to Placement at all, ever; BUT I have no idea how you would do validation.  If the former, now you\u0027ve got a hard dependency on Placement, with all that entails; but that\u0027s probably where the world is headed anyway, so is it a big deal?\n\n[Later] If L164 works, strike the \"no idea\" thing.  And I would go with that option if it were me.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":83,"context_line":"placement, should nova should overwrite them, otherwise how would it know when"},{"line_number":84,"context_line":"to delete a trait when it is deleted in Ironic? Maybe we need to consider"},{"line_number":85,"context_line":"operators directly setting them in placement?"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"Alternatives"},{"line_number":88,"context_line":"------------"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_3e4e0b6f","line":86,"in_reply_to":"7f515b1d_6b9403e9","updated":"2017-10-09 09:14:52.000000000","message":"I am thinking of skipping validation, and just using the bog standard rule, I should spell that all out.\n\nI will leave it to the virt driver to fix up the CUSTOM_TRAITs like we do for custom resource classes.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24abee04a119f56b3e0b15c0cbff5d6383ba7843","unresolved":false,"context_lines":[{"line_number":100,"context_line":"  ResourceProvider by name to find the Resource Provider uuid that matches the"},{"line_number":101,"context_line":"  Ironic Node uuid."},{"line_number":102,"context_line":"  Need to think about how Ironic Inspector would be able to set the traits on"},{"line_number":103,"context_line":"  the node is the correct way."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"* Keep traits APIs in Ironic, but make it a pass-through proxy to Placement."},{"line_number":106,"context_line":"  This is would make Ironic hard depend on placement, which would be a strange"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_f533b4a9","line":103,"range":{"start_line":103,"start_character":11,"end_line":103,"end_character":30},"updated":"2017-09-29 19:20:02.000000000","message":"?","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":100,"context_line":"  ResourceProvider by name to find the Resource Provider uuid that matches the"},{"line_number":101,"context_line":"  Ironic Node uuid."},{"line_number":102,"context_line":"  Need to think about how Ironic Inspector would be able to set the traits on"},{"line_number":103,"context_line":"  the node is the correct way."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"* Keep traits APIs in Ironic, but make it a pass-through proxy to Placement."},{"line_number":106,"context_line":"  This is would make Ironic hard depend on placement, which would be a strange"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_4b7b87f3","line":103,"range":{"start_line":103,"start_character":11,"end_line":103,"end_character":30},"in_reply_to":"7f515b1d_f533b4a9","updated":"2017-10-05 20:30:46.000000000","message":"*in* the correct way?","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"* Keep traits APIs in Ironic, but make it a pass-through proxy to Placement."},{"line_number":106,"context_line":"  This is would make Ironic hard depend on placement, which would be a strange"},{"line_number":107,"context_line":"  requirement for things like Bifrost."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"The big questions appear to include:"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_21997c6d","line":107,"updated":"2017-10-06 10:30:14.000000000","message":"and it may make upgrades funnier. e.g. now we require ironic to be upgraded first, so we\u0027ll have to ensure we don\u0027t fail completely, when placement is not upgraded yet. rolling upgrades can be even more funny..","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"* Keep traits APIs in Ironic, but make it a pass-through proxy to Placement."},{"line_number":106,"context_line":"  This is would make Ironic hard depend on placement, which would be a strange"},{"line_number":107,"context_line":"  requirement for things like Bifrost."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"The big questions appear to include:"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_fe53d314","line":107,"in_reply_to":"7f515b1d_21997c6d","updated":"2017-10-09 09:14:52.000000000","message":"good point!","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"The big questions appear to include:"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"* If traits replaces capabilities, then standalone ironic will need traits,"},{"line_number":112,"context_line":"  and ideally they should appear when looking at Node details."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Should Ironic hard depend on placement being present?"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_019c405b","line":111,"updated":"2017-10-06 10:30:14.000000000","message":"nit: replace","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":117,"context_line":"  required aggregation."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"* Should Operators talk directly to the placement API? We will need policy"},{"line_number":120,"context_line":"  improvements, etc."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"Data model impact"},{"line_number":123,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_e1a2449a","line":120,"updated":"2017-10-06 10:30:14.000000000","message":"I\u0027d avoid operators talking to placement for bare metal nodes.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":117,"context_line":"  required aggregation."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"* Should Operators talk directly to the placement API? We will need policy"},{"line_number":120,"context_line":"  improvements, etc."},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"Data model impact"},{"line_number":123,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_be41db3b","line":120,"in_reply_to":"7f515b1d_e1a2449a","updated":"2017-10-09 09:14:52.000000000","message":"++ I think that is where I have landed too.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":147,"context_line":"as a Unicode string no longer than 255 characters in length."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"TODO - placment has gone for a more efficient DB schmea, but this mirrors the"},{"line_number":150,"context_line":"ironic tag schema"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"State Machine Impact"},{"line_number":153,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_c19d0858","line":150,"updated":"2017-10-06 10:30:14.000000000","message":"I don\u0027t care about following the tags schema too much","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":162,"context_line":"must be first registered with placement before the ironic virt driver will be"},{"line_number":163,"context_line":"able to add the trait to the resource provider, the Ironic API will simply"},{"line_number":164,"context_line":"validate the trait is valid (either in the standard list or with the correct"},{"line_number":165,"context_line":"prefix), rather than checking any traits are correctly registered in placement."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":168,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_a1ac4c8c","line":165,"updated":"2017-10-06 10:30:14.000000000","message":"++","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":161,"context_line":"requires any other trait to start with the prefix of `CUSTOM_`. While any trait"},{"line_number":162,"context_line":"must be first registered with placement before the ironic virt driver will be"},{"line_number":163,"context_line":"able to add the trait to the resource provider, the Ironic API will simply"},{"line_number":164,"context_line":"validate the trait is valid (either in the standard list or with the correct"},{"line_number":165,"context_line":"prefix), rather than checking any traits are correctly registered in placement."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":168,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_cbabf71b","line":165,"range":{"start_line":164,"start_character":0,"end_line":165,"end_character":8},"updated":"2017-10-05 20:30:46.000000000","message":"Oh, so you could validate with just a dep on os-traits, not have to talk to placement at all.  That works.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6dbb75de9b4d27986d6b6466acbb772a3f4fbacd","unresolved":false,"context_lines":[{"line_number":162,"context_line":"must be first registered with placement before the ironic virt driver will be"},{"line_number":163,"context_line":"able to add the trait to the resource provider, the Ironic API will simply"},{"line_number":164,"context_line":"validate the trait is valid (either in the standard list or with the correct"},{"line_number":165,"context_line":"prefix), rather than checking any traits are correctly registered in placement."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":168,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_e477bdd3","line":165,"in_reply_to":"7f515b1d_a1ac4c8c","updated":"2017-10-09 11:52:11.000000000","message":"It occurred to me that this means virt and ironic need to be talking to the same version of os-traits.\n\nOr, I suppose, as long as placement \u003e\u003d virt \u003e\u003d ironic you\u0027d be okay.\n\nYou know, because a later os-traits might have added a trait, and you need to make sure that trait will exist as it travels up the chain.\n\nPerhaps this is what dtantsur was talking about on L107.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":168,"context_line":"traits associated with each node by extending the existing API in the following"},{"line_number":169,"context_line":"way ::"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    GET /v1/nodes/detail"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    {"},{"line_number":174,"context_line":"      \"nodes\": ["}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_81a710a8","line":171,"updated":"2017-10-06 10:30:14.000000000","message":"a tricky bit here: if we allow this, then I think we should also allow\n\n GET /v1/nodes/?fields\u003duuid,traits\n\nnova would benefit from this a lot","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":168,"context_line":"traits associated with each node by extending the existing API in the following"},{"line_number":169,"context_line":"way ::"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    GET /v1/nodes/detail"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    {"},{"line_number":174,"context_line":"      \"nodes\": ["}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_de7137aa","line":171,"in_reply_to":"7f515b1d_81a710a8","updated":"2017-10-09 09:14:52.000000000","message":"Oh, good point. I should add that.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":194,"context_line":"    }"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":197,"context_line":"    {"},{"line_number":198,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":199,"context_line":"    }"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_61b894c7","line":196,"updated":"2017-10-06 10:30:14.000000000","message":"We may easily end up with traits provided by drivers, I\u0027m not sure we want operators to overwrite them.. what will we do in that case?","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"de9df669b4742534dc0b7d110f3e9d3176d978d1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":194,"context_line":"    }"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":197,"context_line":"    {"},{"line_number":198,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":199,"context_line":"    }"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_791ccc69","line":196,"in_reply_to":"7f515b1d_61b894c7","updated":"2017-10-06 12:17:07.000000000","message":"To be anal, there\u0027s no such thing as \"overwrite\", since a trait is just a key with no value.  Realistically, we\u0027re only talking about the operator *deleting* a trait that was set by something else.\n\nBTW, what do you mean by \"drivers\" in this context?\n\nAnyway, one idea is to add a column to your node_traits table indicating the \"source\" of a trait (\"operator\", \"inspector\", etc.) and decide policy from there.  If you want e.g. \"operator\" to be the final arbiter of traits, you can give her unrestricted control over adding/deleting.  If you only want to allow the operator to muck with her own traits, you just bounce any time she tries to delete a trait where \"source\" is not \"operator\".  Etc.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":194,"context_line":"    }"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":197,"context_line":"    {"},{"line_number":198,"context_line":"        \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":199,"context_line":"    }"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_39006501","line":196,"in_reply_to":"7f515b1d_791ccc69","updated":"2017-10-09 09:14:52.000000000","message":"I am partly trying to ignore the driver traits thing, so we get these use cases sorted first.\n\nI am wondering about driver_traits as a separate attribute that the virt driver would union. Then maybe would not allow folks to set a trait that is defined","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"Filtering the node list by tags should work as expected::"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    GET /servers?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":215,"context_line":"    GET /servers?traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":216,"context_line":"    GET /servers?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":217,"context_line":"    GET /servers?not-traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_41bb58c3","line":214,"updated":"2017-10-06 10:30:14.000000000","message":"s/servers/nodes/ here and below","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"Filtering the node list by tags should work as expected::"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    GET /servers?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":215,"context_line":"    GET /servers?traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":216,"context_line":"    GET /servers?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":217,"context_line":"    GET /servers?not-traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_39298589","line":214,"in_reply_to":"7f515b1d_41bb58c3","updated":"2017-10-09 09:14:52.000000000","message":"heh, bad cut and past. oops.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":222,"context_line":"\"ironic\" CLI"},{"line_number":223,"context_line":"~~~~~~~~~~~~"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"No changes, it is deprecated."},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"\"openstack baremetal\" CLI"},{"line_number":228,"context_line":"~~~~~~~~~~~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_21b29ce7","line":225,"updated":"2017-10-06 10:30:14.000000000","message":"nit: not yet, but will be","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"* openstack baremetal node trait list"},{"line_number":231,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":232,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"RPC API impact"},{"line_number":235,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_01b560cd","line":232,"updated":"2017-10-06 10:30:14.000000000","message":"all three examples are missing a node ID","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"* openstack baremetal node trait list"},{"line_number":231,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":232,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"RPC API impact"},{"line_number":235,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_592fa187","line":232,"in_reply_to":"7f515b1d_01b560cd","updated":"2017-10-09 09:14:52.000000000","message":"oops, yes.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":230,"context_line":"* openstack baremetal node trait list"},{"line_number":231,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":232,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"RPC API impact"},{"line_number":235,"context_line":"--------------"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_e1cb6450","line":233,"updated":"2017-10-06 10:30:14.000000000","message":"++ for set and also maybe\n\n openstack baremetal node trait remove $NODE_ID --all","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":230,"context_line":"* openstack baremetal node trait list"},{"line_number":231,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":232,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"RPC API impact"},{"line_number":235,"context_line":"--------------"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_cb59b75b","line":233,"updated":"2017-10-05 20:30:46.000000000","message":"Do you want/need\n\n openstack baremetal node trait set {list of traits}\n\ncorresponding to the PUT /nodes/{node_ident}/traits API?","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":230,"context_line":"* openstack baremetal node trait list"},{"line_number":231,"context_line":"* openstack baremetal node trait add \"CUSTOM_FOO\""},{"line_number":232,"context_line":"* openstack baremetal node trait remove \"CUSTOM_FOO\""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"RPC API impact"},{"line_number":235,"context_line":"--------------"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_79601dd0","line":233,"in_reply_to":"7f515b1d_e1cb6450","updated":"2017-10-09 09:14:52.000000000","message":"maybe that is is an empty set, it does a remove all? but yes, I should add those in.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":262,"context_line":"---------------------"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"Question: should we first register the tag in placement, or will nova driver"},{"line_number":265,"context_line":"just add any missing custom tags in placement?"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"Scalability impact"},{"line_number":268,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_c1c6284a","line":265,"updated":"2017-10-06 10:30:14.000000000","message":"++ for the latter. I\u0027d avoid ironic talking to placement, if possible..","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0ee0e15575d3f6b0af84568d2dbffab8d4046493","unresolved":false,"context_lines":[{"line_number":262,"context_line":"---------------------"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"Question: should we first register the tag in placement, or will nova driver"},{"line_number":265,"context_line":"just add any missing custom tags in placement?"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"Scalability impact"},{"line_number":268,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_6b308308","line":265,"updated":"2017-10-05 20:30:46.000000000","message":"I vote for the second thing","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":262,"context_line":"---------------------"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"Question: should we first register the tag in placement, or will nova driver"},{"line_number":265,"context_line":"just add any missing custom tags in placement?"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"Scalability impact"},{"line_number":268,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_b998d5bf","line":265,"in_reply_to":"7f515b1d_c1c6284a","updated":"2017-10-09 09:14:52.000000000","message":"I was in a bad mood when I wrote this revision, I can stop hedging my bets now!","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3fc7accc61494ac6e5fa729c77793a0fc159750b","unresolved":false,"context_lines":[{"line_number":293,"context_line":"  John Garbutt (johnthetubaguy)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"Other contributors:"},{"line_number":296,"context_line":"  TODO"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"Work Items"},{"line_number":299,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_a1c56c3e","line":296,"updated":"2017-10-06 10:30:14.000000000","message":"feel free to add me","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6a78b3feb14a2508ac5e8de613f1692ee235c3b0","unresolved":false,"context_lines":[{"line_number":293,"context_line":"  John Garbutt (johnthetubaguy)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"Other contributors:"},{"line_number":296,"context_line":"  TODO"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"Work Items"},{"line_number":299,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"7f515b1d_39eaa50e","line":296,"in_reply_to":"7f515b1d_a1c56c3e","updated":"2017-10-09 09:14:52.000000000","message":"sweet, will do. I think I got funding sorted for me working on this. I will probably add in mark too.","commit_id":"9db24b11f9d35f631b2e5af88e2255ff6f3dc8d0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4b05fdbd9ab7b7f615c8f2f1b85ced55e7588a1e","unresolved":false,"context_lines":[{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators much use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."},{"line_number":89,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_ba143dea","line":86,"range":{"start_line":86,"start_character":37,"end_line":86,"end_character":41},"updated":"2017-10-09 14:02:57.000000000","message":"must","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4b05fdbd9ab7b7f615c8f2f1b85ced55e7588a1e","unresolved":false,"context_lines":[{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":240,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED --trait-any CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_7a0b658d","line":240,"range":{"start_line":240,"start_character":32,"end_line":240,"end_character":43},"updated":"2017-10-09 14:02:57.000000000","message":"Eh?  What\u0027s --trait-any?  Meaning glom all values to --trait-any together and give me nodes that have any of those?\n\nSmells weird to me, but I assume you\u0027re duplicating some semantic that\u0027s already present elsewhere.","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a0c976ff846325fcca69d338630a98fde19aaf46","unresolved":false,"context_lines":[{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":240,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED --trait-any CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5f4e5783_f43973f1","line":240,"range":{"start_line":240,"start_character":32,"end_line":240,"end_character":43},"in_reply_to":"7f515b1d_5024fd5f","updated":"2017-10-17 17:03:15.000000000","message":"Yeah, my bad, miss-remembered how argparse worked there.","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b1c5da02f076d8bc244356e69dd20912dfd58291","unresolved":false,"context_lines":[{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":240,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED --trait-any CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_5024fd5f","line":240,"range":{"start_line":240,"start_character":32,"end_line":240,"end_character":43},"in_reply_to":"7f515b1d_69fcbd71","updated":"2017-10-09 21:11:47.000000000","message":"Okay, I\u0027m certainly no expert on the CLI, but I would say separating --*-any opts is a pretty confusing syntax, and should be avoided unless it\u0027s following some established precedent.  Of course if tag-any is supported in the CLI, do it the same way; but if you were doing it from scratch, it sure would read better as\n\n --trait-any TRAIT1 TRAIT2 TRAIT3","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"dc0ddaeb6b2a28ca62f3aa02b6fd183764e8c079","unresolved":false,"context_lines":[{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":240,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED --trait-any CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_69fcbd71","line":240,"range":{"start_line":240,"start_character":32,"end_line":240,"end_character":43},"in_reply_to":"7f515b1d_7a0b658d","updated":"2017-10-09 16:20:37.000000000","message":"its just taken from here:\nhttp://specs.openstack.org/openstack/api-wg/guidelines/tags.html#filtering-and-searching-by-tags\n\nWe could remove that if we don\u0027t want it for the v1.","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4b05fdbd9ab7b7f615c8f2f1b85ced55e7588a1e","unresolved":false,"context_lines":[{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"RPC API impact"},{"line_number":246,"context_line":"--------------"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_3ae84db1","line":244,"updated":"2017-10-09 14:02:57.000000000","message":"Did you want to add\n\n openstack baremetal node trait set \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR CUSTOM_BAZ\n\n?","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"dc0ddaeb6b2a28ca62f3aa02b6fd183764e8c079","unresolved":false,"context_lines":[{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"RPC API impact"},{"line_number":246,"context_line":"--------------"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_4936a153","line":244,"in_reply_to":"7f515b1d_3ae84db1","updated":"2017-10-09 16:20:37.000000000","message":"hmm, not sure really. I am open to suggestions.","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b1c5da02f076d8bc244356e69dd20912dfd58291","unresolved":false,"context_lines":[{"line_number":241,"context_line":"* openstack baremetal node list"},{"line_number":242,"context_line":"  --not-trait-any CUSTOM_RED --not-trait-any CUSTOM_BLUE"},{"line_number":243,"context_line":"* openstack baremetal node list --field traits"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"RPC API impact"},{"line_number":246,"context_line":"--------------"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"7f515b1d_10220553","line":244,"in_reply_to":"7f515b1d_4936a153","updated":"2017-10-09 21:11:47.000000000","message":"Again, no expert here, looking for input from ironic folks.","commit_id":"be204845dc5324c140dd94fe0c7a449354e6faea"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":48,"context_line":"* https://review.openstack.org/#/c/497713"},{"line_number":49,"context_line":"* https://review.openstack.org/#/c/468797"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"The current proposal is for Ironic to store a list of traits that can then be"},{"line_number":52,"context_line":"copied into placement via Nova\u0027s ironic virt driver, in a similar way to how"},{"line_number":53,"context_line":"the RESOURCE_CLASS of each ironic node is reported."},{"line_number":54,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_5c0dd335","line":51,"updated":"2017-10-18 20:01:31.000000000","message":"store a list of traits for each node, right?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":48,"context_line":"* https://review.openstack.org/#/c/497713"},{"line_number":49,"context_line":"* https://review.openstack.org/#/c/468797"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"The current proposal is for Ironic to store a list of traits that can then be"},{"line_number":52,"context_line":"copied into placement via Nova\u0027s ironic virt driver, in a similar way to how"},{"line_number":53,"context_line":"the RESOURCE_CLASS of each ironic node is reported."},{"line_number":54,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a2f80ec4","line":51,"in_reply_to":"3f4b6375_5c0dd335","updated":"2017-10-19 09:04:11.000000000","message":"+1 I missed that bit.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_1c3d1bbb","line":57,"updated":"2017-10-18 20:01:31.000000000","message":"if all the nodes have the \u0027CUSTOM_GENERAL_USE\u0027 trait, then there is no use using it, right? You\u0027d only want this to distinguish some set of ironic nodes from other nodes.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_349e9163","line":57,"in_reply_to":"3f4b6375_020a42d1","updated":"2017-10-19 15:41:17.000000000","message":"Thanks for clarifying this! :)","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6f75ffb332e3ac687e64c2ef16d09294ea38521","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_203545db","line":57,"in_reply_to":"3f4b6375_1c3d1bbb","updated":"2017-10-18 21:20:08.000000000","message":"We\u0027ve haggled over this wording in several revisions now, sad-face.\n\nI believe the intent is for CUSTOM_GENERAL_USE nodes and CUSTOM_RESERVED_PROJECT_B nodes to be disparate (not overlapping).","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"474ce8b1f886ff974e9f7dad2fc49d0608d14e2b","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_b303cd0d","line":57,"in_reply_to":"3f4b6375_203545db","updated":"2017-10-18 22:08:57.000000000","message":"OH, I missed that. So that I understand, there are two sets of nodes, one set for general use, and another set that is reserved for project B? And there is no intersection of these two sets?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"63beacc7e90f1d70effe5ab36f856e9c801af3bb","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_b3bcedbb","line":57,"in_reply_to":"3f4b6375_b303cd0d","updated":"2017-10-18 22:24:14.000000000","message":"Yeah.  Or if there\u0027s intersection, there\u0027s still at least some nodes that aren\u0027t CUSTOM_GENERAL_USE.\n\nHm, I went looking for the patch set where we discussed the wording here, but couldn\u0027t find it.  Maybe it was in IRC.  Or in the nova spec.  Or all in my head...","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_020a42d1","line":57,"in_reply_to":"3f4b6375_b37d0de9","updated":"2017-10-19 09:04:11.000000000","message":"It is really because you can only detect the presence of a trait in the flavor, you can\u0027t say I want a node with no reserved project traits.\n\nNow it means you might have nodes, with no traits, that are selected by no flavor. Those are likely the nodes that still need to go through QA testing, in my head.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cea0d450ce8e0ead7d5bda2d1ae9ff9cb1168fce","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Going back to the use case of dedicating a pool of ironic nodes for a specific"},{"line_number":56,"context_line":"project, this can be done by marking all nodes for general use with a trait"},{"line_number":57,"context_line":"`CUSTOM_GENERAL_USE` and other nodes tagged with `CUSTOM_RESERVED_PROJECT_B`."},{"line_number":58,"context_line":"All the public flavors are updated to require the `CUSTOM_GENERAL_USE` trait"},{"line_number":59,"context_line":"and a new flavor is created for another tenant that is marked with"},{"line_number":60,"context_line":"`CUSTOM_RESERVED_PROJECT_B`. With all this setup the appropriate ironic nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_b37d0de9","line":57,"in_reply_to":"3f4b6375_b3bcedbb","updated":"2017-10-18 22:29:13.000000000","message":"Found it.  It was indeed in the nova spec.  See https://review.openstack.org/#/c/507052/3/specs/queens/approved/ironic-traits.rst@40\nand the followup\nhttps://review.openstack.org/#/c/507052/4/specs/queens/approved/ironic-traits.rst@43\nand the resolution\nhttps://review.openstack.org/#/c/507052/7/specs/queens/approved/ironic-traits.rst@45","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Note when provisioning a node, Nova will send flavor extra_specs to Ironic and"},{"line_number":77,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":78,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":79,"context_line":"also include `/instance_info/traits` in a very similar way. For completeness"},{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_10473a20","line":78,"updated":"2017-10-18 20:01:31.000000000","message":"For \u0027we will need to also include\u0027 : the \u0027we\u0027 is the nova-ironic-driver, right? not ironic?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Note when provisioning a node, Nova will send flavor extra_specs to Ironic and"},{"line_number":77,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":78,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":79,"context_line":"also include `/instance_info/traits` in a very similar way. For completeness"},{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a2212e50","line":78,"in_reply_to":"3f4b6375_10473a20","updated":"2017-10-19 09:04:11.000000000","message":"I was meaning \"we\" the coder, but I should be more precise, but yeah, its the nova-ironic-driver","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":77,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":78,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":79,"context_line":"also include `/instance_info/traits` in a very similar way. For completeness"},{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_30f8f6c2","line":80,"range":{"start_line":80,"start_character":27,"end_line":80,"end_character":46},"updated":"2017-10-18 20:01:31.000000000","message":"not sure what you mean by this, what\u0027s a requested resource?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":77,"context_line":"this will now include the traits the user has requested. Currently the nova"},{"line_number":78,"context_line":"driver only stores extra_specs starting with `capabilities`, we will need to"},{"line_number":79,"context_line":"also include `/instance_info/traits` in a very similar way. For completeness"},{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_e2dcc647","line":80,"range":{"start_line":80,"start_character":27,"end_line":80,"end_character":46},"in_reply_to":"3f4b6375_30f8f6c2","updated":"2017-10-19 09:04:11.000000000","message":"ah, its the extra specs in the flavor that define what resources the user has requested, like resource:CUSTOM_GOLD\u003d1 and resource:VCPU\u003d0","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_3021164f","line":83,"updated":"2017-10-18 20:01:31.000000000","message":"sync the list of traits in nova\u0027s placement, with the list in ironic... ?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":80,"context_line":"we should also include the requested resources. More details on these changes"},{"line_number":81,"context_line":"will be included in the Nova spec."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_82e3d205","line":83,"in_reply_to":"3f4b6375_3021164f","updated":"2017-10-19 09:04:11.000000000","message":"yes, and the sync is kinda triggered by the virt driver, will tidy that up.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_101e1a0c","line":85,"updated":"2017-10-18 20:01:31.000000000","message":"nova\u0027s placement?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6f75ffb332e3ac687e64c2ef16d09294ea38521","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_e083ad55","line":85,"in_reply_to":"3f4b6375_101e1a0c","updated":"2017-10-18 21:20:08.000000000","message":"There is only one placement.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"63beacc7e90f1d70effe5ab36f856e9c801af3bb","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_f3d6e5f1","line":85,"in_reply_to":"3f4b6375_330fdd2f","updated":"2017-10-18 22:24:14.000000000","message":"Well, yeah, but TBH I tend to think of placement as a separate service, which it hopefully still has some chance of becoming one day.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"474ce8b1f886ff974e9f7dad2fc49d0608d14e2b","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_330fdd2f","line":85,"in_reply_to":"3f4b6375_e083ad55","updated":"2017-10-18 22:08:57.000000000","message":"You must be a nova person :D\n\nI did figure it out but I had my ironic hat on.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Note: the Nova virt driver will sync the list of traits with the list in Ironic"},{"line_number":84,"context_line":"in a similar way to the Resource Class. Should an operator talk directly to"},{"line_number":85,"context_line":"Placement to adjust the traits, the driver will remove all those modifications"},{"line_number":86,"context_line":"when the next sync occurs. Operators must use the Ironic API to change the list"},{"line_number":87,"context_line":"of traits on a given node, Ironic is assumed to be the source of truth for the"},{"line_number":88,"context_line":"list of traits."}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_02a6c2c2","line":85,"in_reply_to":"3f4b6375_f3d6e5f1","updated":"2017-10-19 09:04:11.000000000","message":"Yeah, I am still in the camp of it being \"the\" placement service that lives outside of Nova. I can tighten that up.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":119,"context_line":"for tags:"},{"line_number":120,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"This change superseeds the idea of tags, so the above tables should be removed"},{"line_number":123,"context_line":"and replaced with a similar traits table::"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    CREATE TABLE node_traits ("}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_30947644","line":122,"updated":"2017-10-18 20:01:31.000000000","message":"nit s/superseeds/supercedes/. Or is it supersedes? :)","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":119,"context_line":"for tags:"},{"line_number":120,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"This change superseeds the idea of tags, so the above tables should be removed"},{"line_number":123,"context_line":"and replaced with a similar traits table::"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    CREATE TABLE node_traits ("}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_74d089f8","line":122,"in_reply_to":"3f4b6375_028fe22a","updated":"2017-10-19 15:41:17.000000000","message":"hmm, hallowe\u0027en is coming up...","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6f75ffb332e3ac687e64c2ef16d09294ea38521","unresolved":false,"context_lines":[{"line_number":119,"context_line":"for tags:"},{"line_number":120,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"This change superseeds the idea of tags, so the above tables should be removed"},{"line_number":123,"context_line":"and replaced with a similar traits table::"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    CREATE TABLE node_traits ("}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a07db56f","line":122,"in_reply_to":"3f4b6375_30947644","updated":"2017-10-18 21:20:08.000000000","message":"The latter.  Good eye.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":119,"context_line":"for tags:"},{"line_number":120,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"This change superseeds the idea of tags, so the above tables should be removed"},{"line_number":123,"context_line":"and replaced with a similar traits table::"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    CREATE TABLE node_traits ("}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_028fe22a","line":122,"in_reply_to":"3f4b6375_a07db56f","updated":"2017-10-19 09:04:11.000000000","message":"superseeds, like toasted pumpkin seeds","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":132,"context_line":"          ON DELETE CASCADE,"},{"line_number":133,"context_line":"    )"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"A new ironic.objects.tags.NodeTraitList object will be added to the object"},{"line_number":136,"context_line":"model. The ironic.objects.tags.NodeTraitList field in the python object model"},{"line_number":137,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":138,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_d0c1a230","line":135,"updated":"2017-10-18 20:01:31.000000000","message":"Wondering why it isn\u0027t ironic.objects.traits.NodeTraitList. Are we going to keep around ironic.objects.tags then? Or get rid of that too?\n\nOh wait, we never landed ironic.objects.tags :-)","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":136,"context_line":"model. The ironic.objects.tags.NodeTraitList field in the python object model"},{"line_number":137,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"A tag should be defined in a way that matches the placement API definition,"},{"line_number":140,"context_line":"as a Unicode string no longer than 255 characters in length."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"State Machine Impact"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_306c7635","line":139,"updated":"2017-10-18 20:01:31.000000000","message":"so is this \u0027tag\u0027 or \u0027trait\u0027?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":136,"context_line":"model. The ironic.objects.tags.NodeTraitList field in the python object model"},{"line_number":137,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"A tag should be defined in a way that matches the placement API definition,"},{"line_number":140,"context_line":"as a Unicode string no longer than 255 characters in length."},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"State Machine Impact"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a2a24e99","line":139,"in_reply_to":"3f4b6375_306c7635","updated":"2017-10-19 09:04:11.000000000","message":"My bad, its a trait. I talk about tagging a node with a trait, which is probably bad.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":151,"context_line":"requires any other trait to start with the prefix of `CUSTOM_`. While any trait"},{"line_number":152,"context_line":"must be first registered with placement before the ironic virt driver will be"},{"line_number":153,"context_line":"able to add the trait to the resource provider, the Ironic API will simply"},{"line_number":154,"context_line":"validate the trait is valid (either in the standard list or with the correct"},{"line_number":155,"context_line":"prefix), rather than checking any traits are correctly registered in placement."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"For convenience, it will be possible to get the full list of nodes and the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_b0ef4686","line":154,"updated":"2017-10-18 20:01:31.000000000","message":"which API would do this validation? (not sure at what point you\u0027re thinking of this validation.)  I\u0027m fine if we rplace \u0027tags\u0027 with \u0027traits\u0027 as a general mechanism for tagging, as long as these traits have the same behaviour/etc as the old tags. I\u0027m not sure I want to actually restrict this to the standard/accepted list of traits from os-traits or trait strings that are prefixed with \u0027CUSTOM_\u0027. Because this should be usable by a setup that has ironic but no nova.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":151,"context_line":"requires any other trait to start with the prefix of `CUSTOM_`. While any trait"},{"line_number":152,"context_line":"must be first registered with placement before the ironic virt driver will be"},{"line_number":153,"context_line":"able to add the trait to the resource provider, the Ironic API will simply"},{"line_number":154,"context_line":"validate the trait is valid (either in the standard list or with the correct"},{"line_number":155,"context_line":"prefix), rather than checking any traits are correctly registered in placement."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"For convenience, it will be possible to get the full list of nodes and the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_c2eb0aae","line":154,"in_reply_to":"3f4b6375_b0ef4686","updated":"2017-10-19 09:04:11.000000000","message":"Well that is just a tag. A trait has a defined structure. If we don\u0027t follow it, what do we do when we copy those into Nova.\n\nSo I should be clear, this validation only depends on the os-traits library, it doesn\u0027t require any talking to a REST API in Nova, so it should work just fine standalone.\n\nI should define the validation logic that is siting in my head better.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":164,"context_line":"      \"nodes\": ["},{"line_number":165,"context_line":"        {"},{"line_number":166,"context_line":"          ..."},{"line_number":167,"context_line":"          \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":168,"context_line":"          ..."},{"line_number":169,"context_line":"        }"},{"line_number":170,"context_line":"      ]"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_703aeee7","line":167,"updated":"2017-10-18 20:01:31.000000000","message":"if you really want to restrict \u0027traits\u0027 to standard/customized traits, we should use more realistic traits here","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":164,"context_line":"      \"nodes\": ["},{"line_number":165,"context_line":"        {"},{"line_number":166,"context_line":"          ..."},{"line_number":167,"context_line":"          \"traits\": [\u0027foo\u0027, \u0027bar\u0027, \u0027baz\u0027]"},{"line_number":168,"context_line":"          ..."},{"line_number":169,"context_line":"        }"},{"line_number":170,"context_line":"      ]"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_42d71aef","line":167,"in_reply_to":"3f4b6375_703aeee7","updated":"2017-10-19 09:04:11.000000000","message":"oops, yes. cut and paste error","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"The manipulation of node traits will follow the patterns established by both"},{"line_number":190,"context_line":"the placement API and API WG tags spec:"},{"line_number":191,"context_line":"https://developer.openstack.org/api-ref/placement/#resource-provider-traits"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"To start with there will be a new traits resource that follows the above"},{"line_number":194,"context_line":"patterns to get all and set all the traits::"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_d02ac28e","line":191,"updated":"2017-10-18 20:01:31.000000000","message":"this link isn\u0027t to a spec, it is to the placement api-ref. Is that what you wanted to do?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"The manipulation of node traits will follow the patterns established by both"},{"line_number":190,"context_line":"the placement API and API WG tags spec:"},{"line_number":191,"context_line":"https://developer.openstack.org/api-ref/placement/#resource-provider-traits"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"To start with there will be a new traits resource that follows the above"},{"line_number":194,"context_line":"patterns to get all and set all the traits::"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a2f92e81","line":191,"in_reply_to":"3f4b6375_d02ac28e","updated":"2017-10-19 09:04:11.000000000","message":"Yes, I should add both links I guess.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    DELETE /nodes/{node_ident}/traits/CUSTOM_FOO"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"Filtering the node list by tags should work as expected::"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    GET /nodes?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":220,"context_line":"    GET /nodes?traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_4bf747f7","line":217,"updated":"2017-10-18 20:01:31.000000000","message":"s/tags/traits/","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    GET /nodes?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":220,"context_line":"    GET /nodes?traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":221,"context_line":"    GET /nodes?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":222,"context_line":"    GET /nodes?not-traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"Client (CLI) impact"},{"line_number":225,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_8bd65f4f","line":222,"updated":"2017-10-18 20:01:31.000000000","message":"The WG\u0027s tag spec (http://specs.openstack.org/openstack/api-wg/guidelines/tags.html) also has combined tag arguments, eg: GET /servers?tags\u003dred\u0026not-tags\u003dred. Is that something you are going to do in this spec too? L240 seems to indicate yes.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":219,"context_line":"    GET /nodes?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":220,"context_line":"    GET /nodes?traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":221,"context_line":"    GET /nodes?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":222,"context_line":"    GET /nodes?not-traits-any\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"Client (CLI) impact"},{"line_number":225,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_6248d6cb","line":222,"in_reply_to":"3f4b6375_8bd65f4f","updated":"2017-10-19 09:04:11.000000000","message":"Yes, I just got bored typing it out I guess.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":235,"context_line":"* openstack baremetal node trait list \u003cnode-uuid\u003e"},{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_8b3bff9a","line":238,"updated":"2017-10-18 20:01:31.000000000","message":"\u003cnode-uuid\u003e ? (or are you removing all traits from all nodes?)","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":235,"context_line":"* openstack baremetal node trait list \u003cnode-uuid\u003e"},{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_a227ee12","line":238,"in_reply_to":"3f4b6375_8b3bff9a","updated":"2017-10-19 09:04:11.000000000","message":"oops, yes.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":242,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_eb5393e1","line":239,"updated":"2017-10-18 20:01:31.000000000","message":"is this missing \u003cnode-uuid\u003e?  What\u0027s the diff between \u0027set\u0027 and \u0027add\u0027?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"474ce8b1f886ff974e9f7dad2fc49d0608d14e2b","unresolved":false,"context_lines":[{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":242,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_73455517","line":239,"in_reply_to":"3f4b6375_2091c59a","updated":"2017-10-18 22:08:57.000000000","message":"OH. If you want \u0027set\u0027, then we should have \u0027unset\u0027. Instead of remove-all.\n\nHmm. I\u0027m actually worried that someone will make a mistake and accidentally remove everything, either via set or unset. I\u0027d prefer an --all for the \u0027remove\u0027, and not have set/unset. You can do a \u0027set\u0027 via \u0027remove --all\u0027 followed by \u0027add\u0027. If we allow add to take one or more traits, which makes sense.\n\nMy other thought is to see how other osc commands handle lists of things and be consistent with that.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"63beacc7e90f1d70effe5ab36f856e9c801af3bb","unresolved":false,"context_lines":[{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":242,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_f3ef05ae","line":239,"in_reply_to":"3f4b6375_73455517","updated":"2017-10-18 22:24:14.000000000","message":"\u003e My other thought is to see how other osc commands handle lists of\n \u003e things and be consistent with that.\n\nThis, all the way.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6f75ffb332e3ac687e64c2ef16d09294ea38521","unresolved":false,"context_lines":[{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":242,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_2091c59a","line":239,"in_reply_to":"3f4b6375_eb5393e1","updated":"2017-10-18 21:20:08.000000000","message":"\u0027set\u0027 replaces.  \u0027add\u0027 appends.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":236,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":237,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e \"CUSTOM_FOO\""},{"line_number":238,"context_line":"* openstack baremetal node trait remove-all"},{"line_number":239,"context_line":"* openstack baremetal node trait set CUSTOM_FOO CUSTOM_BAR"},{"line_number":240,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":241,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":242,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_42739a16","line":239,"in_reply_to":"3f4b6375_f3ef05ae","updated":"2017-10-19 09:04:11.000000000","message":"+1 that was my intent, I just couldn\u0027t find any when I last looked, I should look harder.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":267,"context_line":"---------------"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"There will be a hardcoded limit of 50 traits for any Node to prevent miss-use"},{"line_number":270,"context_line":"of the API."},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"Other end user impact"},{"line_number":273,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_cbc5775d","line":270,"updated":"2017-10-18 20:01:31.000000000","message":"I\u0027m not sure I understand this. Who would s/miss-use/misuse/ this?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":267,"context_line":"---------------"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"There will be a hardcoded limit of 50 traits for any Node to prevent miss-use"},{"line_number":270,"context_line":"of the API."},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"Other end user impact"},{"line_number":273,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_2262de3f","line":270,"in_reply_to":"3f4b6375_cbc5775d","updated":"2017-10-19 09:04:11.000000000","message":"Someone trying to DDoS your system. Its just to prevent the worst of filling all your database with rubbish.\n\nIt has the advantage of matching the limit enforced in placement.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":307,"context_line":"Other contributors:"},{"line_number":308,"context_line":"  Dmitry Tantsur (dtantsur)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"Work Items"},{"line_number":311,"context_line":"----------"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"* Update tables to store traits for nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_2b21ab4e","line":310,"updated":"2017-10-18 20:01:31.000000000","message":"is removing/cleaning up tags-related work part of this spec?","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":307,"context_line":"Other contributors:"},{"line_number":308,"context_line":"  Dmitry Tantsur (dtantsur)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"Work Items"},{"line_number":311,"context_line":"----------"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"* Update tables to store traits for nodes"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_22c77e15","line":310,"in_reply_to":"3f4b6375_2b21ab4e","updated":"2017-10-19 09:04:11.000000000","message":"I wasn\u0027t going to, that felt separate to me, but I am not against adding it.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":314,"context_line":"* Add new traits API"},{"line_number":315,"context_line":"* Add openstack cli support for the new API"},{"line_number":316,"context_line":"* Follow up with Nova driver work"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"Dependencies"},{"line_number":319,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_eb1a7315","line":317,"updated":"2017-10-18 20:01:31.000000000","message":"also need to add the new Object for this","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":314,"context_line":"* Add new traits API"},{"line_number":315,"context_line":"* Add openstack cli support for the new API"},{"line_number":316,"context_line":"* Follow up with Nova driver work"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"Dependencies"},{"line_number":319,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_02f5a257","line":317,"in_reply_to":"3f4b6375_eb1a7315","updated":"2017-10-19 09:04:11.000000000","message":"oops, +1","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"287ffc2a7d6f76fccaf9914e136a714f2b57cad7","unresolved":false,"context_lines":[{"line_number":342,"context_line":"References"},{"line_number":343,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"None."},{"line_number":346,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_abd23b33","line":345,"updated":"2017-10-18 20:01:31.000000000","message":"actually, there are a few references, you have mentioned them already.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":342,"context_line":"References"},{"line_number":343,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"None."},{"line_number":346,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_9ac9ef79","line":345,"in_reply_to":"3f4b6375_62e456ad","updated":"2017-10-19 15:41:17.000000000","message":"yeah, some specs have text that reference the links here so it is only in this section. No big deal.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"746ff9bdb83cd3b3cb67401a5d663e50ae4d749f","unresolved":false,"context_lines":[{"line_number":342,"context_line":"References"},{"line_number":343,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"None."},{"line_number":346,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3f4b6375_62e456ad","line":345,"in_reply_to":"3f4b6375_abd23b33","updated":"2017-10-19 09:04:11.000000000","message":"Seemed odd to list them twice, but happy to be consistent.","commit_id":"d611fe1df7a12719a39b1b501e71817a6b7bfe9a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":49,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/add-trait-support-in-allocation-candidates.rst"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"The current proposal is for Ironic to store a list of traits for each node."},{"line_number":52,"context_line":"That list can then by synced into the appropriate resource provider in the"},{"line_number":53,"context_line":"Placement API by Nova\u0027s ironic virt driver, in a similar way to how"},{"line_number":54,"context_line":"the RESOURCE_CLASS of each ironic node is reported today."},{"line_number":55,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_f45fd9e2","line":52,"updated":"2017-10-19 15:41:17.000000000","message":"nit s/by synced/be synced/","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":118,"context_line":"The current approach keeps Ironic from needing to depend on placement in any"},{"line_number":119,"context_line":"way, which works well."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"The REST API described below allows fine grained control of settings traits,"},{"line_number":122,"context_line":"following the patterns from the API-WG and existing placement APIs. We could"},{"line_number":123,"context_line":"instead have just extended the existing ironic node PATCH interface. In terms"},{"line_number":124,"context_line":"of the CLI, regardless of the chose API direction we would add it into the"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_74cbe952","line":121,"updated":"2017-10-19 15:41:17.000000000","message":"nit s/settings/setting/","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":121,"context_line":"The REST API described below allows fine grained control of settings traits,"},{"line_number":122,"context_line":"following the patterns from the API-WG and existing placement APIs. We could"},{"line_number":123,"context_line":"instead have just extended the existing ironic node PATCH interface. In terms"},{"line_number":124,"context_line":"of the CLI, regardless of the chose API direction we would add it into the"},{"line_number":125,"context_line":"existing `openstack baremetal node set` and `unset` like this:"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"* openstack baremetal node set --trait CUSTOM_FOO \u003cnode-uuid\u003e"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_54c8ed53","line":124,"updated":"2017-10-19 15:41:17.000000000","message":"nit s/chose/chosen/ ?","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":155,"context_line":"model. The ironic.objects.traits.NodeTraitList field in the python object model"},{"line_number":156,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"A tag should be defined in a way that matches the placement API definition,"},{"line_number":159,"context_line":"as a Unicode string no longer than 255 characters in length."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"State Machine Impact"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_34da1117","line":158,"updated":"2017-10-19 15:41:17.000000000","message":"s/tag/trait/","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":178,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":179,"context_line":"Ironic to assign arbitrary tags on particular Ironic nodes."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":182,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_74276914","line":179,"updated":"2017-10-19 15:41:17.000000000","message":"Is the plan then, that if the os-traits library is available, there will be validation of the trait names? I am wondering if there might be a case where someone has that library but doesn\u0027t want the check (wondering if we should have an explicit config). I guess we can defer that til later if someone needs it. The default would be to use the library if it was avail anyway, so it wouldn\u0027t be hard to add a config later.\n\nI know that \u0027traits\u0027 means something in OpenStack, but given that we\u0027re replacing tags with traits, I want to make sure that the tags features are still available with traits.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":178,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":179,"context_line":"Ironic to assign arbitrary tags on particular Ironic nodes."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":182,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_f580e28d","line":179,"in_reply_to":"3f4b6375_74276914","updated":"2017-10-20 09:55:04.000000000","message":"Totally understood, its just they need to add CUSTOM_ at the start of their tags.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"d803afa296524cb514e4e01c4747bb6cdb34e638","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":178,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":179,"context_line":"Ironic to assign arbitrary tags on particular Ironic nodes."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":182,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_4c6a8cb3","line":179,"in_reply_to":"3f4b6375_f580e28d","updated":"2017-10-20 13:14:24.000000000","message":"I guess this is a more general question: is the traits API really nova-specific, or is it going to be fully supported for the standalone case? It seems there\u0027s a conflict here between making it flexible in the standalone case (e.g. set any trait), and easing the nova integration path (e.g. enforce os-traits).\n\nMaking this a config option could lead to odd behaviour if the option is ever changed, although we could just be clear that that is unsupported.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"The manipulation of node traits will follow the patterns established by both"},{"line_number":214,"context_line":"the placement API and API WG tags spec:"},{"line_number":215,"context_line":"https://developer.openstack.org/api-ref/placement/#resource-provider-traits"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"To start with there will be a new traits resource that follows the above"},{"line_number":218,"context_line":"patterns to get all and set all the traits::"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_3407b11f","line":215,"updated":"2017-10-19 15:41:17.000000000","message":"I think you wanted to add both links. You could do something like \u0027established by both the `placement API \u003chttps://developer....\u003e`_ and `API WG tags spec \u003chttps://...\u003e`_.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"The manipulation of node traits will follow the patterns established by both"},{"line_number":214,"context_line":"the placement API and API WG tags spec:"},{"line_number":215,"context_line":"https://developer.openstack.org/api-ref/placement/#resource-provider-traits"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"To start with there will be a new traits resource that follows the above"},{"line_number":218,"context_line":"patterns to get all and set all the traits::"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_954f6e91","line":215,"in_reply_to":"3f4b6375_3407b11f","updated":"2017-10-20 09:55:04.000000000","message":"dang, I missed update quite a few bits here.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":218,"context_line":"patterns to get all and set all the traits::"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    GET /nodes/{node_ident}/traits"},{"line_number":221,"context_line":"    {"},{"line_number":222,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":223,"context_line":"    }"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":226,"context_line":"    {"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_fa26a3b7","line":223,"range":{"start_line":221,"start_character":4,"end_line":223,"end_character":5},"updated":"2017-10-19 15:41:17.000000000","message":"This is what is returned in the response, right?\n\nVs below (L226-229) is the data in the request?","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":218,"context_line":"patterns to get all and set all the traits::"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    GET /nodes/{node_ident}/traits"},{"line_number":221,"context_line":"    {"},{"line_number":222,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":223,"context_line":"    }"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":226,"context_line":"    {"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_7546327f","line":223,"range":{"start_line":221,"start_character":4,"end_line":223,"end_character":5},"in_reply_to":"3f4b6375_fa26a3b7","updated":"2017-10-20 09:55:04.000000000","message":"yeah, being fast and loose there.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    DELETE /nodes/{node_ident}/traits/CUSTOM_FOO"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"Filtering the node list by tags should work as expected::"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    GET /nodes?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":244,"context_line":"    GET /nodes?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE\u0026traits\u003dCUSTOM_FOO"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_7a47534d","line":241,"updated":"2017-10-19 15:41:17.000000000","message":"s/tags/traits/","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":259,"context_line":"You can list the traits on a node:"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":262,"context_line":"* openstack baremetal node show --fields uuid name traits \u003cnode-uuid\u003e"},{"line_number":263,"context_line":"* openstack baremetal node trait list \u003cnode-uuid\u003e"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"You can update the list of traits on a node:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_5a9b77c8","line":262,"updated":"2017-10-19 15:41:17.000000000","message":"nit s/node-uuid/node-ident/\n\ncuz we can use a name or uuid.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":259,"context_line":"You can list the traits on a node:"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":262,"context_line":"* openstack baremetal node show --fields uuid name traits \u003cnode-uuid\u003e"},{"line_number":263,"context_line":"* openstack baremetal node trait list \u003cnode-uuid\u003e"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"You can update the list of traits on a node:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_3560ba01","line":262,"in_reply_to":"3f4b6375_5a9b77c8","updated":"2017-10-20 09:55:04.000000000","message":"good call.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"You can update the list of traits on a node:"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":268,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":269,"context_line":"* openstack baremetal node trait remove-all \u003cnode-uuid\u003e"},{"line_number":270,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_3ae2fbd6","line":267,"updated":"2017-10-19 15:41:17.000000000","message":"I looked, and this is the first time we\u0027re using the \u0027add\u0027 verb. I then looked at the osc docs [1] and for add/remove it sez \"add some object to a container object; the command is built in the order of container add object \u003ccontainer\u003e \u003cobject\u003e, the positional arguments appear in the same order\".\n\nWhich means it should be \u0027openstack baremetal node add trait \u003cnode\u003e \u003ctrait\u003e. I wonder if it can be \u003ctrait1\u003e \u003ctrait2\u003e ...\n\nHey, do you have any experience with the aggregate command [2]? An aggregate can have more than one host, right? Cuz this only seems to allow adding/removing one at a time.\n\nYay, consistency group [3] allows for adding/removing more than one.\n\n[1] https://docs.openstack.org/python-openstackclient/latest/cli/commands.html\n\n[2] https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/aggregate.html#aggregate-add-host\n\n[3] https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/consistency-group.html#consistency-group-add-volume","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"You can update the list of traits on a node:"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":268,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":269,"context_line":"* openstack baremetal node trait remove-all \u003cnode-uuid\u003e"},{"line_number":270,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_a050ea0b","line":267,"in_reply_to":"3f4b6375_3ae2fbd6","updated":"2017-10-20 09:55:04.000000000","message":"You know I thought that was the order, but I got talked around on IRC to the above order. I probably need to add some links.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":268,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":269,"context_line":"* openstack baremetal node trait remove-all \u003cnode-uuid\u003e"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":272,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_baaeeb2b","line":269,"updated":"2017-10-19 15:41:17.000000000","message":"i feel like \u0027openstack baremetal node remove trait --all \u003cnode-ident\u003e\u0027 might be better. Might be worth asking the OSC folks.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0bc518896e27482a3314273ef349d736f94ad","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"* openstack baremetal node trait add \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":268,"context_line":"* openstack baremetal node trait remove \u003cnode-uuid\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":269,"context_line":"* openstack baremetal node trait remove-all \u003cnode-uuid\u003e"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":272,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_c07646a5","line":269,"in_reply_to":"3f4b6375_baaeeb2b","updated":"2017-10-20 09:55:04.000000000","message":"Oh, interesting... I like that now I see it.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":279,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":280,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"RPC API impact"},{"line_number":283,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_ba470b1e","line":280,"updated":"2017-10-19 15:41:17.000000000","message":"if we allow \u0027openstack baremetal node trait list\u0027, do we also want to allow these optional things for that too?","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0dc1b537d1489c4d0244c8dc5f8927d39bbaa926","unresolved":false,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":279,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":280,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"RPC API impact"},{"line_number":283,"context_line":"--------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_01ca206c","line":280,"in_reply_to":"3f4b6375_ba470b1e","updated":"2017-10-20 10:17:05.000000000","message":"I don\u0027t think so, because the node trait list is specific to the node, so its a bit odd just trimming that list.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":278,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":279,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":280,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"RPC API impact"},{"line_number":283,"context_line":"--------------"},{"line_number":284,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_fa7103e2","line":281,"updated":"2017-10-19 15:41:17.000000000","message":"I was wondering. would someone want an API to get a list of all traits being used by any nodes? You could do it by getting the response for \u0027openstack baremetal node list --fields trait\u0027 and then deduping. Anyway, it can also be added later :-)","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"d803afa296524cb514e4e01c4747bb6cdb34e638","unresolved":false,"context_lines":[{"line_number":278,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":279,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":280,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"RPC API impact"},{"line_number":283,"context_line":"--------------"},{"line_number":284,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_ac87c061","line":281,"in_reply_to":"3f4b6375_a1a46cba","updated":"2017-10-20 13:14:24.000000000","message":"This will be particularly useful if drivers offer traits. I think we\u0027d need to understand those interactions better before offering an API to discover traits.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0dc1b537d1489c4d0244c8dc5f8927d39bbaa926","unresolved":false,"context_lines":[{"line_number":278,"context_line":"* openstack baremetal node list --trait CUSTOM_RED --not-trait CUSTOM_BLUE"},{"line_number":279,"context_line":"* openstack baremetal node list --trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":280,"context_line":"* openstack baremetal node list --not-trait-any CUSTOM_RED CUSTOM_BLUE"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"RPC API impact"},{"line_number":283,"context_line":"--------------"},{"line_number":284,"context_line":""}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_a1a46cba","line":281,"in_reply_to":"3f4b6375_fa7103e2","updated":"2017-10-20 10:17:05.000000000","message":"That is interesting, but it feels like v2, I would go for that.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":306,"context_line":"There will be a hard coded limit of 50 traits for any Node to prevent miss-use"},{"line_number":307,"context_line":"of the API. This prevents denial of service attack where the database is filled"},{"line_number":308,"context_line":"up by a rogue user setting lots of traits. Really the limit is in place to"},{"line_number":309,"context_line":"match the limit applied in the placement API."},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"Other end user impact"},{"line_number":312,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_baf5cbcd","line":309,"updated":"2017-10-19 15:41:17.000000000","message":"Ah, the truth comes out :)","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0dc1b537d1489c4d0244c8dc5f8927d39bbaa926","unresolved":false,"context_lines":[{"line_number":306,"context_line":"There will be a hard coded limit of 50 traits for any Node to prevent miss-use"},{"line_number":307,"context_line":"of the API. This prevents denial of service attack where the database is filled"},{"line_number":308,"context_line":"up by a rogue user setting lots of traits. Really the limit is in place to"},{"line_number":309,"context_line":"match the limit applied in the placement API."},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"Other end user impact"},{"line_number":312,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_4195b83f","line":309,"in_reply_to":"3f4b6375_baf5cbcd","updated":"2017-10-20 10:17:05.000000000","message":"Hehe.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3f3f78fee25b856f4ae300949326c3955bf56009","unresolved":false,"context_lines":[{"line_number":354,"context_line":"* Add new traits API"},{"line_number":355,"context_line":"* Add openstack cli support for the new API"},{"line_number":356,"context_line":"* Follow up with Nova driver work"},{"line_number":357,"context_line":"* Clean up the remnants of partially complete tags work"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"Dependencies"},{"line_number":360,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_7aed13ee","line":357,"updated":"2017-10-19 15:41:17.000000000","message":"thx, i\u0027m fine if it isn\u0027t part of this spec; i just want to make sure we don\u0027t forget to do this.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0dc1b537d1489c4d0244c8dc5f8927d39bbaa926","unresolved":false,"context_lines":[{"line_number":354,"context_line":"* Add new traits API"},{"line_number":355,"context_line":"* Add openstack cli support for the new API"},{"line_number":356,"context_line":"* Follow up with Nova driver work"},{"line_number":357,"context_line":"* Clean up the remnants of partially complete tags work"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"Dependencies"},{"line_number":360,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f4b6375_61879411","line":357,"in_reply_to":"3f4b6375_7aed13ee","updated":"2017-10-20 10:17:05.000000000","message":"No, no, good call.","commit_id":"4d5eae593e15eb8f7a95df249871c69dca5d3e34"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"the RESOURCE_CLASS of each ironic node is reported today."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"Lets talk about the use case of dedicating specific Ironic nodes for use only"},{"line_number":57,"context_line":"by a specific set of projects. The remainder of the hosts are for general use."},{"line_number":58,"context_line":"If a user has a dedicated pool of resources, they have the ability to pick if"},{"line_number":59,"context_line":"they create an instance in their dedicated pool or in the general pool. Other"},{"line_number":60,"context_line":"users are only able to build in the general pool. One way to bisect the nodes"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_6d63fb1b","line":57,"range":{"start_line":57,"start_character":35,"end_line":57,"end_character":44},"updated":"2017-10-20 21:20:19.000000000","message":"yusss","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":57,"context_line":"by a specific set of projects. The remainder of the hosts are for general use."},{"line_number":58,"context_line":"If a user has a dedicated pool of resources, they have the ability to pick if"},{"line_number":59,"context_line":"they create an instance in their dedicated pool or in the general pool. Other"},{"line_number":60,"context_line":"users are only able to build in the general pool. One way to bisect the nodes"},{"line_number":61,"context_line":"like this is assigning traits such as `CUSTOM_IRONIC_NODE_PROJECT_B` and"},{"line_number":62,"context_line":"`CUSTOM_IRONIC_NODE_GENERAL_USE` to the appropriate ironic nodes. Then there"},{"line_number":63,"context_line":"is a public flavor to target the general pool of hosts, and a private project"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_4d66ff2b","line":60,"range":{"start_line":60,"start_character":61,"end_line":60,"end_character":67},"updated":"2017-10-20 21:20:19.000000000","message":"noyce","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":61,"context_line":"like this is assigning traits such as `CUSTOM_IRONIC_NODE_PROJECT_B` and"},{"line_number":62,"context_line":"`CUSTOM_IRONIC_NODE_GENERAL_USE` to the appropriate ironic nodes. Then there"},{"line_number":63,"context_line":"is a public flavor to target the general pool of hosts, and a private project"},{"line_number":64,"context_line":"specific flavor that targets their dedicated pool. By taking this approach it"},{"line_number":65,"context_line":"is easy to add additional pools of nodes for other sets of projects. It is"},{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_0d7007dc","line":64,"range":{"start_line":64,"start_character":35,"end_line":64,"end_character":44},"updated":"2017-10-20 21:20:19.000000000","message":"schweet\n\nThis paragraph is much clearer now, thank you.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8e09afa844d3699ab4ac85e179cbca4705552a10","unresolved":false,"context_lines":[{"line_number":63,"context_line":"is a public flavor to target the general pool of hosts, and a private project"},{"line_number":64,"context_line":"specific flavor that targets their dedicated pool. By taking this approach it"},{"line_number":65,"context_line":"is easy to add additional pools of nodes for other sets of projects. It is"},{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Note that ironic inspector rules can be used to set the initial value for a"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_8cabe487","line":66,"updated":"2017-10-20 13:38:41.000000000","message":"Perhaps this paragraph could be moved to a \u0027use cases\u0027 subsection before alternatives. Perhaps also we could have one or two other short use case descriptions?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":63,"context_line":"is a public flavor to target the general pool of hosts, and a private project"},{"line_number":64,"context_line":"specific flavor that targets their dedicated pool. By taking this approach it"},{"line_number":65,"context_line":"is easy to add additional pools of nodes for other sets of projects. It is"},{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Note that ironic inspector rules can be used to set the initial value for a"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_f79e74b6","line":66,"in_reply_to":"3f4b6375_8cabe487","updated":"2017-10-23 13:53:28.000000000","message":"I could, but I didn\u0027t want to move too far away from the ironic template.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8e09afa844d3699ab4ac85e179cbca4705552a10","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, we will also"},{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":89,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_ec8db8d3","line":86,"updated":"2017-10-20 13:38:41.000000000","message":"Will ironic validate that the instance traits are a subset of the node\u0027s traits? Will it prevent node traits requested by an instance from being removed from the node?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, we will also"},{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":89,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_ed0dab3e","line":86,"in_reply_to":"3f4b6375_ec8db8d3","updated":"2017-10-20 21:20:19.000000000","message":"\u003e Will it prevent node traits requested by an instance\n \u003e from being removed from the node?\n\nAfter deploy?  It shouldn\u0027t matter, should it?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, we will also"},{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":89,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_573668e8","line":86,"in_reply_to":"3f4b6375_ed0dab3e","updated":"2017-10-23 13:53:28.000000000","message":"Checking for matching trait list on deploy would help avoid any races around trait changes syncing into placement, I like it.\n\nI don\u0027t think we need to prevent changes, because there is no claim in placement that matches a trait, the trait simply helps pick the resource provider to claim against, which is tied to the ironic node uuid and not the traits.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":122,"context_line":"following the patterns from the API-WG and existing placement APIs. We could"},{"line_number":123,"context_line":"instead have just extended the existing ironic node PATCH interface. In terms"},{"line_number":124,"context_line":"of the CLI, regardless of the chosen API direction we would add it into the"},{"line_number":125,"context_line":"existing `openstack baremetal node set` and `unset` like this:"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"* openstack baremetal node set --trait CUSTOM_FOO \u003cnode-uuid\u003e"},{"line_number":128,"context_line":"* openstack baremetal node unset --trait CUSTOM_FOO \u003cnode-uuid\u003e"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_180cc339","line":125,"range":{"start_line":125,"start_character":35,"end_line":125,"end_character":51},"updated":"2017-10-20 21:20:19.000000000","message":"Missing some quotes here.  I think both should be ``double`` backquoted.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":156,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"A trait should be defined in a way that matches the placement API definition,"},{"line_number":159,"context_line":"as a Unicode string no longer than 255 characters in length."},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"State Machine Impact"},{"line_number":162,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_78285785","line":159,"range":{"start_line":159,"start_character":20,"end_line":159,"end_character":60},"updated":"2017-10-20 21:20:19.000000000","message":"\"...no more than 255 characters in length.\" or \"...no longer than 255 characters.\"","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8e09afa844d3699ab4ac85e179cbca4705552a10","unresolved":false,"context_lines":[{"line_number":171,"context_line":"`CUSTOM_`. Any trait set in Ironic must follow these rules, else the ironic"},{"line_number":172,"context_line":"Nova virt driver will be unable to add the traits in Placement. For similar"},{"line_number":173,"context_line":"reasons there is a limit of 50 traits on any node, to match the limit in"},{"line_number":174,"context_line":"Placement."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_2c72b0e4","line":174,"updated":"2017-10-20 13:38:41.000000000","message":"Response code for invalid format?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":171,"context_line":"`CUSTOM_`. Any trait set in Ironic must follow these rules, else the ironic"},{"line_number":172,"context_line":"Nova virt driver will be unable to add the traits in Placement. For similar"},{"line_number":173,"context_line":"reasons there is a limit of 50 traits on any node, to match the limit in"},{"line_number":174,"context_line":"Placement."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_97e74013","line":174,"in_reply_to":"3f4b6375_2c72b0e4","updated":"2017-10-23 13:53:28.000000000","message":"Should be 400 for an invalid trait, I can add that in.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":176,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":178,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":179,"context_line":"Ironic to assign arbitrary tags on particular Ironic nodes. Any non-standard"},{"line_number":180,"context_line":"traits simply need to have a prefix of \"CUSTOM_\" added."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"For convenience, it will be possible to get the full list of nodes and the"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_582b5b81","line":179,"range":{"start_line":179,"start_character":27,"end_line":179,"end_character":31},"updated":"2017-10-20 21:20:19.000000000","message":"traits","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"134985af3c95946e45253b98877a8ac1d0535320","unresolved":false,"context_lines":[{"line_number":177,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":178,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":179,"context_line":"Ironic to assign arbitrary tags on particular Ironic nodes. Any non-standard"},{"line_number":180,"context_line":"traits simply need to have a prefix of \"CUSTOM_\" added."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":183,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_0c9ff45f","line":180,"range":{"start_line":180,"start_character":39,"end_line":180,"end_character":48},"updated":"2017-10-20 13:28:49.000000000","message":"You have to escape the underscore, or sphinx looks for a hyperlink target (and fails cause it doesn\u0027t find one).\n\nI think the most readable way would be to make this an inline literal: ``CUSTOM_``","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8e09afa844d3699ab4ac85e179cbca4705552a10","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":231,"context_line":"    }"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"Example request to set all node traits to given list:::"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":236,"context_line":"    {"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_0c781428","line":233,"updated":"2017-10-20 13:38:41.000000000","message":"Are there any scenarios when setting or updating the traits for a node is not possible? Perhaps it would be restricted in the same way as the resource class? What are the response codes in the various failure cases?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":231,"context_line":"    }"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"Example request to set all node traits to given list:::"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    PUT /nodes/{node_ident}/traits"},{"line_number":236,"context_line":"    {"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_d784980e","line":233,"in_reply_to":"3f4b6375_0c781428","updated":"2017-10-23 13:53:28.000000000","message":"There is no claiming of traits, so I don\u0027t believe we need that.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"* openstack baremetal node add trait \u003cnode-ident\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":286,"context_line":"* openstack baremetal node remove trait \u003cnode-ident\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":287,"context_line":"* openstack baremetal node remove trait --all \u003cnode-ident\u003e"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"This is roughly copying the following command\u0027s syntax:"},{"line_number":290,"context_line":"https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/consistency-group.html#consistency-group-add-volume"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_58311ba2","line":287,"range":{"start_line":287,"start_character":40,"end_line":287,"end_character":45},"updated":"2017-10-20 21:20:19.000000000","message":"eh?","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"* openstack baremetal node add trait \u003cnode-ident\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":286,"context_line":"* openstack baremetal node remove trait \u003cnode-ident\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":287,"context_line":"* openstack baremetal node remove trait --all \u003cnode-ident\u003e"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"This is roughly copying the following command\u0027s syntax:"},{"line_number":290,"context_line":"https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/consistency-group.html#consistency-group-add-volume"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_f7bb14c1","line":287,"range":{"start_line":287,"start_character":40,"end_line":287,"end_character":45},"in_reply_to":"3f4b6375_58311ba2","updated":"2017-10-23 13:53:28.000000000","message":"See discussion on last patch with rloo, can make that clearer.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"8e09afa844d3699ab4ac85e179cbca4705552a10","unresolved":false,"context_lines":[{"line_number":292,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":293,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"},{"line_number":294,"context_line":"set meaning either an addition of a list of traits or replacing the whole list"},{"line_number":295,"context_line":"of traits."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"You can query the list of nodes using traits:"},{"line_number":298,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_8c83e4cc","line":295,"updated":"2017-10-20 13:38:41.000000000","message":"Although that operation is a valid API call - is the CLI not going to support it? Same for delete all.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":292,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":293,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"},{"line_number":294,"context_line":"set meaning either an addition of a list of traits or replacing the whole list"},{"line_number":295,"context_line":"of traits."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"You can query the list of nodes using traits:"},{"line_number":298,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_973f4024","line":295,"in_reply_to":"3f4b6375_588d9b2f","updated":"2017-10-23 13:53:28.000000000","message":"I replaced this with the above after review from rloo, I am not against either, probably needs discussion with OpenStack CLI folks.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":292,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":293,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"},{"line_number":294,"context_line":"set meaning either an addition of a list of traits or replacing the whole list"},{"line_number":295,"context_line":"of traits."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"You can query the list of nodes using traits:"},{"line_number":298,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_588d9b2f","line":295,"in_reply_to":"3f4b6375_8c83e4cc","updated":"2017-10-20 21:20:19.000000000","message":"I prefer the consistency of using node set/unset.\n\n openstack baremetal node set --trait name [name...] \u003cnode-ident\u003e\n openstack baremetal node unset --trait name [name...] \u003cnode-ident\u003e\n openstack baremetal node unset --all-traits \u003cnode-ident\u003e","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":292,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":293,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"},{"line_number":294,"context_line":"set meaning either an addition of a list of traits or replacing the whole list"},{"line_number":295,"context_line":"of traits."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"You can query the list of nodes using traits:"},{"line_number":298,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_07cf7456","line":295,"in_reply_to":"3f4b6375_973f4024","updated":"2017-10-24 21:08:12.000000000","message":"So I\u0027m fine with set/unset, or add/remove, but not both.\n\nThe problem I see with set, is that it isn\u0027t as clear what you are doing. Eg, you need --all-traits, to unset them all. How would you distinguish set (replacing existing traits) vs set (adding a trait to existing set of traits) ? It isn\u0027t clear to me what \u0027set\u0027 should do although if i had to choose, it would be replacing any existing traits. Whereas I think that \u0027add\u0027 is clear in that it is adding, not replacing. So do we want both set \u0026 add? (ugh).","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":324,"context_line":"Security impact"},{"line_number":325,"context_line":"---------------"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"There will be a hard coded limit of 50 traits for any Node to prevent miss-use"},{"line_number":328,"context_line":"of the API. This prevents denial of service attack where the database is filled"},{"line_number":329,"context_line":"up by a rogue user setting lots of traits. Really the limit is in place to"},{"line_number":330,"context_line":"match the limit applied in the placement API."}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_9872f348","line":327,"range":{"start_line":327,"start_character":70,"end_line":327,"end_character":78},"updated":"2017-10-20 21:20:19.000000000","message":"misuse","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3b65e113406d65814724a8fe13c23c00557c58b1","unresolved":false,"context_lines":[{"line_number":332,"context_line":"Other end user impact"},{"line_number":333,"context_line":"---------------------"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"Question: should we first register the tag in placement, or will nova driver"},{"line_number":336,"context_line":"just add any missing custom tags in placement?"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"Scalability impact"},{"line_number":339,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_58bb5bc3","line":336,"range":{"start_line":335,"start_character":0,"end_line":336,"end_character":46},"updated":"2017-10-20 21:20:19.000000000","message":"The latter.  You decided on L176 that you won\u0027t talk directly to placement.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ab9da465a50639317eee78e554d4008212b75bfb","unresolved":false,"context_lines":[{"line_number":332,"context_line":"Other end user impact"},{"line_number":333,"context_line":"---------------------"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"Question: should we first register the tag in placement, or will nova driver"},{"line_number":336,"context_line":"just add any missing custom tags in placement?"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"Scalability impact"},{"line_number":339,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f4b6375_773a8434","line":336,"range":{"start_line":335,"start_character":0,"end_line":336,"end_character":46},"in_reply_to":"3f4b6375_58bb5bc3","updated":"2017-10-23 13:53:28.000000000","message":"oops, this is left behind.","commit_id":"8901a18eef969e13e0933ff0f9119276278b538e"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, we will also"},{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`. When the traits field is populated we"},{"line_number":87,"context_line":"should do validation to ensure it is a subset of the traits current set on the"},{"line_number":88,"context_line":"Ironic node, in particular we should ensure this is the case when a deploy"},{"line_number":89,"context_line":"happens. This stops any problems with traits having changed since the last sync"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_0ce55b09","line":86,"updated":"2017-10-24 21:08:12.000000000","message":"Which traits field are we talking about? the node\u0027s instance_info[\u0027traits\u0027] field, or the \u0027traits\u0027 field ? I\u0027m guessing you mean instance_info[\u0027traits\u0027]. If so, maybe s/the traits/this instance_info[\u0027traits\u0027]/\n\nAs a comment, you use \u0027we\u0027 for us developers I think, but it makes it unclear *where* the stuff is done, in ironic virt driver, or ironic code. Cuz \u0027we\u0027 is you and you are going to do changes in both :)\n\nI\u0027m guessing the validation is going to be done in ironic. In nova-ironic-virt driver\u0027s spawn(), that instance stuff is set, then the API request \u0027node.validate\u0027 is issued -- that\u0027s where the validation is going to be done, right? And all this work is outside the scope of this spec.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"494c8f7b092e8af9b7d7e57e9a772cc7a5a1477f","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, we will also"},{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`. When the traits field is populated we"},{"line_number":87,"context_line":"should do validation to ensure it is a subset of the traits current set on the"},{"line_number":88,"context_line":"Ironic node, in particular we should ensure this is the case when a deploy"},{"line_number":89,"context_line":"happens. This stops any problems with traits having changed since the last sync"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_1f19e920","line":86,"in_reply_to":"3f4b6375_0ce55b09","updated":"2017-10-25 11:20:57.000000000","message":"I think the node.validate stuff has to be in the scope of this spec. I should straighten that out.\n\nI need to get out of the habbit of using \"we\", its very unclear.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":85,"context_line":"include the flavor extra specs that define what traits have been requested,"},{"line_number":86,"context_line":"setting those in `/instance_info/traits`. When the traits field is populated we"},{"line_number":87,"context_line":"should do validation to ensure it is a subset of the traits current set on the"},{"line_number":88,"context_line":"Ironic node, in particular we should ensure this is the case when a deploy"},{"line_number":89,"context_line":"happens. This stops any problems with traits having changed since the last sync"},{"line_number":90,"context_line":"to placement causing unexpected results."},{"line_number":91,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_cceaa3d6","line":88,"updated":"2017-10-24 21:08:12.000000000","message":"maybe s/Ironic node/Ironic node\u0027s \u0027trait\u0027 field/\n\nAnd i\u0027d probably start a new sentence, \"In particular, ...\"","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":125,"context_line":"The REST API described below allows fine grained control of setting traits,"},{"line_number":126,"context_line":"following the patterns from the API-WG and existing placement APIs. We could"},{"line_number":127,"context_line":"instead have just extended the existing ironic node PATCH interface. In terms"},{"line_number":128,"context_line":"of the CLI, regardless of the chosen API direction we would add it into the"},{"line_number":129,"context_line":"existing ``openstack baremetal node set`` and ``unset`` like this:"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"* openstack baremetal node set --trait CUSTOM_FOO \u003cnode-uuid\u003e"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_277f1016","line":128,"updated":"2017-10-24 21:08:12.000000000","message":"This is in the alternatives section. Does this mean that *if* we were to have it added to the .. node set \u0026 node unset, we\u0027d do the following (except that we aren\u0027t going to do it)?","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"494c8f7b092e8af9b7d7e57e9a772cc7a5a1477f","unresolved":false,"context_lines":[{"line_number":125,"context_line":"The REST API described below allows fine grained control of setting traits,"},{"line_number":126,"context_line":"following the patterns from the API-WG and existing placement APIs. We could"},{"line_number":127,"context_line":"instead have just extended the existing ironic node PATCH interface. In terms"},{"line_number":128,"context_line":"of the CLI, regardless of the chosen API direction we would add it into the"},{"line_number":129,"context_line":"existing ``openstack baremetal node set`` and ``unset`` like this:"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"* openstack baremetal node set --trait CUSTOM_FOO \u003cnode-uuid\u003e"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_ffea8dd5","line":128,"in_reply_to":"3f4b6375_277f1016","updated":"2017-10-25 11:20:57.000000000","message":"Ah, this has drifted out of sync, I should fix it.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":182,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":183,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":184,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":185,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":188,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_ac9647ef","line":185,"updated":"2017-10-24 21:08:12.000000000","message":"Sorry, continuing the discussion in patch set 67. Personally, if we can design/plan this to handle both standalone AND with nova, then that is great. If we had to prioritize, support with nova is what we/some of us want first, but there are folks using ironic w/o nova, and the older tags feature could support this. I don\u0027t see why, for standalone, we should impose that all traits have to conform to strings in os-traits or with this \"CUSTOM_\" prefix.\n\nI am fine with this behaviour for now; as long as there\u0027s a simple way to NOT have this behaviour in the future, if someone wants it.\n\nMaybe we ought to ask folks that use standalone ironic about this.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"494c8f7b092e8af9b7d7e57e9a772cc7a5a1477f","unresolved":false,"context_lines":[{"line_number":182,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":183,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":184,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":185,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":188,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_ff984d75","line":185,"in_reply_to":"3f4b6375_ac9647ef","updated":"2017-10-25 11:20:57.000000000","message":"I still think its a bad idea to have both API \"modes\", it breaks the API really.\n\nI don\u0027t understand why this restricts how the standalone folks would use this. I get it means they can\u0027t set the tag \"FOO\" they have to set \"CUSTOM_FOO\", but that means they gain the ability to use standard traits, and know what those mean.\n\nIf that is all a massive issue, then maybe we finish off tags rather than deleting the unfinished tags code. But that seems like a bad idea.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"530b9a04aea8fec507058f79c12d8cf5415b4784","unresolved":false,"context_lines":[{"line_number":182,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":183,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":184,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":185,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":188,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_45966f4e","line":185,"in_reply_to":"3f4b6375_ff704d0a","updated":"2017-10-25 13:56:55.000000000","message":"i\u0027ll send out email to the list and see if anyone has any concerns about this.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"605f4b32cc4aa7fbdf7fda19bea7e5d14fe689fa","unresolved":false,"context_lines":[{"line_number":182,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":183,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":184,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":185,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":188,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_ff704d0a","line":185,"in_reply_to":"3f4b6375_ff984d75","updated":"2017-10-25 11:29:45.000000000","message":"I agree with John on multiple modes. You make a good point. This is a traits API. If a standalone user wants to use it differently then they\u0027re not using traits, it\u0027s using something else.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":253,"context_line":"although the Resource Class and Ironic node are used in the allocations in"},{"line_number":254,"context_line":"placement, traits are not used in allocations."},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"In a similar way the following API removes all the traits:::"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    DELETE /nodes/{node_ident}/traits"},{"line_number":259,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_a7eae00b","line":256,"updated":"2017-10-24 21:08:12.000000000","message":"extra \u0027:\u0027","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":259,"context_line":""},{"line_number":260,"context_line":"The response on success is status code 204, with an empty body."},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"To add or remove an individual trait use:::"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    PUT /nodes/{node_ident}/traits/CUSTOM_FOO"},{"line_number":265,"context_line":"    \u003cno body\u003e"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_87efe41c","line":262,"updated":"2017-10-24 21:08:12.000000000","message":"extra \u0027:\u0027","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    DELETE /nodes/{node_ident}/traits/CUSTOM_FOO"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"Filtering the node list by traits should work as expected:::"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    GET /nodes?traits\u003dCUSTOM_RED,CUSTOM_BLUE"},{"line_number":272,"context_line":"    GET /nodes?not-traits\u003dCUSTOM_RED,CUSTOM_BLUE\u0026traits\u003dCUSTOM_FOO"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_27def06c","line":269,"updated":"2017-10-24 21:08:12.000000000","message":"ditto","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"20530f3d428107b05bba8edfb0556bbbf4dd9e63","unresolved":false,"context_lines":[{"line_number":276,"context_line":"As mentioned above, the final change that is made is to ensure that when"},{"line_number":277,"context_line":"``instance_info/traits`` is set via a PATCH call on an ironic node, we ensure"},{"line_number":278,"context_line":"that is a subset of the traits set on the node at that time. In addition, when"},{"line_number":279,"context_line":"a node is deployed, we ensure the instance_info/traits is either empty or is a"},{"line_number":280,"context_line":"subset of the traits set on the node."},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"Client (CLI) impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_a785405c","line":279,"updated":"2017-10-24 21:08:12.000000000","message":"I wonder if it is sufficient to do it here, via the node.validate call [1], instead of also in a PATCH /nodes/{node_ident} {op:add, path /instance/traits, ...}. If you DID validate in the PATCH, what http status would you return if it isn\u0027t a subset?\n\n[1] https://developer.openstack.org/api-ref/baremetal/#validate-node","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"530b9a04aea8fec507058f79c12d8cf5415b4784","unresolved":false,"context_lines":[{"line_number":276,"context_line":"As mentioned above, the final change that is made is to ensure that when"},{"line_number":277,"context_line":"``instance_info/traits`` is set via a PATCH call on an ironic node, we ensure"},{"line_number":278,"context_line":"that is a subset of the traits set on the node at that time. In addition, when"},{"line_number":279,"context_line":"a node is deployed, we ensure the instance_info/traits is either empty or is a"},{"line_number":280,"context_line":"subset of the traits set on the node."},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"Client (CLI) impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_e5eafbd5","line":279,"in_reply_to":"3f4b6375_3fa14595","updated":"2017-10-25 13:56:55.000000000","message":"Our code internally always does a validate before deploying [1]. Although now that we\u0027ve added more interfaces, I wonder if we need to add other interface validations there.\n\n[1] https://github.com/openstack/ironic/blob/6525a0da29b937f26ae855175265f539bfbee591/ironic/conductor/manager.py#L596","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"494c8f7b092e8af9b7d7e57e9a772cc7a5a1477f","unresolved":false,"context_lines":[{"line_number":276,"context_line":"As mentioned above, the final change that is made is to ensure that when"},{"line_number":277,"context_line":"``instance_info/traits`` is set via a PATCH call on an ironic node, we ensure"},{"line_number":278,"context_line":"that is a subset of the traits set on the node at that time. In addition, when"},{"line_number":279,"context_line":"a node is deployed, we ensure the instance_info/traits is either empty or is a"},{"line_number":280,"context_line":"subset of the traits set on the node."},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"Client (CLI) impact"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3f4b6375_3fa14595","line":279,"in_reply_to":"3f4b6375_a785405c","updated":"2017-10-25 11:20:57.000000000","message":"Good point, I like that approach, as long as we do a validate during the deploy.\n\nNeeds more thought.\n\nFWIW, I was thinking you return 400 and apply none of the changes in the PATCH.","commit_id":"47ee3e6b27ea7dc10d3b3671c99f9210c35317a0"},{"author":{"_account_id":11076,"name":"Shivanand Tendulker","email":"stendulker@gmail.com","username":"stendulker"},"change_message_id":"115aa1a7f5af55e6971612b4462d5cb914c6e043","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, the ironic virt"},{"line_number":85,"context_line":"driver will also include the flavor extra specs that define what traits have"},{"line_number":86,"context_line":"been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_f152a812","line":86,"range":{"start_line":84,"start_character":41,"end_line":86,"end_character":55},"updated":"2017-10-26 05:19:31.000000000","message":"Will Ironic continue to consume instance_info[\u0027capabilities\u0027] as it is doing today? Ironic configures the node based on these capabilities. Ex. boot mode, boot_option etc.\n\nThough it is beyond purview of Ironic as operator needs to add traits, there could be conflicting data in instance_info[\u0027traits\u0027] and instance_info[\u0027capabilities\u0027]. Ex. trait may indicate a node with \u0027uefi\u0027 boot mode and \u0027capabilities\u0027 may request \u0027bios\u0027. This may be something Ironic may just need to document...","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":10202,"name":"Wan-yen Hsu","email":"wanyenhsu@gmail.com"},"change_message_id":"3dfba112fc847c7a9ec62d92d3d0404310779351","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, the ironic virt"},{"line_number":85,"context_line":"driver will also include the flavor extra specs that define what traits have"},{"line_number":86,"context_line":"been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_bb0bd48b","line":86,"range":{"start_line":84,"start_character":41,"end_line":86,"end_character":55},"in_reply_to":"3f4b6375_48342dde","updated":"2017-10-26 23:17:05.000000000","message":"Capabilities can handle quantitative attributes.  It can be used to support important use cases such as GPU count and SSD capacity, etc.    Quantity matters for workloads running on bare metals.   For instance, a user wants to run 3 containers on a bare metal.  He/she want each container to have a GPU device.  Therefore, a bare metal node with at least 3 GPUs will be required.   As far as I know, Capabilities are widely used, please do not take it away from users and cause regression.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"cc03adac927e6de56ac26073ceee8e16d067d930","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, the ironic virt"},{"line_number":85,"context_line":"driver will also include the flavor extra specs that define what traits have"},{"line_number":86,"context_line":"been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_cfca1219","line":86,"range":{"start_line":84,"start_character":41,"end_line":86,"end_character":55},"in_reply_to":"3f4b6375_bb0bd48b","updated":"2017-10-30 13:48:21.000000000","message":"John, if you want, you could mention that the issue/future of instance_info[\u0027capabilities\u0027] is outside the scope of this spec.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, the ironic virt"},{"line_number":85,"context_line":"driver will also include the flavor extra specs that define what traits have"},{"line_number":86,"context_line":"been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_d41dfd73","line":86,"range":{"start_line":84,"start_character":41,"end_line":86,"end_character":55},"in_reply_to":"3f4b6375_cfca1219","updated":"2017-10-30 17:34:58.000000000","message":"I have already stated that on line 420, at least that is what I had intended to express down there.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9e43399512ba365f23a9bb7dbd76e73650c971a9","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently extra_specs starting with"},{"line_number":84,"context_line":"`capabilities` are set in instance_info. After the above spec, the ironic virt"},{"line_number":85,"context_line":"driver will also include the flavor extra specs that define what traits have"},{"line_number":86,"context_line":"been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_48342dde","line":86,"range":{"start_line":84,"start_character":41,"end_line":86,"end_character":55},"in_reply_to":"3f4b6375_f152a812","updated":"2017-10-26 08:34:22.000000000","message":"Will need to work through that. I think we discussed removing capabilities in the longer term, but we need to make sure traits gives us all we need first.\n\nFor now, its only about placement of the nodes. That is step 1.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"bf244dc85a236aa22bc7e048d9d473af95d27121","unresolved":false,"context_lines":[{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"},{"line_number":90,"context_line":"`instance_info[\u0027traits\u0027]` only includes traits that are already in the list"},{"line_number":91,"context_line":"of traits set on the Ironic node. While useful for the standalone mode, this"},{"line_number":92,"context_line":"is particularly useful in preventing strange behaviour due to races between"},{"line_number":93,"context_line":"the update of an Ironic node\u0027s trait list and getting that list copied into the"},{"line_number":94,"context_line":"Placement API by the Ironic virt driver. This validation is probably best"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_4509612f","line":91,"range":{"start_line":91,"start_character":40,"end_line":91,"end_character":46},"updated":"2017-10-30 15:23:44.000000000","message":"did you mean \u0027not useful for the standalone mode\u0027 here? Otherwise, I might be missing something","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":88,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":89,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"},{"line_number":90,"context_line":"`instance_info[\u0027traits\u0027]` only includes traits that are already in the list"},{"line_number":91,"context_line":"of traits set on the Ironic node. While useful for the standalone mode, this"},{"line_number":92,"context_line":"is particularly useful in preventing strange behaviour due to races between"},{"line_number":93,"context_line":"the update of an Ironic node\u0027s trait list and getting that list copied into the"},{"line_number":94,"context_line":"Placement API by the Ironic virt driver. This validation is probably best"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_5afed705","line":91,"range":{"start_line":91,"start_character":40,"end_line":91,"end_character":46},"in_reply_to":"3f4b6375_4509612f","updated":"2017-10-30 17:34:58.000000000","message":"You can do the same thing Nova does in standalone mode, so this would keep things consistent in that case too.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"cc03adac927e6de56ac26073ceee8e16d067d930","unresolved":false,"context_lines":[{"line_number":142,"context_line":"Data model impact"},{"line_number":143,"context_line":"-----------------"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"The following partially implemented spec has already added some data tables"},{"line_number":146,"context_line":"for tags:"},{"line_number":147,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"This change supersedes the idea of tags, so the above tables should be removed"},{"line_number":150,"context_line":"and replaced with a similar traits table::"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    CREATE TABLE node_traits ("},{"line_number":153,"context_line":"        node_id INT(11) NOT NULL,"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_f2fdb108","line":150,"range":{"start_line":145,"start_character":0,"end_line":150,"end_character":17},"updated":"2017-10-30 13:48:21.000000000","message":"Nope. After seeing the comments in [1], I think we need to provide both tags and traits. At the very least, just take out any mention of tags from this spec. If the decision is to remove tags, that can be done separately, but I am leaning towards keeping it.\n[1] http://lists.openstack.org/pipermail/openstack-dev/2017-October/123986.html","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":142,"context_line":"Data model impact"},{"line_number":143,"context_line":"-----------------"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"The following partially implemented spec has already added some data tables"},{"line_number":146,"context_line":"for tags:"},{"line_number":147,"context_line":"http://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/nodes-tagging.html"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"This change supersedes the idea of tags, so the above tables should be removed"},{"line_number":150,"context_line":"and replaced with a similar traits table::"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    CREATE TABLE node_traits ("},{"line_number":153,"context_line":"        node_id INT(11) NOT NULL,"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_fa5a2328","line":150,"range":{"start_line":145,"start_character":0,"end_line":150,"end_character":17},"in_reply_to":"3f4b6375_f2fdb108","updated":"2017-10-30 17:34:58.000000000","message":"Yeah, good discussion on there. Lets keep it separate.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"bf244dc85a236aa22bc7e048d9d473af95d27121","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"No impact."},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"REST API impact"},{"line_number":175,"context_line":"---------------"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The placement API defines a set of standard traits in the `os-traits` library."}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_680c3442","line":174,"range":{"start_line":174,"start_character":0,"end_line":174,"end_character":15},"updated":"2017-10-30 15:23:44.000000000","message":"I think you intend to do all the actions with traits in the api itself, without adding or reusing any of existing rpc methods, right?","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"No impact."},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"REST API impact"},{"line_number":175,"context_line":"---------------"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The placement API defines a set of standard traits in the `os-traits` library."}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_5a75b797","line":174,"range":{"start_line":174,"start_character":0,"end_line":174,"end_character":15},"in_reply_to":"3f4b6375_680c3442","updated":"2017-10-30 17:34:58.000000000","message":"Yes, I assume that is the normal thing to do for this kind of thing?","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"bf244dc85a236aa22bc7e048d9d473af95d27121","unresolved":false,"context_lines":[{"line_number":391,"context_line":"Work Items"},{"line_number":392,"context_line":"----------"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"* Update tables to store traits for nodes"},{"line_number":395,"context_line":"* Add objects to expose the tables"},{"line_number":396,"context_line":"* Add new traits API"},{"line_number":397,"context_line":"* Add openstack cli support for the new API"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_436279da","line":394,"updated":"2017-10-30 15:23:44.000000000","message":"Isn\u0027t it \u0027Add table to store...\u0027?","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":391,"context_line":"Work Items"},{"line_number":392,"context_line":"----------"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"* Update tables to store traits for nodes"},{"line_number":395,"context_line":"* Add objects to expose the tables"},{"line_number":396,"context_line":"* Add new traits API"},{"line_number":397,"context_line":"* Add openstack cli support for the new API"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_7423e9bf","line":394,"in_reply_to":"3f4b6375_436279da","updated":"2017-10-30 17:34:58.000000000","message":"ops, yes.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"cc03adac927e6de56ac26073ceee8e16d067d930","unresolved":false,"context_lines":[{"line_number":396,"context_line":"* Add new traits API"},{"line_number":397,"context_line":"* Add openstack cli support for the new API"},{"line_number":398,"context_line":"* Follow up with Nova driver work"},{"line_number":399,"context_line":"* Ensure we clean up the remnants of the partially complete tags work"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"Dependencies"},{"line_number":402,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_323469de","line":399,"updated":"2017-10-30 13:48:21.000000000","message":"remove this if we\u0027re not replacing tags","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":10202,"name":"Wan-yen Hsu","email":"wanyenhsu@gmail.com"},"change_message_id":"3dfba112fc847c7a9ec62d92d3d0404310779351","unresolved":false,"context_lines":[{"line_number":417,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"Longer term, capabilities and other APIs may be phased out. We are not"},{"line_number":420,"context_line":"considering that as part of this spec."},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"Documentation Impact"},{"line_number":423,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_9b449875","line":420,"range":{"start_line":420,"start_character":37,"end_line":420,"end_character":38},"updated":"2017-10-26 23:17:05.000000000","message":"Unless we can expand trait to provide equivalent features and flexibility of \"Capabilities\", please do not deprecate \"Capabilities\" to cause regression.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"93b06e71b5e0e2a4c78dba0c032ccc3116df960a","unresolved":false,"context_lines":[{"line_number":417,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"Longer term, capabilities and other APIs may be phased out. We are not"},{"line_number":420,"context_line":"considering that as part of this spec."},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"Documentation Impact"},{"line_number":423,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":12,"id":"3f4b6375_7a8d137a","line":420,"range":{"start_line":420,"start_character":37,"end_line":420,"end_character":38},"in_reply_to":"3f4b6375_9b449875","updated":"2017-10-30 17:34:58.000000000","message":"Totally agreed. This is hinting at the plan from the PTG to slowly replace all existing features, with a transition path, so there is no regression. I can explicitly state the need for feature parity before any removal, I just assumed that point.","commit_id":"30ffb404abc214a7c00c6c6ff4a7e425a2220e58"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":45,"context_line":"resource provider, and the implementation of the following Nova specs that"},{"line_number":46,"context_line":"ensure the traits requested in flavors are honored by the scheduling process:"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/request-traits-in-nova.rst"},{"line_number":49,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/add-trait-support-in-allocation-candidates.rst"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"The current proposal is for Ironic to store a list of traits for each node."},{"line_number":52,"context_line":"That list can then be synced into the appropriate resource provider in the"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_3a1e7b70","line":49,"range":{"start_line":48,"start_character":0,"end_line":49,"end_character":126},"updated":"2017-10-30 18:15:09.000000000","message":"nit: let\u0027s link to rendered specs","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":45,"context_line":"resource provider, and the implementation of the following Nova specs that"},{"line_number":46,"context_line":"ensure the traits requested in flavors are honored by the scheduling process:"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/request-traits-in-nova.rst"},{"line_number":49,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/add-trait-support-in-allocation-candidates.rst"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"The current proposal is for Ironic to store a list of traits for each node."},{"line_number":52,"context_line":"That list can then be synced into the appropriate resource provider in the"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_ab448a48","line":49,"range":{"start_line":48,"start_character":0,"end_line":49,"end_character":126},"in_reply_to":"3f4b6375_3a1e7b70","updated":"2017-11-08 21:07:44.000000000","message":"yeah, they were not updating properly when I wrote this.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Note that ironic inspector rules can be used to set the initial value for a"},{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_1a13bf68","line":69,"range":{"start_line":69,"start_character":10,"end_line":69,"end_character":26},"updated":"2017-10-30 18:15:09.000000000","message":"nit: s/ironic inspector/introspection/ (this is how we call them, and we really hope to make them generic)","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Note that ironic inspector rules can be used to set the initial value for a"},{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_abe00a28","line":69,"range":{"start_line":69,"start_character":10,"end_line":69,"end_character":26},"in_reply_to":"3f4b6375_1a13bf68","updated":"2017-11-08 21:07:44.000000000","message":"sure.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"We extend the node to include not only Resource Class, but also a list of"},{"line_number":74,"context_line":"associated traits."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"There is already a Nova spec proposed discussing the Nova side of how we can"},{"line_number":77,"context_line":"assign traits to the resource providers, and on Nova boot send the chosen"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_da0c0744","line":74,"range":{"start_line":73,"start_character":0,"end_line":74,"end_character":18},"updated":"2017-10-30 18:15:09.000000000","message":"this sentence feels out of order here, and duplicates what is written above","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"We extend the node to include not only Resource Class, but also a list of"},{"line_number":74,"context_line":"associated traits."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"There is already a Nova spec proposed discussing the Nova side of how we can"},{"line_number":77,"context_line":"assign traits to the resource providers, and on Nova boot send the chosen"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_f91dca0e","line":74,"range":{"start_line":73,"start_character":0,"end_line":74,"end_character":18},"in_reply_to":"3f4b6375_da0c0744","updated":"2017-11-07 22:54:40.000000000","message":"+1","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"We extend the node to include not only Resource Class, but also a list of"},{"line_number":74,"context_line":"associated traits."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"There is already a Nova spec proposed discussing the Nova side of how we can"},{"line_number":77,"context_line":"assign traits to the resource providers, and on Nova boot send the chosen"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_8bcf8eb2","line":74,"range":{"start_line":73,"start_character":0,"end_line":74,"end_character":18},"in_reply_to":"3f4b6375_f91dca0e","updated":"2017-11-08 21:07:44.000000000","message":"+1","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":76,"context_line":"There is already a Nova spec proposed discussing the Nova side of how we can"},{"line_number":77,"context_line":"assign traits to the resource providers, and on Nova boot send the chosen"},{"line_number":78,"context_line":"traits back to ironic:"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/ironic-driver-traits.rst"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_39dda240","line":79,"updated":"2017-11-07 22:54:40.000000000","message":"This seems like a contextual note:: syntax worthy item. Perhaps move out of the main body of what is being described?","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/ironic-driver-traits.rst"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note when provisioning a node, the Ironic virt driver in Nova will now send"},{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently only extra_specs starting"},{"line_number":84,"context_line":"with `capabilities` are set in instance_info. After the above spec, the ironic"},{"line_number":85,"context_line":"virt driver will also include the flavor extra specs that define what traits"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_fd0c5d3c","line":82,"range":{"start_line":82,"start_character":0,"end_line":82,"end_character":5},"updated":"2017-10-30 18:15:09.000000000","message":"nit: s/Note//","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently only extra_specs starting"},{"line_number":84,"context_line":"with `capabilities` are set in instance_info. After the above spec, the ironic"},{"line_number":85,"context_line":"virt driver will also include the flavor extra specs that define what traits"},{"line_number":86,"context_line":"have been requested, storing them `instance_info[\u0027traits\u0027]`. Note there is no"},{"line_number":87,"context_line":"change to how capabilities are used as part of this spec."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":90,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_1d085926","line":87,"range":{"start_line":86,"start_character":61,"end_line":87,"end_character":57},"updated":"2017-10-30 18:15:09.000000000","message":"let\u0027s convert notes to note syntax\n\n .. note::\n     There is no change to ...","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"additional flavor extra_specs to Ironic. Currently only extra_specs starting"},{"line_number":84,"context_line":"with `capabilities` are set in instance_info. After the above spec, the ironic"},{"line_number":85,"context_line":"virt driver will also include the flavor extra specs that define what traits"},{"line_number":86,"context_line":"have been requested, storing them `instance_info[\u0027traits\u0027]`. Note there is no"},{"line_number":87,"context_line":"change to how capabilities are used as part of this spec."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":90,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_79643ab9","line":87,"range":{"start_line":86,"start_character":61,"end_line":87,"end_character":57},"in_reply_to":"3f4b6375_1d085926","updated":"2017-11-07 22:54:40.000000000","message":"Agreed, but I think that is a viable follow-up.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":90,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"},{"line_number":91,"context_line":"`instance_info[\u0027traits\u0027]` only includes traits that are already in the list"},{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"},{"line_number":93,"context_line":"strange behaviour due to races between the update of an Ironic node\u0027s trait"},{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_19fae6bb","line":91,"range":{"start_line":91,"start_character":0,"end_line":91,"end_character":24},"updated":"2017-11-07 22:54:40.000000000","message":"Have we double checked that we have enough storage for instance_info?","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":90,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"},{"line_number":91,"context_line":"`instance_info[\u0027traits\u0027]` only includes traits that are already in the list"},{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"},{"line_number":93,"context_line":"strange behaviour due to races between the update of an Ironic node\u0027s trait"},{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_ba15eee7","line":91,"range":{"start_line":91,"start_character":0,"end_line":91,"end_character":24},"in_reply_to":"3f4b6375_19fae6bb","updated":"2017-11-08 21:07:44.000000000","message":"We have 64Kb, as its Text. That sounds OK, but its a good question.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"},{"line_number":93,"context_line":"strange behaviour due to races between the update of an Ironic node\u0027s trait"},{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"},{"line_number":95,"context_line":"driver. This validation is probably best integrated as part of"},{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_9d1b2972","line":97,"range":{"start_line":95,"start_character":8,"end_line":97,"end_character":44},"updated":"2017-10-30 18:15:09.000000000","message":"No, I think it will be done in generic code. Driver-level validation will happen what setting traits on nodes, not here.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"},{"line_number":93,"context_line":"strange behaviour due to races between the update of an Ironic node\u0027s trait"},{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"},{"line_number":95,"context_line":"driver. This validation is probably best integrated as part of"},{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_b9b07241","line":97,"range":{"start_line":95,"start_character":8,"end_line":97,"end_character":44},"in_reply_to":"3f4b6375_9d1b2972","updated":"2017-11-07 22:54:40.000000000","message":"I disagree, that is unless we want to build entirely new interfaces. If I remember correctly, nova presently performs its last go/no-go check based upon our deploy interface validation reply. Of course, we may just be using different words.\n\nThat being said, validation code _should_ common shared code across deploy interfaces, and those deploy interfaces could then could call the common code to process the sanity check as necessary. I\u0027m sure there are some drivers where it will also not matter as well, but an additional method in task.driver.deploy.validate\u0027s execution seems fine for our general use cases.\n\nI this can be clarified in a follow-up.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"},{"line_number":93,"context_line":"strange behaviour due to races between the update of an Ironic node\u0027s trait"},{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"},{"line_number":95,"context_line":"driver. This validation is probably best integrated as part of"},{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_7abe56d5","line":97,"range":{"start_line":95,"start_character":8,"end_line":97,"end_character":44},"in_reply_to":"3f4b6375_b9b07241","updated":"2017-11-08 21:07:44.000000000","message":"I am probably leaning too heavily on \"or similar\" here :)\n\nIt seems to fit in deploy.validate, quite how I get it in there is a different question for the code reivew I think.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"},{"line_number":101,"context_line":"done today. Should an operator talk directly to Placement to adjust the traits,"},{"line_number":102,"context_line":"this process will remove all those modifications when the next sync occurs."}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_3d2c3598","line":99,"range":{"start_line":99,"start_character":0,"end_line":99,"end_character":6},"updated":"2017-10-30 18:15:09.000000000","message":".. note::","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"},{"line_number":101,"context_line":"done today. Should an operator talk directly to Placement to adjust the traits,"},{"line_number":102,"context_line":"this process will remove all those modifications when the next sync occurs."}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_99bdb6d9","line":99,"range":{"start_line":99,"start_character":15,"end_line":99,"end_character":26},"updated":"2017-11-07 22:54:40.000000000","message":"We should likely note it is the ironic virt driver in nova.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Note: the Nova virt driver will sync the list of traits reported by each Ironic"},{"line_number":100,"context_line":"Node into Nova\u0027s Placement API, in a similar way to how the Resource Class is"},{"line_number":101,"context_line":"done today. Should an operator talk directly to Placement to adjust the traits,"},{"line_number":102,"context_line":"this process will remove all those modifications when the next sync occurs."}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_597bbe79","line":99,"range":{"start_line":99,"start_character":0,"end_line":99,"end_character":6},"in_reply_to":"3f4b6375_3d2c3598","updated":"2017-11-07 22:54:40.000000000","message":"Agree, would be perfect to just submit as a follow-up.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":156,"context_line":"          ON DELETE CASCADE,"},{"line_number":157,"context_line":"    )"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"A new ironic.objects.traits.NodeTraitList object will be added to the object"},{"line_number":160,"context_line":"model. The ironic.objects.traits.NodeTraitList field in the python object model"},{"line_number":161,"context_line":"will be populated on-demand (i.e. not eager-loaded)."},{"line_number":162,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_bd3705e4","line":159,"range":{"start_line":159,"start_character":6,"end_line":159,"end_character":41},"updated":"2017-10-30 18:15:09.000000000","message":"nit: `` around python object paths","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":180,"context_line":"the status code 400."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":183,"context_line":"depends only on access to the python library `os-traits`. As such, this"},{"line_number":184,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":185,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":186,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added."}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_dd3f61b3","line":183,"range":{"start_line":183,"start_character":45,"end_line":183,"end_character":56},"updated":"2017-10-30 18:15:09.000000000","message":"nit: let\u0027s make this a link to os-traits docs","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    {"},{"line_number":249,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":250,"context_line":"    }"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Note that unlike with Resource Class, we are allowing the trait to be updated"},{"line_number":253,"context_line":"at any time. This is mostly because placement allows such updates and because"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_1d0e19e5","line":250,"updated":"2017-10-30 18:15:09.000000000","message":"error codes? specifically, let\u0027s reserve an error code for a driver validation failure, when we have it (probably just 400)","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    {"},{"line_number":249,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":250,"context_line":"    }"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Note that unlike with Resource Class, we are allowing the trait to be updated"},{"line_number":253,"context_line":"at any time. This is mostly because placement allows such updates and because"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_b989926f","line":250,"in_reply_to":"3f4b6375_1d0e19e5","updated":"2017-11-07 22:54:40.000000000","message":"I\u0027m not sure your statement makes sense, at least to me, because otherwise we\u0027re building another validation framework that will increase rest api calls across services. This just seems like pure data storage and  validation replies don\u0027t seem appropriate here.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    {"},{"line_number":249,"context_line":"        \"traits\": [\u0027CUSTOM_FOO\u0027, \u0027CUSTOM_BAR\u0027, \u0027CUSTOM_BAZ\u0027]"},{"line_number":250,"context_line":"    }"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"Note that unlike with Resource Class, we are allowing the trait to be updated"},{"line_number":253,"context_line":"at any time. This is mostly because placement allows such updates and because"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_1aa08291","line":250,"in_reply_to":"3f4b6375_b989926f","updated":"2017-11-08 21:07:44.000000000","message":"Its 400 if os-traits validation fails.\n\nThe driver validation is done in the deploy phase, as julia mentions.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"As mentioned above, the final change that is made is to ensure"},{"line_number":278,"context_line":"``instance_info/traits`` is a subset of the traits set on the Ironic node."},{"line_number":279,"context_line":"This should be part of the existing ``node.deploy.validate()`` call (or"},{"line_number":280,"context_line":"similar) such that the traits will be checked both before a deploy starts and"},{"line_number":281,"context_line":"on an explicit node validate call."},{"line_number":282,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_7932faa8","line":279,"range":{"start_line":279,"start_character":38,"end_line":279,"end_character":60},"updated":"2017-11-07 22:54:40.000000000","message":"I think we should refer to the internal call in ironic, which would be ?task.driver.deploy.validate() ? which is ultimately one of the items called from the general validate interface on the ironic rest api.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":294,"context_line":"You can list the traits on a node:"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":297,"context_line":"* openstack baremetal node show --fields uuid name traits \u003cnode-ident\u003e"},{"line_number":298,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"You can update the list of traits on a node:"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_7d462d52","line":297,"range":{"start_line":297,"start_character":58,"end_line":297,"end_character":70},"updated":"2017-10-30 18:15:09.000000000","message":"I cannot validate it right now, but I think this command has to look:\n\n openstack baremetal node show \u003cnode-ident\u003e --fields uuid name traits \n\notherwise OSC will treat \u003cnode-ident\u003e as part of --fields","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":294,"context_line":"You can list the traits on a node:"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":297,"context_line":"* openstack baremetal node show --fields uuid name traits \u003cnode-ident\u003e"},{"line_number":298,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"You can update the list of traits on a node:"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_f9c14a81","line":297,"range":{"start_line":297,"start_character":58,"end_line":297,"end_character":70},"in_reply_to":"3f4b6375_7d462d52","updated":"2017-11-07 22:54:40.000000000","message":"I\u0027m 95% sure Dmitry is correct about this.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f3313ce405b28358dd61a60a67442414fc023f6","unresolved":false,"context_lines":[{"line_number":294,"context_line":"You can list the traits on a node:"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":297,"context_line":"* openstack baremetal node show --fields uuid name traits \u003cnode-ident\u003e"},{"line_number":298,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"You can update the list of traits on a node:"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_7abad6c0","line":297,"range":{"start_line":297,"start_character":58,"end_line":297,"end_character":70},"in_reply_to":"3f4b6375_f9c14a81","updated":"2017-11-08 21:07:44.000000000","message":"Opps, yes, bad cut and paste fixing that up.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":303,"context_line":"* openstack baremetal node remove trait \u003cnode-ident\u003e CUSTOM_FOO CUSTOM_BAR"},{"line_number":304,"context_line":"* openstack baremetal node remove trait --all \u003cnode-ident\u003e"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"This is roughly copying the following command\u0027s syntax:"},{"line_number":307,"context_line":"https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/consistency-group.html#consistency-group-add-volume"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"It is common to use set and unset for key value pairs, but add and remove seems"},{"line_number":310,"context_line":"a better fit for in-place modifications of a list. It stops any ambiguity of"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_ddad21eb","line":307,"range":{"start_line":306,"start_character":0,"end_line":307,"end_character":128},"updated":"2017-10-30 18:15:09.000000000","message":"nit:\n\n This is roughly copying the \u003ccommand syntax of consistency groups \u003clink\u003e`_","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":397,"context_line":"Dependencies"},{"line_number":398,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"This work has the most value once the following Nova spec is implemented:"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/ironic-driver-traits.rst"},{"line_number":403,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_bd9385a3","line":400,"range":{"start_line":400,"start_character":0,"end_line":400,"end_character":73},"updated":"2017-10-30 18:15:09.000000000","message":"Let\u0027s make it clear that the nova spec depends on this one. This sentence can be read as if it was the opposite.","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"97a68a52eaa5bac76092ecdff7061707a8941fb1","unresolved":false,"context_lines":[{"line_number":397,"context_line":"Dependencies"},{"line_number":398,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"This work has the most value once the following Nova spec is implemented:"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/ironic-driver-traits.rst"},{"line_number":403,"context_line":""}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_194c2640","line":400,"range":{"start_line":400,"start_character":0,"end_line":400,"end_character":73},"in_reply_to":"3f4b6375_bd9385a3","updated":"2017-11-07 22:54:40.000000000","message":"+1","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"996f3d3e10fa54cfedea4db23b7dbe8d8f1d4b07","unresolved":false,"context_lines":[{"line_number":424,"context_line":"References"},{"line_number":425,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/ironic-driver-traits.rst"},{"line_number":428,"context_line":"* http://specs.openstack.org/openstack/nova-specs/specs/pike/approved/resource-provider-traits.html"},{"line_number":429,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/request-traits-in-nova.rst"},{"line_number":430,"context_line":"* http://git.openstack.org/cgit/openstack/nova-specs/tree/specs/queens/approved/add-trait-support-in-allocation-candidates.rst"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3f4b6375_3ded5525","line":430,"range":{"start_line":427,"start_character":0,"end_line":430,"end_character":126},"updated":"2017-10-30 18:15:09.000000000","message":"nit: let\u0027s link to rendered specs","commit_id":"f0392b4a2ce8a7299bc13180cb551479a946ea65"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c43fe2ecf20a0a77608ffa8bc4d1ab7529a7769c","unresolved":false,"context_lines":[{"line_number":66,"context_line":"easier because you don\u0027t need to modify any of the existing nodes and flavors"},{"line_number":67,"context_line":"when you add an additional pool of nodes."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Note that inspection rules can be used to set the initial value for a"},{"line_number":70,"context_line":"node\u0027s Resource Class. It is expected the initial set of traits for a node can"},{"line_number":71,"context_line":"be set in the same way."},{"line_number":72,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_426cd707","line":69,"updated":"2017-11-09 16:36:31.000000000","message":"dmitry prefers s/inspection/introspection/ but I think it is probably clear what this refers to.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c43fe2ecf20a0a77608ffa8bc4d1ab7529a7769c","unresolved":false,"context_lines":[{"line_number":81,"context_line":"additional flavor extra_specs to Ironic. Currently only extra_specs starting"},{"line_number":82,"context_line":"with `capabilities` are set in instance_info. After the above spec, the ironic"},{"line_number":83,"context_line":"virt driver will also include the flavor extra specs that define what traits"},{"line_number":84,"context_line":"have been requested, storing them `instance_info[\u0027traits\u0027]`."},{"line_number":85,"context_line":""},{"line_number":86,"context_line":".. note::"},{"line_number":87,"context_line":"    Note there is no change to how capabilities are used as part of this spec."}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_c247a7bc","line":84,"updated":"2017-11-09 16:36:31.000000000","message":"nit s/them/them in/","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"32cb20e67600ce191462b9faa6d82be5fdc7a52b","unresolved":false,"context_lines":[{"line_number":86,"context_line":".. note::"},{"line_number":87,"context_line":"    Note there is no change to how capabilities are used as part of this spec."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Ironic needs to validate that `instance_info[\u0027traits\u0027]` is consistent with the"},{"line_number":90,"context_line":"list of traits currently associated with Ironic node, i.e. we must check that"},{"line_number":91,"context_line":"`instance_info[\u0027traits\u0027]` only includes traits that are already in the list"},{"line_number":92,"context_line":"of traits set on the Ironic node. This is particularly useful in preventing"}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_2bcce168","line":89,"updated":"2017-11-13 14:35:20.000000000","message":"nit: please use double ticks `` for code snippets","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"32cb20e67600ce191462b9faa6d82be5fdc7a52b","unresolved":false,"context_lines":[{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"},{"line_number":95,"context_line":"driver. This validation is probably best integrated as part of"},{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":".. note::"},{"line_number":100,"context_line":"    The ironic virt driver in Nova will sync the list of traits reported by"}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_0bc7e547","line":97,"updated":"2017-11-13 14:35:20.000000000","message":"I understans the reasoning, but this is generic code that is absolutely not deploy-specific. The only solution I have is to return failure on trying to set instance_info[\u0027traits\u0027] to a bad value, but it won\u0027t catch the case when an operator changes the list of traits on a node.\n\nSigh, let\u0027s leave it here for now, I guess.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"625025ed9d8501e55e1cfc8a874a7bcf67c728d7","unresolved":false,"context_lines":[{"line_number":94,"context_line":"list and getting that list copied into the Placement API by the Ironic virt"},{"line_number":95,"context_line":"driver. This validation is probably best integrated as part of"},{"line_number":96,"context_line":"``driver.deploy.validate`` or similar, to ensure it is triggered by a call to"},{"line_number":97,"context_line":"deploy a node and a call to validate a node."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":".. note::"},{"line_number":100,"context_line":"    The ironic virt driver in Nova will sync the list of traits reported by"}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_ae845f5f","line":97,"in_reply_to":"1f485f77_0bc7e547","updated":"2017-11-13 14:59:30.000000000","message":"I think how/when we handle this validation (of instance_info[\u0027traits\u0027] being consistent with the node\u0027s traits), will become clearer when we code \u0026 see how the nova/deploy part hooks into this. Maybe we do a similar thing to how/when we check the instance_info stuff against the node\u0027s info.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c43fe2ecf20a0a77608ffa8bc4d1ab7529a7769c","unresolved":false,"context_lines":[{"line_number":179,"context_line":"Nova virt driver will be unable to add the traits in Placement. For similar"},{"line_number":180,"context_line":"reasons there is a limit of 50 traits on any node, to match the limit in"},{"line_number":181,"context_line":"Placement. A request to add a badly formatted trait should get a response with"},{"line_number":182,"context_line":"the status code 400."},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Note at no point does Ironic talk to the Placement API. The above validation"},{"line_number":185,"context_line":"depends only on access to the python library `os-traits`. As such, this"}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_82940fea","line":182,"updated":"2017-11-09 16:36:31.000000000","message":"or if \u003e 50 traits?","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c43fe2ecf20a0a77608ffa8bc4d1ab7529a7769c","unresolved":false,"context_lines":[{"line_number":186,"context_line":"validation poses little restriction on how traits can be used in standalone"},{"line_number":187,"context_line":"Ironic to assign arbitrary traits on particular Ironic nodes. Any non-standard"},{"line_number":188,"context_line":"traits simply need to have a prefix of ``CUSTOM_`` added. For more details on"},{"line_number":189,"context_line":"`os-traits` please see: https://docs.openstack.org/os-traits/latest"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"For convenience, it will be possible to get the full list of nodes and the"},{"line_number":192,"context_line":"traits associated with each node by extending the existing API in the following"}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_6266b354","line":189,"updated":"2017-11-09 16:36:31.000000000","message":"nit, missing period at end of sentence.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"32cb20e67600ce191462b9faa6d82be5fdc7a52b","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":304,"context_line":"* openstack baremetal node show \u003cnode-ident\u003e --fields uuid name traits"},{"line_number":305,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"You can update the list of traits on a node:"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_ebc5a93d","line":305,"updated":"2017-11-13 14:35:20.000000000","message":"this is vastly inconsistent with \"add trait\"/\"remove trait\". as we cannot do \"list trait\", I suggest just removing this.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":11297,"name":"Nisha Agarwal","email":"agarwalnisha1980@gmail.com","username":"Nisha"},"change_message_id":"6f09930e3aa1d519aca843f26c090053346c6d50","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":304,"context_line":"* openstack baremetal node show \u003cnode-ident\u003e --fields uuid name traits"},{"line_number":305,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"You can update the list of traits on a node:"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_c39e4462","line":305,"in_reply_to":"1f485f77_ae28df85","updated":"2017-11-14 04:02:48.000000000","message":"+1.  I think we should just remove it.","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"625025ed9d8501e55e1cfc8a874a7bcf67c728d7","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"* openstack baremetal node list --fields uuid name traits"},{"line_number":304,"context_line":"* openstack baremetal node show \u003cnode-ident\u003e --fields uuid name traits"},{"line_number":305,"context_line":"* openstack baremetal node trait list \u003cnode-ident\u003e"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"You can update the list of traits on a node:"},{"line_number":308,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"1f485f77_ae28df85","line":305,"in_reply_to":"1f485f77_ebc5a93d","updated":"2017-11-13 14:59:30.000000000","message":"Good point. Isn\u0027t this the same as \u0027openstack baremetal node show \u003cnode-ident\u003e --fields traits\u0027 ?","commit_id":"c912844dc5b27e89ee284bc6fe1c7ba074db1db7"}]}