Bug 1598197

Why on earth wouldn't we be able to update bundle resources?

There are two approaches to the "update" functionality:
A. The user tells pcs which part of a resource settings they want to change. Pcs changes that bit and does not touch anything else.
B. The user tells pcs they want a resource to look exactly like specified. Pcs drops all current setting of the resource and applies the specified settings.

All update commands in pcs work as described in A. "pcs resource update my_resource attr1=value1" changes my_resource's instance attribute attr1's value to value1. It does not change or remove any existing instance attributes not named attr1. Setting cluster properties, utilization, quorum settings, you name it, all updates work the same way. The reason for this is to keep the commands short and easy to use.

Generally, the syntax is "attr1=value1" to set attr1's value to value1 and "attr2=" to delete attr2. With bundles, the situation is a bit complicated. Managing individual attributes is not enough, it is needed to manage groups of them called storage-map and port-map. So we came up with "storage-map add attr1=value1 attr2=value2..." and "storage-map remove storage_map_id". "storage-map update storage_map_id attr1=value1 attr2=value2" could be added to replace add-remove combo to change an existing storage-map. This does not seem to be requested here, though.

What I think is being requested here is the way described as B. This requires all the settings of a resource (all the cluster properties, utilization attributes, quorum settings) to be specified in a command as not specified settings get removed. This is difficult to use by people because for changing one little setting all the existing settings have to be repeated in a command just to keep them the same they already were (and prevent them to be deleted). On the other hand, it fits Michele's use case better.

The best way to implement this is to introduce new pcs command(s) to prevent confusion with existing update command(s) as their semantics (A vs. B) is very different.

Related question:
Michele, when you update primitive resources, how do you delete attributes? Syntax for that is "pcs resource update attr_to_delete=", so you must know which attributes to delete and specify them with no values in the update command.

re [comment 5]:

> With bundles, the situation is a bit complicated. Managing individual
> attributes is not enough, it is needed to manage groups of them called
> storage-map and port-map.

To be honest, it's not that different assuming that a special semantic
rule is established:

> id of a keyword-denoted attribute set (here: {port,storage}-map)
> is always immutable regarding the effect on the configuration,
> meaning that its only direct footprint there is with the pristine
> addition, otherwise it is used just to access the right one with
> updates (here: triggered with pcs resource bundle update)
> -- it therefore needs to be specified unless the ambiguity is
> resolved by other means (here: "{port,storage}-map add" achieves
> that implicitly with selection of yet-unused id automatically)

Also With additional normalization rule:

> do not leave empty attribute sets around
> (would be a flaw in pacemaker's schemas if any such
> dropping was prevented)

in place,

  storage-map remove storage_map_id

would become equivalent with

  storage-map id=storage_map_id [nothing but a start of a new
                                 syntactical group if at all]

meaning that "{port,storage}-map add/remove" would become completely
avoidable (could be emulated by the proposed logic).


I think this would:

- satisfy Michele's use case (have very exact idea of what needs
  to be in the resulting configuration AND do not want to bother
  how to get there piecewise AND do not want to get there in two
  invocations, i.e., drop and define anew -- why wasn't this
  mentioned so far?)

- would hopefully not clash with existing syntactical uses (anyway, is
  changing identifiers a sane thing to do? I'd argue it's not and the
  rule of immutability should have been firmly established in early
  days)

- would not incur users' frustration arising from having to edit
  the previous new-bundle-establishing command twice+ in possibly
  a very long, screen-wrapped command(!) in case that was the
  easiest way to post-fix something about the bundle defined

1 Kč

(In reply to Tomas Jelinek from comment #5)
> There are two approaches to the "update" functionality:
> A. The user tells pcs which part of a resource settings they want to change.
> Pcs changes that bit and does not touch anything else.
> B. The user tells pcs they want a resource to look exactly like specified.
> Pcs drops all current setting of the resource and applies the specified
> settings.
> 
> All update commands in pcs work as described in A. "pcs resource update
> my_resource attr1=value1" changes my_resource's instance attribute attr1's
> value to value1. It does not change or remove any existing instance
> attributes not named attr1. Setting cluster properties, utilization, quorum
> settings, you name it, all updates work the same way. The reason for this is
> to keep the commands short and easy to use.
> 
> Generally, the syntax is "attr1=value1" to set attr1's value to value1 and
> "attr2=" to delete attr2. With bundles, the situation is a bit complicated.
> Managing individual attributes is not enough, it is needed to manage groups
> of them called storage-map and port-map. So we came up with "storage-map add
> attr1=value1 attr2=value2..." and "storage-map remove storage_map_id".
> "storage-map update storage_map_id attr1=value1 attr2=value2" could be added
> to replace add-remove combo to change an existing storage-map. This does not
> seem to be requested here, though.
> 
> What I think is being requested here is the way described as B. This
> requires all the settings of a resource (all the cluster properties,
> utilization attributes, quorum settings) to be specified in a command as not
> specified settings get removed. This is difficult to use by people because
> for changing one little setting all the existing settings have to be
> repeated in a command just to keep them the same they already were (and
> prevent them to be deleted). On the other hand, it fits Michele's use case
> better.
> 
> The best way to implement this is to introduce new pcs command(s) to prevent
> confusion with existing update command(s) as their semantics (A vs. B) is
> very different.
> 
> Related question:
> Michele, when you update primitive resources, how do you delete attributes?
> Syntax for that is "pcs resource update attr_to_delete=", so you must know
> which attributes to delete and specify them with no values in the update
> command.

Well we cannot use the update command just yet due to this very BZ.

Right now the only workaround we managed to get to, is to dump the cluster cib, make a copy,
remove the resource from the copy and recreate it against the copy with potentially a different
set of storage-maps for the bundle. Then we crm_diff --cib -o cib.orig -n cib.new and
parse the diff. We only push back the cib.new file to the live CIB if we detect any changes
that are relevant to the resource we are considering for update.

We're increasing the priority of this because this will hit us harder than we initially thought.
Namely, in a composable HA scenario, we will briefly repush the CIB of puppet without the location constraints
because pcs resource remove foo-bundle will remove those too (to keep the XML schema validated)
and to work around this.

A new command is totally fine for us.

Created attachment 1522184 [details]
proposed fix

Basic test:
[kid76 ~] $ pcs cluster cib|grep 'id="B1"' -A2
      <bundle id="B1">
        <docker image="pcs:test"/>
      </bundle>
[kid76 ~] $ pcs resource bundle reset B1 container docker image=pcs:new
[kid76 ~] $ pcs cluster cib|grep 'id="B1"' -A2
      <bundle id="B1">
        <docker image="pcs:new"/>
      </bundle>

After Fix:

[kid76 ~] $ rpm -q pcs
pcs-0.9.167-1.el7.x86_64

[kid76 ~] $ pcs cluster cib|grep 'id="B1"' -A2
      <bundle id="B1">
        <docker image="pcs:test"/>
      </bundle>
[kid76 ~] $ pcs resource bundle reset B1 container docker image=pcs:new
[kid76 ~] $ pcs cluster cib|grep 'id="B1"' -A2
      <bundle id="B1">
        <docker image="pcs:new"/>
      </bundle>

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2019:2244