Skip to content

Commit

Permalink
Merge pull request #12437 from achilleasa/2.9-set-unit-error-state-wh…
Browse files Browse the repository at this point in the history
…en-relation-created-hooks-fail

#12437

This is a forward port of #12431.

## QA steps


```sh
# apply this patch
diff --git a/acceptancetests/repository/charms/ubuntu/metadata.yaml b/acceptancetests/repository/charms/ubuntu/metadata.yaml
index 1846048417..e5b7b19648 100644
--- a/acceptancetests/repository/charms/ubuntu/metadata.yaml
+++ b/acceptancetests/repository/charms/ubuntu/metadata.yaml
@@ -12,4 +12,6 @@ series:
 - bionic
 - eoan
 - focal
-
+peers:
+ peer:
+ interface: http


# Then create this file with the specified contents
$ cat acceptancetests/repository/charms/ubuntu/hooks/peer-relation-created
#!/bin/bash
set -e
# This should fail when executed by a non-leader unit!
relation-set -r $JUJU_RELATION_ID --app foo=bar

# Finally, run the following command and check that the non-leader unit reaches an error state
$ juju deploy -n2 ./acceptancetests/repository/charms/ubuntu

$ juju status
Model Controller Cloud/Region Version SLA Timestamp
default test lxd-with-cache/default 2.8.7.1 unsupported 18:33:58Z

App Version Status Scale Charm Store Rev OS Notes
ubuntu error 2 ubuntu local 1 ubuntu

Unit Workload Agent Machine Public address Ports Message
ubuntu/0* unknown idle 0 10.59.233.117
ubuntu/1 error idle 1 10.59.233.252 hook failed: "peer-relation-created" <---- look for this

Machine State DNS Inst id Series AZ Message
0 started 10.59.233.117 juju-be2af9-0 bionic Running
1 started 10.59.233.252 juju-be2af9-1 bionic Running
```

## Bug reference
https://bugs.launchpad.net/juju/+bug/1906706
  • Loading branch information
jujubot committed Dec 10, 2020
2 parents 0350a3a + d0cf9c4 commit 97fbb53
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
5 changes: 5 additions & 0 deletions worker/uniter/relation/resolver.go
Expand Up @@ -325,6 +325,11 @@ func (r *createdRelationsResolver) NextOp(
return nil, resolver.ErrNoOperation
}

// We should only evaluate the resolver logic if there is no other pending operation
if localState.Kind != operation.Continue {
return nil, resolver.ErrNoOperation
}

if err := r.stateTracker.SynchronizeScopes(remoteState); err != nil {
return nil, errors.Trace(err)
}
Expand Down
23 changes: 23 additions & 0 deletions worker/uniter/relation/resolver_test.go
Expand Up @@ -1295,6 +1295,29 @@ func (s *relationCreatedResolverSuite) TestCreatedRelationResolverFordRelationNo
})
}

// This is a regression test for LP1906706
func (s *relationCreatedResolverSuite) TestCreatedRelationsResolverWithPendingHook(c *gc.C) {
ctrl := gomock.NewController(c)
defer ctrl.Finish()

r := mocks.NewMockRelationStateTracker(ctrl)

localState := resolver.LocalState{
State: operation.State{
Installed: true,
Kind: operation.RunHook,
Step: operation.Pending,
},
}
remoteState := remotestate.Snapshot{
Life: life.Alive,
}

createdRelationsResolver := relation.NewCreatedRelationResolver(r)
_, err := createdRelationsResolver.NextOp(localState, remoteState, &mockOperations{})
c.Assert(errors.Cause(err), gc.Equals, resolver.ErrNoOperation, gc.Commentf("expected to get ErrNoOperation when a RunHook operation is pending"))
}

type mockRelationResolverSuite struct {
mockRelStTracker *mocks.MockRelationStateTracker
mockSupDestroyer *mocks.MockSubordinateDestroyer
Expand Down

0 comments on commit 97fbb53

Please sign in to comment.