<div dir="ltr"><div>I agree with a lot of your desires, but not your reasoning as to why the changes are problematic. Also, your statements about API changes are pretty reasonable and probably want writing down somewhere so that future specs are evaluated against them. <br><br></div>Firstly, if we add to the core Neutron interface it's clear that old plugins have to behave sensibly. If we add an MTU attribute or a VLAN attribute and we allow the user to tinker with it (in the case of MTU that's a future plan, but in the VLAN case that's true now) then there has to be a backward compatibility plan - we can't have plugins appearing as if they support the functionality when they actually ignored the request. The specs are quite explicit about that and how to make sure it doesn't work that way, so your criticism about not correctly implementing the opt-in approach does not apply. They're not aiming for an opt-in approach but an opt-out one; they're aiming to add an attribute and ensure that unaware, unchanged plugins respect the proposed API behaviour.<br><div><br>That being the case, neither the MTU plugin not the VLAN plugin require support from the driver or plugin to work. If the current patches do not implement the model, and in particular if they work as you describe, then they are indeed broken (not to spec, specifically) and need to be fixed. The specs are quite clear about what should happen in those cases, and what you're describing is a bug in the code, not a flaw in the proposed design - I'll get someone to take a look, but can you file it so that it doesn't get lost, along with a way to trigger it?<br><br></div><div>I think that, in these circumstances, the intended behaviour is logical and in line with what you want to see. If you are still interested in either or both of them being extensions rather than core attributes, then that's fine and something we should discuss further, but I don't think what you've said here is justification.<br>-- <br></div><div>Ian.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 19 March 2015 at 16:23, Armando M. <span dir="ltr"><<a href="mailto:armamig@gmail.com" target="_blank">armamig@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Forwarding my reply to the other thread here:<div><br></div><div>~~~~<br><div><br></div><div><span style="font-size:12.8000001907349px">If my memory does not fail me, changes to the API (new resources, new resource attributes or new operations allowed to resources) have always been done according to these criteria:</span><div style="font-size:12.8000001907349px"><ul><li style="margin-left:15px">an opt-in approach: this means we know the expected behavior of the plugin as someone has coded the plugin in such a way that the API change is supported;<br></li><li style="margin-left:15px">an opt-out approach: if the API change does not require explicit backend support, and hence can be deemed supported by all plugins.</li><li style="margin-left:15px">a 'core' extension (ones available in neutron/extensions) should be implemented at least by the reference implementation;</li></ul><div>Now, there might have been examples in the past where criteria were not met, but these should be seen as exceptions rather than the rule, and as such, fixed as defects so that an attribute/resource/operation that is accidentally exposed to a plugin will either be honored as expected or an appropriate failure is propagated to the user. Bottom line, the server must avoid to fail silently, because failing silently is bad for the user.</div><div><br></div><div>Now both features [1] and [2] violated the opt-in criterion above: they introduced resources attributes in the core models, forcing an undetermined behavior on plugins. </div><div><br></div><div>I think that keeping [3,4] as is can lead to a poor user experience; IMO it's unacceptable to let a user specify the attribute, and see that ultimately the plugin does not support it. I'd be fine if this was an accident, but doing this by design is a bit evil. So, I'd suggest the following, in order to keep the features in Kilo:</div><div><ul><li style="margin-left:15px">Patches [3, 4] did introduce config flags to control the plugin behavior, but it looks like they were not applied correctly; for instance, the vlan_transparent case was only applied to ML2. Similarly the MTU config flag was not processed server side to ensure that plugins that do not support advertisement do not fail silently. This needs to be rectified.</li><li style="margin-left:15px">As for VLAN transparency, we'd need to implement work item 5 (of 6) of spec [2], as this extension without at least a backend able to let tagged traffic pass doesn't seem right.</li><li style="margin-left:15px">Ensure we sort out the API tests so that we know how the features behave.</li></ul></div><div>Now granted that controlling the API via config flags is not the best solution, as this was always handled through the extension mechanism, but since we've been talking about moving away from extension attributes with [5], it does sound like a reasonable stop-gap solution.</div></div><div style="font-size:12.8000001907349px"><br></div><div style="font-size:12.8000001907349px">Thoughts?</div><div style="font-size:12.8000001907349px">Armando</div><div style="font-size:12.8000001907349px"><br></div><div style="font-size:12.8000001907349px">[1] <a href="http://specs.openstack.org/openstack/neutron-specs/specs/kilo/mtu-selection-and-advertisement.html" target="_blank">http://specs.openstack.org/openstack/neutron-specs/specs/kilo/mtu-selection-and-advertisement.html</a></div><div style="font-size:12.8000001907349px">[2] <a href="http://specs.openstack.org/openstack/neutron-specs/specs/kilo/nfv-vlan-trunks.html" target="_blank">http://specs.openstack.org/openstack/neutron-specs/specs/kilo/nfv-vlan-trunks.html</a></div><div style="font-size:12.8000001907349px">[3] <a href="https://review.openstack.org/#/q/project:openstack/neutron+branch:master+topic:bp/mtu-selection-and-advertisement,n,z" target="_blank">https://review.openstack.org/#/q/project:openstack/neutron+branch:master+topic:bp/mtu-selection-and-advertisement,n,z</a></div><div style="font-size:12.8000001907349px">[4] <a href="https://review.openstack.org/#/q/project:openstack/neutron+branch:master+topic:bp/nfv-vlan-trunks,n,z" target="_blank">https://review.openstack.org/#/q/project:openstack/neutron+branch:master+topic:bp/nfv-vlan-trunks,n,z</a></div><div style="font-size:12.8000001907349px">[5] <a href="https://review.openstack.org/#/c/136760/" target="_blank">https://review.openstack.org/#/c/136760/</a></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On 19 March 2015 at 14:56, Ian Wells <span dir="ltr"><<a href="mailto:ijw.ubuntu@cack.org.uk" target="_blank">ijw.ubuntu@cack.org.uk</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr"><span>On 19 March 2015 at 11:44, Gary Kotton <span dir="ltr"><<a href="mailto:gkotton@vmware.com" target="_blank">gkotton@vmware.com</a>></span> wrote:<br></span><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
Just the fact that we did this does not make it right. But I guess that we<br>
are starting to bend the rules. I think that we really need to be far more<br>
diligent about this kind of stuff. Having said that we decided the<br>
following on IRC:<br>
1. Mtu will be left in the core (all plugins should be aware of this and<br>
treat it if necessary)<br>
2. Vlan-transparency will be moved to an extension. Pritesh is working on<br>
this.<br></blockquote><div><br></div></span><div>The spec started out as an extension, and in its public review people requested that it not be an extension and that it should instead be core. I accept that we can change our minds, but I believe there should be a good reason for doing so. You haven't given that reason here and you haven't even said who the 'we' is that decided this. Also, as the spec author, I had a conversation with you all but there was no decision at the end of it (I presume that came afterward) and I feel that I have a reasonable right to be involved. Could you at least summarise your reasoning here?<br><br>I admit that I prefer this to be in core, but I'm not terribly choosy and that's not why I'm asking. I'm more concerned that this is changing our mind at literally the last moment, and in turn wasting a developer's time, when there was a perfectly good process to debate this before coding was begun, and again when the code was up for review, both of which apparently failed. I'd like to understand how we avoid getting here again in the future. I'd also like to be certain we are not simply reversing a choice on a whim.<span><font color="#888888"><br>-- <br></font></span></div><span><font color="#888888"><div>Ian.<br></div></font></span></div></div></div>
<br></div></div><span class="">__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></span></blockquote></div><br></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>