Support for MAAS storage binding in bundles

[Ante Karamatić]

Now that bug 1674148 and bug 1677001 are fixed, I've tried using storage binding in the bundle:

series: xenial
machines:
  "0":
    constraints: tags=compute
    zone: az1
    series: xenial
services:
  ceph-osd:
    charm: cs:xenial/ceph-osd
    num_units: 1
    storage:
      osd-devices: sata,3
      osd-journals: ssd,1
    bindings:
      "": oam-space
      public: oam-space
      cluster: oam-space
    options:
      osd-reformat: 'yes'
      source: distro
    to:
    - '0'

But this results in error:

ERROR cannot deploy bundle: cannot add unit for application "ceph-osd": adding new machine to host unit "ceph-osd/0": cannot assign unit "ceph-osd/0" to machine 0: "maas" storage provider does not support dynamic storage (not supported)

Whole process:

ubuntu@maas:~$ juju create-storage-pool sata maas tags=rotary
ubuntu@maas:~$ juju create-storage-pool ssd maas tags=ssd
ubuntu@maas:~$ juju deploy storage.yaml
Deploying charm "cs:ceph-osd-241"
ERROR cannot deploy bundle: cannot add unit for application "ceph-osd": adding new machine to host unit "ceph-osd/0": cannot assign unit "ceph-osd/0" to machine 0: "maas" storage provider does not support dynamic storage (not supported)

[Andrew Wilkins (axwalk)]

The problem here is that the bundle deployment code splits an application deployment into "juju add-machine" and "juju deploy application --to <machine> --storage foo". Because MAAS storage is "static" (i.e. tied to the machine), this doesn't work.

Bundle deployments should not split apart the operations unless necessary (e.g. the bundle specifies machine placement).

[Dmitrii Shcherbakov (dmitriis)]

More info in a dup:

https://bugs.launchpad.net/juju/+bug/1713247

[Tim Penhey (thumper)]

After reading the source, it has become clear that this would probably work if we could avoid having to define the machine in the bundle.

Perhaps move the constraints to the application, and remove the machine block and the to directive on the application. Does that then work?

[Dmitrii Shcherbakov (dmitriis)]

Tim,

Without machine definitions it certainly does work - tested originally this way in a dup:

https://bugs.launchpad.net/juju/+bug/1713247

I can see that v3 vs v4 bundle spec differences are, in part, about having the "machines" section though:

https://github.com/juju/charmstore/blob/v5-unstable/docs/bundles.md#version-4-bundles
"Version 4 bundles

Version 4 bundles are identical to version 3 bundles except for a few key differences: the branch attribute of the application spec is no longer supported, they may contain a **machine specification**, and their **deployment directives are different** from version 3 bundles."

https://github.com/juju/charmstore/blob/v5-unstable/docs/bundles.md#deployment-directives
"Deployment directives

Version 4 deployment directives (the to attribute on the application spec) is a YAML list of items following the format:

(<containertype>:)?(<unit>|<machine>|new)"

https://github.com/juju/juju-bundlelib/blob/0.5.4/jujubundlelib/validation.py#L318-L325
is_legacy_bundle = machines is None

We currently use the "machines" section for our large bundles.

It may not be the most elegant way and we might need to think on how to improve large bundle experience in terms of using placement directives (e.g. using "--to 42" isn't the most descriptive directive) but it is a valid format.

[Dmitrii Shcherbakov (dmitriis)]

It's old code judging by `git blame` but:

https://github.com/juju/charm/blob/v5/migratebundle/migrate.go#L41

// Migrate parses the old-style bundles.yaml file in bundlesYAML
// and returns a map containing an entry for each bundle
// found in that basket, keyed by the name of the bundle.
//
// It performs the following changes:

...

// - when a "to" placement directive refers to machine 0,
// an ***explicit machines section*** is added. Also, convert
// it to a slice.
//

...

I think we just need to define what the -next bundle format is going to be. Then we could make it work uniformly for both cases.

I can see that the problem may be harder to solve than it looks:

https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/state/unit.go#L1547-L1564

https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/state/unit.go#L2190-L2197

MAAS (as of this message anyway) doesn't support dynamic node reconfiguration as opposed to public/private cloud scenarios where hot-plugging of storage is possible. So Juju seems to consider storage to be dynamic when the 'machines' section is used triggering: https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/provider/maas/volumes.go#L127-L131

as opposed to https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/provider/ec2/ebs.go#L217-L220

In terms of assignment this is:

AssignToCleanMachine
https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/state/unit.go#L2027-L2035

vs

AssignToMachine
https://github.com/juju/juju/blob/4119ff89e7971d3e1136ef89efffe698b9958cf7/state/unit.go#L1575-L1579

I think this is related to https://bugs.launchpad.net/juju/+bug/1567169 as well. Juju needs to be able to figure out that it should not allocate a clean machine right away if there are principal units that require storage to be allocated. Provider's ability to do dynamic storage should be assessed only when a machine is reused.

To make it generic it shouldn't be the first application's storage bindings but rather a summarized storage requirement if there are no conflicts between the two

app-one:
...
  storage:
    storage:
      osd-devices: sata,3
      osd-journals: ssd,1
  to:
  - 0
app-two:
...
  storage:
      backup-devices: scsi,3
  to:
  - 0