Changes in flags need a update-status for later actions

[Rodrigo Barbieri]

When running actions such as remove-instance with force=true, the following flags are removed for the specified instance address:

cluster-instance-clustered-10-5-2-206: "True"
cluster-instance-configured-10-5-2-206: "True"

Then, if an action such as add-instance is run for same instance, the charm code does not recognize the updated flags, as can be seen in the logs:

2023-05-29 14:34:07 INFO unit.mysql/0.juju-log server.go:316 Configuring and adding instance to the cluster: 10.5.2.206.
2023-05-29 14:34:09 WARNING unit.mysql/0.juju-log server.go:316 User clusteruser exists.
2023-05-29 14:34:11 WARNING unit.mysql/0.juju-log server.go:316 User clusteruser exists.
2023-05-29 14:34:11 WARNING unit.mysql/0.juju-log server.go:316 Instance: 10.5.2.206, already configured.
2023-05-29 14:34:11 WARNING unit.mysql/0.juju-log server.go:316 Instance: 10.5.2.206, already clustered.

Even if the action is repeated multiple times, the result seen in the above logs is the same, despite the flags not existing anymore in leader-get.

Later, whenever an update-status hook runs (or is manually run using juju run -u mysql/leader -- ./hooks/update-status), the add-instance action recognizes the flags are not there anymore:

2023-05-29 14:34:55 INFO unit.mysql/0.juju-log server.go:316 Configuring and adding instance to the cluster: 10.5.2.206.
2023-05-29 14:34:57 WARNING unit.mysql/0.juju-log server.go:316 User clusteruser exists.
2023-05-29 14:34:59 WARNING unit.mysql/0.juju-log server.go:316 User clusteruser exists.
2023-05-29 14:35:00 INFO unit.mysql/0.juju-log server.go:316 Configuring instance for clustering: 10.5.2.206.

Therefore it is clear that the actions do not really set the flags as they are currently being read by the code using reactive.is_flag_set(). A hook such as update-status needs to run to set those flags.

This variation of behavior when update-status has or hasn't run is very misleading when performing those actions because the results end up being not what we expect them to be when we run the action.

I have tried hacking the code and inserting the method "update_dotted_flags" immediately at the end of remove-instance, but it still did not update the flags, as modifying the leader-settings is the same thing that both remove-instance and add-instance actions do. Apparently it is the fact that actions are not hook that do not get the flags saved, and a hook needs to run to read the updated leader-settings and apply them to flags.

[1] https://github.com/openstack/charm-mysql-innodb-cluster/blob/446eaabbfded2400245bbf630fb91418847f4c81/src/lib/charm/openstack/mysql_innodb_cluster.py#L2089

[Alex Kavanagh (ajkavanagh)]

mysql-innodb-cluster is a charms.reactive charm; thus there is a function that sets the flags at the end of the hook depending on whether the hook succeeds or not. This function needs to also be called at the end of an action if the action changes the flags. I've not dug into it, but I think that the charmhelpers hookenv_run_atexit() needs to be called at the end of the action handling code.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/charm-mysql-innodb-cluster/+/885039

[Rodrigo Barbieri (rodrigo-barbieri2010)]

Hey Alex. Could you please elaborate what you mean by adding the atexit call at the end of the action? Looking at the code, it appears every single action already has that. See [1]

[1] https://github.com/openstack/charm-mysql-innodb-cluster/blob/3150d34f196b87b121d9cb65608e2f47729f284b/src/actions/actions.py#L436

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/charm-mysql-innodb-cluster/+/885039
Committed: https://opendev.org/openstack/charm-mysql-innodb-cluster/commit/c4bcd2a23ada0617d3d253ec491350de8f3118ad
Submitter: "Zuul (22348)"
Branch: master

commit c4bcd2a23ada0617d3d253ec491350de8f3118ad
Author: Rodrigo Barbieri <email address hidden>
Date: Thu Jun 1 09:04:39 2023 -0300

    Fix side-effect from delayed flag update

    Actions such as remove-instance and add-instance
    cause leader-settings updates, which in turn result
    in flag updates that are later checked in actions
    and hooks. However, a hook is required to run after
    an action to apply the leader-settings updates into
    the flags, causing issues when actions are run
    without a hook in-between.

    This patch addresses that by changing the flag
    checks to leader-settings checks, so it is
    always using the most up-to-date information
    and preventing the issue.

    Closes-bug: #2021880
    Change-Id: Ic44fb0895016164f0a3c573c3247443b74e8faa8