Open Stack

On 10/8/2018 11:05 AM, Carlos Goncalves wrote:
> The Octavia team merged a patch in master [1] that fixed an issue where 
> load balancers could be deleted whenever queue_event_streamer driver is 
> enabled and RabbitMQ goes down [2].
> 
> As this is a critical bug, we would like to backport as much back as 
> possible. The question is whether these backports comply with the stable 
> policy because it adds two new configuration options and deprecates one. 
> The patch was prepared so that the deprecated option has precedence if 
> set over the other two.
> 
> Reading the review guidelines [3], I only see "Incompatible config file 
> changes" as relevant, but the patch doesn't seem to go against that. We 
> had a patch that added a new config option backported to Queens that 
> raised some concern, so we'd like to be on the safe side this time ;-)
> 
> We'd appreciate guidance to whether such backports are acceptable or not.
> 

Well, a few things:

* I would have introduced the new config options as part of the bug fix 
but *not* deprecated the existing option in the same change but rather 
as a follow up. Then the new options, which do nothing by default (?), 
could be backported and the deprecation would remain on master.

* The release note mentions the new options as a feature, but that's not 
really correct is it? They are for fixing a bug, not new feature 
functionality as much.

In general, as long as the new options don't introduce new behavior by 
default for existing configuration (as you said, the existing option 
takes precedence if set), and don't require configuration then it should 
be OK to backport those new options. But the sticky parts here are (1) 
deprecating an option on stable (we shouldn't do that) and (2) the 
release note mentioning a feature.

What I'd probably do is (1) change the 'feature' release note to a 
'fixes' release note on master and then (2) backport the change but (a) 
drop the deprecation and (b) fix the release note in the backport to not 
call out a feature (since it's not a feature I don't think?) - and just 
make it clear with a note in the backport commit message why the 
backport is different from the original change.

-- 

Thanks,

Matt