Code review comment for lp:~hazmat/pyjuju/formula-upgrades-spec

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Thanks for the changes Kapil. Looking great now.

+1, taking into account the following points:

[10]

Please merge trunk into the branch and move both documents into the drafts/
section, so that people don't think this is already working.

[11]

50 +The new hook-cli api would be called ``list-relations`` and would list
51 +all the relations a unit is a member of.

Having relation-list and list-relations will be confusing. The internal implementation
of relation-list even uses list_relations() methods.

Then, it's not clear why we need a method with these semantics. The formula necessarily
knows which relations it may be a member of, because it's a fixed set specified in the
formula metadata.

What we need is being able to list the units for other relations, so we might just have
relation-list itself accepting a [name] parameter:

    relation-list => Current behavior
    relation-list db => Same behavior, but for the db relation

[12]

+After the ``upgrade-formula`` hook is, new hooks of the formula will

s/is/is executed|run/

[13]

+The cli command is responsible

s/responsible/responsible for/, given the wording in the items.

[14]

70 + - Determining if a newer version of the formula exists in the
71 + origin repository. Strict linear numbering is used.

There are some details about the namespace here, but I guess that as long as
we check the same place the formula was originally obtained from, it should
be good for now.

[15]

75 + - Marking the unit state as needing an upgrade, with a pointer
76 + to the provider stored formula.

Interesting. I wonder how we should approach this part of the problem properly.

The formula right now is associated to the service, and we have no hints about
which formula was deployed in an individual unit. Also, ideally the mechanism
we implement can optionally upgrade a single unit at a time, out of a set of
multiple.

What about:

- Whenever a unit is deployed, copy the current formula reference from the service
  and into the unit metadata in zookeeper.

- When an upgrade is requested, change the formula reference in the service only.

- Then, for each individual unit which should be upgraded, just flag it as "upgrade=true"
  in its metadata.

- The unit then is responsible for evaluating the distinction between its formula and the
  one currently in its service and perform an action, if necessary.

What do you think?

Can you please integrate something along those lines which you agree with into the document?

[16]

86 + - Transition the unit and its relations to the stopped state (or equivalent).
87 +
88 + - Downloading the formula, and extracting the formula into the unit container.

As a small nit, it shouldn't stop the state before the formula is downloaded and ready to
be unpacked.

review: Approve

« Back to merge proposal