Backport k8s install hook

testcharms/charm-repo/kubernetes/ubuntu/hooks/install

@@ -0,0 +1,4 @@
+#!/bin/sh
+date=`date`
+status-set waiting "Hello from install, it is $date."
+juju-log -l INFO "Hello from install."

testcharms/charm-repo/kubernetes/ubuntu/hooks/start

@@ -0,0 +1,3 @@
+#!/bin/bash
+application-version-set $(grep DISTRIB_RELEASE /etc/lsb-release | cut -d= -sf2)
+juju-log -l INFO "Hello from start."

testcharms/charm-repo/kubernetes/ubuntu/hooks/update-status

@@ -0,0 +1,4 @@
+#!/bin/sh
+date=`date`
+status-set active "Hello from update-status, it is $date."
+juju-log -l INFO "Hello from update-status."

testcharms/charm-repo/kubernetes/ubuntu/metadata.yaml

@@ -0,0 +1,12 @@
+name: ubuntu-k8s
+summary: "ubuntu operating system"
+description: "A popular operating system"
+provides:
+  ubuntu:
+    interface: ubuntu
+resources:
+  ubuntu_image:
+    type: "oci-image"
+    description: "Image used for ubuntu pod."
+series:
+  - kubernetes

worker/uniter/resolver.go

@@ -316,11 +316,3 @@ func (s *uniterResolver) nextOp(
 
 	return nil, resolver.ErrNoOperation
 }
-
-// NopResolver is a resolver that does nothing.
-type NopResolver struct{}
-
-// The NopResolver's NextOp operation should always return the no operation error.
-func (NopResolver) NextOp(resolver.LocalState, remotestate.Snapshot, operation.Factory) (operation.Operation, error) {
-	return nil, resolver.ErrNoOperation
-}

worker/uniter/storage/attachments_test.go

@@ -224,6 +224,20 @@ func (s *attachmentsSuite) TestAttachmentsUpdateShortCircuitDeath(c *gc.C) {
 }
 
 func (s *attachmentsSuite) TestAttachmentsStorage(c *gc.C) {
+	s.testAttachmentsStorage(c, operation.State{Kind: operation.Continue})
+}
+
+func (s *caasAttachmentsSuite) TestAttachmentsStorageStarted(c *gc.C) {
+	opState := operation.State{
+		Kind:      operation.RunHook,
+		Step:      operation.Queued,
+		Installed: true,
+		Started:   true,
+	}
+	s.testAttachmentsStorage(c, opState)
+}
+
+func (s *attachmentsSuite) testAttachmentsStorage(c *gc.C, opState operation.State) {
 	stateDir := c.MkDir()
 	unitTag := names.NewUnitTag("mysql/0")
 	abort := make(chan struct{})
@@ -244,9 +258,7 @@ func (s *attachmentsSuite) TestAttachmentsStorage(c *gc.C) {
 	assertStorageTags(c, att)
 
 	// Inform the resolver of an attachment.
-	localState := resolver.LocalState{State: operation.State{
-		Kind: operation.Continue,
-	}}
+	localState := resolver.LocalState{State: opState}
 	op, err := r.NextOp(localState, remotestate.Snapshot{
 		Life: params.Alive,
 		Storage: map[names.StorageTag]remotestate.StorageSnapshot{
@@ -270,6 +282,47 @@ func (s *attachmentsSuite) TestAttachmentsStorage(c *gc.C) {
 	c.Assert(ctx.Location(), gc.Equals, "/dev/sdb")
 }
 
+func (s *caasAttachmentsSuite) TestAttachmentsStorageNotStarted(c *gc.C) {
+	stateDir := c.MkDir()
+	unitTag := names.NewUnitTag("mysql/0")
+	abort := make(chan struct{})
+
+	st := &mockStorageAccessor{
+		unitStorageAttachments: func(u names.UnitTag) ([]params.StorageAttachmentId, error) {
+			return nil, nil
+		},
+	}
+
+	att, err := storage.NewAttachments(st, unitTag, stateDir, abort)
+	c.Assert(err, jc.ErrorIsNil)
+	r := storage.NewResolver(att, s.modelType)
+
+	storageTag := names.NewStorageTag("data/0")
+	_, err = att.Storage(storageTag)
+	c.Assert(err, jc.Satisfies, errors.IsNotFound)
+	assertStorageTags(c, att)
+
+	// Inform the resolver of an attachment.
+	localState := resolver.LocalState{State: operation.State{
+		Kind:      operation.RunHook,
+		Step:      operation.Queued,
+		Installed: true,
+		Started:   false,
+	}}
+	_, err = r.NextOp(localState, remotestate.Snapshot{
+		Life: life.Alive,
+		Storage: map[names.StorageTag]remotestate.StorageSnapshot{
+			storageTag: {
+				Kind:     params.StorageKindBlock,
+				Life:     life.Alive,
+				Location: "/dev/sdb",
+				Attached: true,
+			},
+		},
+	}, &mockOperations{})
+	c.Assert(err, gc.Equals, resolver.ErrNoOperation)
+}
+
 func (s *attachmentsSuite) TestAttachmentsCommitHook(c *gc.C) {
 	stateDir := c.MkDir()
 	unitTag := names.NewUnitTag("mysql/0")

worker/uniter/storage/resolver.go

@@ -47,11 +47,6 @@ func (s *storageResolver) NextOp(
 	opFactory operation.Factory,
 ) (operation.Operation, error) {
 
-	// Only IAAS models need to first run the install hook.
-	// For CAAS models, storage is specified in the pod config
-	// and mounted as the pod in started.
-	blockedWaitingForInstall := !localState.Installed && s.modelType == model.IAAS
-
 	if remoteState.Life == params.Dying {
 		// The unit is dying, so destroy all of its storage.
 		if !s.dying {
@@ -70,21 +65,51 @@ func (s *storageResolver) NextOp(
 		return nil, errors.Trace(err)
 	}
 
+	// The decision making below with regard to when to run the storage hooks for
+	// the first time after a charm is deployed is applicable only to IAAS models.
+	// For IAAS models, we do not run install hook before any storage provisioned
+	// with the deployment of the unit is available. The presumption is that IAAS
+	// charms may need storage on which to install their workloads.
+	//
+	// For CAAS models, it's different because we need to create the pod before
+	// any storage is provisioned. See in-line explanation below.
+	//
+	// TODO(juju3) - allow storage hooks to run after install for IAAS models
+	// This will make IAAS and CAAS behaviour the same, and charms should be
+	// resilient to resources such as storage not being ready at any given time.
 	var runStorageHooks bool
 	switch {
 	case localState.Kind == operation.Continue:
 		// There's nothing in progress.
 		runStorageHooks = true
-	case blockedWaitingForInstall && localState.Kind == operation.RunHook && localState.Step == operation.Queued:
-		// The install operation completed, and there's an install
-		// hook queued. Run storage-attached hooks first.
-		runStorageHooks = true
+	case localState.Kind == operation.RunHook && localState.Step == operation.Queued:
+		// There's a hook queued.
+		switch s.modelType {
+		case model.CAAS:
+			// For CAAS models, we wait until after the start hook has run before
+			// running storage-attached hooks since storage is provisioned only
+			// after the pod has been created.
+			//
+			// NB we must be careful here. The initial hook sequence is
+			// install->leader-elected->config-changed->started
+			// This chain is activated by each preceding hook queuing the next.
+			// If we allow a storage-attached hook to run before the start hook, we
+			// will potentially overwrite the state to initiate the next hook.
+			// So we need to get to at least started. This means that any charm logic
+			// that needs storage cannot be in install or start and if in config-changed,
+			// needs to be deferred until storage is available.
+			runStorageHooks = localState.Started
+		case model.IAAS:
+			// For IAAS models, we run storage-attached hooks before install.
+			runStorageHooks = !localState.Installed
+		}
 	}
 	if !runStorageHooks {
 		return nil, resolver.ErrNoOperation
 	}
 
-	if !localState.Installed && s.storage.Pending() == 0 {
+	// This message is only interesting for IAAS models.
+	if s.modelType == model.IAAS && !localState.Installed && s.storage.Pending() == 0 {
 		logger.Infof("initial storage attachments ready")
 	}
 
@@ -97,11 +122,13 @@ func (s *storageResolver) NextOp(
 	}
 	if s.storage.Pending() > 0 {
 		logger.Debugf("still pending %v", s.storage.pending.SortedValues())
-		if blockedWaitingForInstall {
-			// We only wait for pending storage before
-			// the install hook runs; we should not block
-			// other hooks from running while storage is
-			// being provisioned.
+		// For IAAS models, storage hooks are run before install.
+		// If the install hook has not yet run and there's still
+		// pending storage, we wait. We don't wait after the
+		// install hook has run as we should not block other
+		// hooks from running while new storage added post install
+		// is being provisioned.
+		if !localState.Installed && s.modelType == model.IAAS {
 			return nil, resolver.ErrWaiting
 		}
 	}

worker/uniter/uniter.go

@@ -618,24 +618,25 @@ func (u *Uniter) init(unitTag names.UnitTag) (err error) {
 		return errors.Trace(err)
 	}
 
-	var initialState operation.State
-	if u.modelType == model.IAAS {
-		initialState = operation.State{
-			Kind:     operation.Install,
-			Step:     operation.Queued,
-			CharmURL: charmURL,
-		}
-	} else {
+	initialState := operation.State{
+		Kind:     operation.Install,
+		Step:     operation.Queued,
+		CharmURL: charmURL,
+	}
+
+	if u.modelType == model.CAAS {
+		// For CAAS, run the install hook, but not the
+		// full install operation.
 		initialState = operation.State{
-			Hook:      &hook.Info{Kind: hooks.Start},
-			Kind:      operation.RunHook,
-			Step:      operation.Queued,
-			Installed: true,
+			Hook: &hook.Info{Kind: hooks.Install},
+			Kind: operation.RunHook,
+			Step: operation.Queued,
 		}
 		if err := u.unit.SetCharmURL(charmURL); err != nil {
 			return errors.Trace(err)
 		}
 	}
+
 	operationExecutor, err := u.newOperationExecutor(u.paths.State.OperationsFile, initialState, u.acquireExecutionLock)
 	if err != nil {
 		return errors.Trace(err)