)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"958afa25ae967c8f31ff382442b8df64cce150d2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5f7dd934_f260f206","updated":"2022-01-19 07:32:13.000000000","message":"code looks good and it is a simple change. However, I have a question, why not changing the documentation about what pre-population means, and maintain the current way, i.e., only perform the pre-population upon the first pod creation?\n\nTo me, pre-population means we create X ports in the nodes to be more ready. This does not need to happen on namespace creation, and can happen on the first pod creation. In fact, due to the amount of ports created (X ports in all the trunks), perhaps it makes even more needed the current behavior, to ensure we save ports/flows for namespaces that won\u0027t needed. Either that or we should do the pre-population per namespace (only tag the namespace we want to pre-populate or something like that)\n\nAdding -1 just to start the discussion","commit_id":"02579be8dc33b31d127c3fbe5bb5d20f538a756e"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"fab930d4b86a735902b6bbf519576c3d4134d36b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"669776fc_ab066dc4","in_reply_to":"40ed3b7d_acf0e8f4","updated":"2022-01-19 20:27:59.000000000","message":"Indeed, that is a good point. I think the solution of adding to the documentation explaining the current approach might be better.\n\nHere are my takes from the testing:\nI enabled the population handler, created a Namespace, no KuryrNetwork CR was created as expected, until a Pod got created in the Namespace. Pre-population happened to all hosts except the one the Pod was scheduled (a normal population had already happened to this one). However, I think it\u0027s possible that duplicated populations(prepopulation and normal population) happens since we\u0027re not locking the force_population[1], it doesn\u0027t sound like a big problem though. I\u0027m moving the bz to non blocker[3], let me know if you think otherwise.\n\nJust another thought not on the scope of this patch...\nShould we lock pool population for operations like _trigger_return_to_pool[2]?\n\n[1] https://github.com/openshift/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/drivers/vif_pool.py#L1120-L1134.\n[2] https://github.com/openshift/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/drivers/vif_pool.py#L986-L988\n[3] https://bugzilla.redhat.com/show_bug.cgi?id\u003d2041940","commit_id":"02579be8dc33b31d127c3fbe5bb5d20f538a756e"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"77b53084568c909e36c94ad316703a040f19913e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"40ed3b7d_acf0e8f4","in_reply_to":"5f7dd934_f260f206","updated":"2022-01-19 09:28:21.000000000","message":"Hm… This is fair point, prepopulation should still \"do something\". And it would make it aligned with our decision to delay subnet creation to when a pod gets created (as some namespaces doesn\u0027t have pods at all).","commit_id":"02579be8dc33b31d127c3fbe5bb5d20f538a756e"}]}