/juju

Implement "juju deploy --attach-storage" #7355

Merged
merged 5 commits into from May 18, 2017

Conversation

Reviewers

@wallyworld wallyworld

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@axw @jujubot @wallyworld
@axw
Member

axw commented May 18, 2017

Description of change

This branch adds the --attach-storage flag to the "juju deploy" command, along with the corresponding API and API server changes. The state changes were made in a previous branch. With this change, it is possible to attach existing storage to the initial unit of an application.

There are a few fixes bundled in here as separate commits:

  • revert the changes to handling Dead volume/filesystem attachments. Machine storage attachments go from Alive to Dying, and Dying to removed. There is no Dead state.
  • modify the preconditions for block device updates so that they can still occur for Dying, but not Dead, machines. A Dying machine may still require block device updates to clean up storage (e.g. to detach a volume-backed filesystem).
  • add short-circuit removal of volume-backed filesystems. Once a volume-backed filesystem is detached, it has no responsible storage provisioner. Thus, destroying a detached volume-backed filesystem should cause it to be removed immediately; the contents will be destroyed along with the volume.

QA steps

  1. juju bootstrap (tested with aws and google; google assumed below)
  2. juju deploy postgresql --storage pgdata=gce,10G
  3. juju ssh postgresql/0
    $ sudo -u postgres psql

CREATE TABLE FOO(bar VARCHAR);
INSERT INTO FOO VALUES('baz');

  1. juju remove-application postgresql
  2. juju deploy postgresql pg2 --attach-storage pgdata/0 --to zone=<same availability zone as postgresql/0 was deployed to>
  3. juju ssh pg2/0
    $ sudo -u postgres psql

SELECT * FROM FOO;
(check for previously inserted row)

  1. juju destroy-controller -y

NOTE: it is a current limitation that the AZ must be explicitly specified. This limitation will be addressed before 2.3 is released.

Documentation changes

Yes, we will need to add documentation of the new --attach-storage flag. It accepts a list of detached storage IDs, which will be attached to the initial unit of the application. This flag may not be used with "-n".

Bug reference

None.
@axw
Add --attach-storage to juju deploy 
Updates to api, apiserver, and cmd/juju
to add the --attach-storage flag to the
"juju deploy" command. This flag takes
a set of storage IDs which will be attached
to the initial unit of the deployed
application. This flag may not be used
with "-n".

The juju.DeployApplication function has
been moved out of the juju sub-package,
and into apiserver/application. This is
the only place it is used, and the logic
is minimal enough that it does not deserve
a separate package.
This commit was signed with a verified signature.
@axw axw Andrew Wilkins
GPG key ID: 55314F7628952B05 Learn about signing commits
6406116
@wallyworld

wallyworld approved these changes May 18, 2017
View changes

Nice

api/application/client.go
@@ -102,6 +107,16 @@ type DeployArgs struct {
// it. Placement directives, if provided, specify the machine on which the charm
// is deployed.
func (c *Client) Deploy(args DeployArgs) error {
+ if len(args.AttachStorage) > 0 && args.NumUnits != 1 {
+ return errors.Errorf("AttachStorage is non-empty, but NumUnits is %d", args.NumUnits)
@wallyworld

wallyworld May 18, 2017

Owner

I think this could be more user friendly
"Cannot specify storage attachments when more than one unit is requested"
or something like that

@axw

axw May 18, 2017

Member

Done. The error message is intended for the programmer that writes to the API, but I guess we should assume that they'll just send the error up to the user.

cmd/jujud/agent/machine_test.go
c.Assert(err, jc.ErrorIsNil)
// It should be allocated to a machine, which should then be provisioned.
- c.Logf("service %q added with 1 unit, waiting for unit %q's machine to be started...", svc.Name(), units[0].Name())
+ c.Logf("service %q added with 1 unit, waiting for unit %q's machine to be started...", svc.Name(), unit.Name())
@wallyworld

wallyworld May 18, 2017

Owner

s/service/application

@axw

axw May 18, 2017

Member

fixed

axw added some commits May 17, 2017

@axw
Revert "worker/storageprovisioner: handle Dead attachments" 
This reverts commit d828986.
This commit was signed with a verified signature.
@axw axw Andrew Wilkins
GPG key ID: 55314F7628952B05 Learn about signing commits
e707823
@axw
state: allow block device updates while machine is Dying
This commit was signed with a verified signature.
@axw axw Andrew Wilkins
GPG key ID: 55314F7628952B05 Learn about signing commits
9755d75
@axw
state: volume-backed fs removed when destroyed
This commit was signed with a verified signature.
@axw axw Andrew Wilkins
GPG key ID: 55314F7628952B05 Learn about signing commits
e8cb963
@axw
WIP
This commit was signed with a verified signature.
@axw axw Andrew Wilkins
GPG key ID: 55314F7628952B05 Learn about signing commits
7f41736
@axw
Member

axw commented May 18, 2017

$$merge$$
@jujubot
Contributor

jujubot commented May 18, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 6a6874a into juju:feature-persistent-storage May 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment