Bug when configuring Keystone events format

[Rafael Weingartner]

By default Kolla-ansible defines 'enable_cadf_notifications' as 'no'. This variable is used to enable/disable CADF format in Keystone. The default of Keystone is 'CADF' already, but it (Keystone) does not set any messaging drivers; as a consequence, the default behavior of Keystone is not to send event messages to the queueing system. We were led to believe that using 'enable_cadf_notifications' with value 'no' would lead Kolla-ansible to configure the 'basic' message format. However, that is not what happens.

Kolla-ansible will configure Keystone without setting the oslo.messaging driver as messagingv2 when 'enable_cadf_notifications: no'. This will create a configuration that does not publish events in RabbitMQ. A PR was pull request (PR) was proposed to fix this misunderstanding in https://review.opendev.org/#/c/670626. That PR is introducing a few things:
* Moving the definition of 'enable_cadf_notifications' to Keystone role as it is only used there
* Changing the default value defined to 'yes' because that is the default behavior in Keystone. Keystone uses CADF format by Default.
* Add an else condition in Keystone.conf template. When CADF is not enabled, we need to explicitly set the format as 'basic'. Moreover, enabling the message driver to allow us to get messages in the queueing system.

After opening the PR, the fellow Radosław Piliszek questioned the proposed changes. More details can be found there (https://review.opendev.org/#/c/670626/2), at the PR's comments. In summary, it was questioned the use of a parameter in Kolla-ansible to enable/disable a feature in Keystone. It is argued that this is not the goal of Kolla-ansible. Right now, we have a few options with respect to this issue:
    option 1 -- we can use the PR as is;
    option 2 -- we can remove the "feature" (enable_cadf_notifications) in kolla-ansible to configure CADF notification format;
    option 3 -- do nothing (abandon this PR), and leave things as they are.

The community now has to decide on which path we will follow to address this situation. Afterwards, we can move on and propose a PR to apply/address the selected option into Kolla-ansible.

[Radosław Piliszek (yoctozepto)]

I strongly doubt keystone (as a project) is affected by this. ;-)

Option 2 is best for kolla because it aligns with letting users configure stuff like this using native config format via overrides. This option was not even documented. It needs to be deprecated/obsoleted and removed. If there is something for k-a to orchestrate there, which would be unwieldy for overrides, then we can discuss it but in this form it's not worth the trouble of changing behavior.

Please see the change proposal comments for details
TL;DR: the proposal changed the default behavior by enabling the notification messaging driver by default (which is actually not a default for keystone). K-a simply did not enable it by default (and notification type was always CADF if only enabled). We would need to slap an upgrade note that we are changing that behavior and (while it is sensible to run notifications) it would need some explanation anyway.

[Rafael Weingartner (rafaelweingartner)]

Radoslaw, I put Keystone because for the end-user (operators) that is what is going to look like at first sight. You use Kolla-ansible thinking that it will configure the basic messaging method, but it does not. Therefore, the first thing for us was to see what was going on there. Also, from what you told me, there is a lack of documentation with respect to this feature, that is why I put keystone as an affected project. We can remove it though.

I am confused, I am seeing the comments in the pull request, but I thought we were going to ignore the changes proposed there until we discuss the idea/plan we have with respect to this situation. Before moving on with a proposal to change Kolla-ansible (code-wise), I would like to understand and close on a goal (e.g. remote the feature from Kolla-ansible, change the feature as I initially proposed, and so on). Instead of having these discussions across different systems, this seems to be the right place for them, as you suggested there.

Do I need to take into consideration only your opinion? Or should we wait for other people from the community?

[Colleen Murphy (krinkle)]

Marking this as invalid for keystone since it seems to be a configuration issue with a deployment tool and not a bug in keystone itself. If you think keystone itself needs to be fixed, feel free to change the status back to "new".

[Rafael Weingartner (rafaelweingartner)]

This is being addressed here: https://review.opendev.org/#/c/670626/3

[Michal Nasiadka (mnasiadka)]

The change has been merged in master, is there a plan to back port it to Stein (since it's affected here)?

[Mark Goddard (mgoddard)]

I don't think it's necessary, there wasn't really a bug involved IIRC, just slightly confusing config for keystone notifications.

Is there a problem that needs fixing?

[Rafael Weingartner (rafaelweingartner)]

Not that I am aware of. Would you guys like me to backport it to Stein?

[Rafael Weingartner (rafaelweingartner)]

Well, for us, there was a problem. Because of some misconfigurations that
Kolla-ansible was creating, we ended up with 500k thousands of messages in
one of the RabbitMq topics. This almost crashed our PROD environment.

On Wed, Oct 9, 2019 at 9:11 AM Mark Goddard <email address hidden> wrote:

> I don't think it's necessary, there wasn't really a bug involved IIRC,
> just slightly confusing config for keystone notifications.
>
> Is there a problem that needs fixing?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1836650
>
> Title:
> Bug when configuring Keystone events format
>
> Status in OpenStack Identity (keystone):
> Invalid
> Status in kolla-ansible:
> Fix Committed
> Status in kolla-ansible stein series:
> New
> Status in kolla-ansible train series:
> Fix Committed
>
> Bug description:
> By default Kolla-ansible defines 'enable_cadf_notifications' as 'no'.
> This variable is used to enable/disable CADF format in Keystone. The
> default of Keystone is 'CADF' already, but it (Keystone) does not set
> any messaging drivers; as a consequence, the default behavior of
> Keystone is not to send event messages to the queueing system. We were
> led to believe that using 'enable_cadf_notifications' with value 'no'
> would lead Kolla-ansible to configure the 'basic' message format.
> However, that is not what happens.
>
> Kolla-ansible will configure Keystone without setting the oslo.messaging
> driver as messagingv2 when 'enable_cadf_notifications: no'. This will
> create a configuration that does not publish events in RabbitMQ. A PR was
> pull request (PR) was proposed to fix this misunderstanding in
> https://review.opendev.org/#/c/670626. That PR is introducing a few
> things:
> * Moving the definition of 'enable_cadf_notifications' to Keystone role
> as it is only used there
> * Changing the default value defined to 'yes' because that is the
> default behavior in Keystone. Keystone uses CADF format by Default.
> * Add an else condition in Keystone.conf template. When CADF is not
> enabled, we need to explicitly set the format as 'basic'. Moreover,
> enabling the message driver to allow us to get messages in the queueing
> system.
>
> After opening the PR, the fellow Radosław Piliszek questioned the
> proposed changes. More details can be found there (
> https://review.opendev.org/#/c/670626/2), at the PR's comments. In
> summary, it was questioned the use of a parameter in Kolla-ansible to
> enable/disable a feature in Keystone. It is argued that this is not the
> goal of Kolla-ansible. Right now, we have a few options with respect to
> this issue:
> option 1 -- we can use the PR as is;
> option 2 -- we can remove the "feature" (enable_cadf_notifications)
> in kolla-ansible to configure CADF notification format;
> option 3 -- do nothing (abandon this PR), and leave things as they
> are.
>
> The community now has to decide on which path we will follow to
> address this situation. Afterwards, we can move on and propose a PR to
> apply/address the selected option into Kolla-ansible.
>
> To manage notifications...