Skip to content

Commit 9a34f2a

Browse files
author
Menno Smits
committed
state: Remove charm docs once unused
Instead of leaving dead charm documents around they are now removed once no longer required.
1 parent 5b60f95 commit 9a34f2a

File tree

5 files changed

+52
-46
lines changed

5 files changed

+52
-46
lines changed

‎state/allwatcher_internal_test.go‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/juju/errors"
1212
"github.com/juju/loggo"
1313
jc "github.com/juju/testing/checkers"
14-
"github.com/juju/utils"
1514
gc "gopkg.in/check.v1"
1615
"gopkg.in/juju/charm.v6-unstable"
1716
"gopkg.in/juju/names.v2"
@@ -22,7 +21,6 @@ import (
2221
"github.com/juju/juju/state/multiwatcher"
2322
"github.com/juju/juju/state/watcher"
2423
"github.com/juju/juju/status"
25-
"github.com/juju/juju/storage"
2624
"github.com/juju/juju/testing"
2725
)
2826

@@ -48,22 +46,6 @@ options:
4846

4947
type allWatcherBaseSuite struct {
5048
internalStateSuite
51-
modelCount int
52-
}
53-
54-
func (s *allWatcherBaseSuite) newState(c *gc.C) *State {
55-
s.modelCount++
56-
cfg := testing.CustomModelConfig(c, testing.Attrs{
57-
"name": fmt.Sprintf("testenv%d", s.modelCount),
58-
"uuid": utils.MustNewUUID().String(),
59-
})
60-
_, st, err := s.state.NewModel(ModelArgs{
61-
CloudName: "dummy", CloudRegion: "dummy-region", Config: cfg, Owner: s.owner,
62-
StorageProviderRegistry: storage.StaticProviderRegistry{},
63-
})
64-
c.Assert(err, jc.ErrorIsNil)
65-
s.AddCleanup(func(*gc.C) { st.Close() })
66-
return st
6749
}
6850

6951
// setUpScenario adds some entities to the state so that

‎state/charm.go‎

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -381,11 +381,8 @@ func (c *Charm) Destroy() error {
381381
// inaccessible to future clients. It will fail unless the charm is
382382
// already Dying (indicating that someone has called Destroy).
383383
func (c *Charm) Remove() error {
384-
switch c.doc.Life {
385-
case Alive:
384+
if c.doc.Life == Alive {
386385
return errors.New("still alive")
387-
case Dead:
388-
return nil
389386
}
390387

391388
stor := storage.NewStorage(c.st.ModelUUID(), c.st.MongoSession())
@@ -397,18 +394,15 @@ func (c *Charm) Remove() error {
397394
return errors.Annotate(err, "deleting archive")
398395
}
399396

400-
buildTxn := func(_ int) ([]txn.Op, error) {
401-
ops, err := charmRemoveOps(c.st, c.doc.URL)
402-
switch errors.Cause(err) {
403-
case nil:
404-
case errAlreadyDead:
405-
return nil, jujutxn.ErrNoOperations
406-
default:
407-
return nil, errors.Trace(err)
408-
}
409-
return ops, nil
410-
}
411-
if err := c.st.run(buildTxn); err != nil {
397+
// We know the charm is already dying, dead or removed at this
398+
// point (life can *never* go backwards) so an unasserted remove
399+
// is safe.
400+
removeOps := []txn.Op{{
401+
C: charmsC,
402+
Id: c.doc.URL.String(),
403+
Remove: true,
404+
}}
405+
if err := c.st.runTransaction(removeOps); err != nil {
412406
return errors.Trace(err)
413407
}
414408
c.doc.Life = Dead
@@ -528,12 +522,12 @@ func (st *State) AddCharm(info CharmInfo) (stch *Charm, err error) {
528522
return nil, errors.Trace(err)
529523
}
530524

531-
query := charms.FindId(info.ID.String()).Select(bson.M{"placeholder": 1})
532-
525+
query := charms.FindId(info.ID.String()).Select(bson.M{
526+
"placeholder": 1,
527+
"pendingupload": 1,
528+
})
533529
buildTxn := func(attempt int) ([]txn.Op, error) {
534-
var doc struct {
535-
Placeholder bool `bson:"placeholder"`
536-
}
530+
var doc charmDoc
537531
if err := query.One(&doc); err == mgo.ErrNotFound {
538532
if info.ID.Schema == "local" {
539533
curl, err := st.PrepareLocalCharmUpload(info.ID)
@@ -546,6 +540,8 @@ func (st *State) AddCharm(info CharmInfo) (stch *Charm, err error) {
546540
return insertCharmOps(st, info)
547541
} else if err != nil {
548542
return nil, errors.Trace(err)
543+
} else if doc.PendingUpload {
544+
return updateCharmOps(st, info, stillPending)
549545
} else if doc.Placeholder {
550546
return updateCharmOps(st, info, stillPlaceholder)
551547
}

‎state/charm_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ func (s *CharmSuite) checkRemoved(c *gc.C) {
5252
_, err := s.State.Charm(s.curl)
5353
c.Check(err, gc.ErrorMatches, `charm ".*" not found`)
5454
c.Check(err, jc.Satisfies, errors.IsNotFound)
55+
56+
// Ensure the document is actually gone.
57+
coll, closer := state.GetCollection(s.State, "charms")
58+
defer closer()
59+
count, err := coll.FindId(s.curl.String()).Count()
60+
c.Assert(err, jc.ErrorIsNil)
61+
c.Check(count, gc.Equals, 0)
5562
}
5663

5764
func (s *CharmSuite) TestAliveCharm(c *gc.C) {

‎state/charmref.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,17 +154,16 @@ func charmDestroyOps(st modelBackend, curl *charm.URL) ([]txn.Op, error) {
154154

155155
// charmRemoveOps implements the logic of charm.Remove.
156156
func charmRemoveOps(st modelBackend, curl *charm.URL) ([]txn.Op, error) {
157-
158157
charms, closer := st.getCollection(charmsC)
159158
defer closer()
160159

161-
// NOTE: we don't actually remove the charm document. The
162-
// "remove" terminology refers to the client's view of the change
163-
// (after which the charm really will be inaccessible).
164160
charmKey := curl.String()
165-
charmOp, err := nsLife.dieOp(charms, charmKey, nil)
161+
162+
// Remove the charm document as long as the charm is dying.
163+
charmOp, err := nsLife.dyingOp(charms, charmKey)
166164
if err != nil {
167165
return nil, errors.Annotate(err, "charm")
168166
}
167+
charmOp.Remove = true
169168
return []txn.Op{charmOp}, nil
170169
}

‎state/internal_test.go‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
package state
55

66
import (
7+
"fmt"
8+
79
"github.com/juju/errors"
810
jujutesting "github.com/juju/testing"
911
jc "github.com/juju/testing/checkers"
12+
"github.com/juju/utils"
1013
"github.com/juju/utils/clock"
1114
gc "gopkg.in/check.v1"
1215
"gopkg.in/juju/names.v2"
@@ -31,8 +34,9 @@ var _ = gc.Suite(&internalStateSuite{})
3134
type internalStateSuite struct {
3235
jujutesting.MgoSuite
3336
testing.BaseSuite
34-
state *State
35-
owner names.UserTag
37+
state *State
38+
owner names.UserTag
39+
modelCount int
3640
}
3741

3842
func (s *internalStateSuite) SetUpSuite(c *gc.C) {
@@ -96,6 +100,24 @@ func (s *internalStateSuite) TearDownTest(c *gc.C) {
96100
s.MgoSuite.TearDownTest(c)
97101
}
98102

103+
func (s *internalStateSuite) newState(c *gc.C) *State {
104+
s.modelCount++
105+
cfg := testing.CustomModelConfig(c, testing.Attrs{
106+
"name": fmt.Sprintf("testmodel%d", s.modelCount),
107+
"uuid": utils.MustNewUUID().String(),
108+
})
109+
_, st, err := s.state.NewModel(ModelArgs{
110+
CloudName: "dummy",
111+
CloudRegion: "dummy-region",
112+
Config: cfg,
113+
Owner: s.owner,
114+
StorageProviderRegistry: storage.StaticProviderRegistry{},
115+
})
116+
c.Assert(err, jc.ErrorIsNil)
117+
s.AddCleanup(func(*gc.C) { st.Close() })
118+
return st
119+
}
120+
99121
type internalStatePolicy struct{}
100122

101123
func (internalStatePolicy) Prechecker() (Prechecker, error) {

0 commit comments

Comments
 (0)