/juju

apiserver: Remove charm get cache #6302

Merged
merged 2 commits into from Sep 22, 2016

Conversation

Reviewers

@reedobrien reedobrien

@axw axw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@mjs @jujubot @reedobrien @axw
@mjs
Contributor

mjs commented Sep 21, 2016 •
Edited 1 time

The filesystem cache used when retrieving charms has been removed. It
was implemented when charms lived in a remote S3 bucket and is no longer
necessary now that charm live in MongoDB's GridFS store. This helps to
reduce disk space usage in the controllers and is one less thing that
needs cleaning up when models are removed.

See https://bugs.launchpad.net/juju/+bug/1608959

QA

Manually deployed charms and confirmed that old cache directory at <datadir>/charm-get-cache is no longer created and new <datadir>/charm-get-tmp directory is left empty.

Manually created the old <datadir>/charm-get-cache directory and populated it and then ran an upgrade. The directory was removed.
@reedobrien

reedobrien approved these changes Sep 21, 2016
View changes

LGTM, but IANAGR.
apiserver tests pass on branch.
I don't see QA so I can't validate that.

apiserver/charms.go
-// saves the corresponding zip archive to the given charmArchivePath.
-func (h *charmsHandler) downloadCharm(st *state.State, curl *charm.URL, charmArchivePath string) error {
+ // Ensure the working directory exists.
+ tmpDir := filepath.Join(h.dataDir, "charm-get-tmp")
@axw

axw Sep 21, 2016

Member

It would be great if we could stop using the temp-dir altogether. If we made some tweaks to the state/storage.Storage interface so that it returned an io.ReaderAt, we could do that.

mgo.GridFile does not implement io.ReaderAt, but it easily could from the looks of it. We could either add that upstream, or wrap the GridFile to seek&read (but would need to do so in a goroutine safe way).

@mjs

mjs Sep 22, 2016

Contributor

I like this idea but I might do this in a follow up PR

@axw

axw approved these changes Sep 22, 2016
View changes

LGTM. Please add QA.

@mjs
Contributor

mjs commented Sep 22, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Sep 22, 2016

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

jujubot commented Sep 22, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9301

mjs added some commits Sep 21, 2016

@mjs
apiserver: Remove charm get cache 
The filesystem cache used when retrieving charms has been removed. It
was implemented when charms lived in a remote S3 bucket and is no longer
necessary now that charm live in MongoDB's GridFS store. This helps to
reduce disk space usage in the controllers and is one less thing that
needs cleaning up when models are removed.

See https://bugs.launchpad.net/juju/+bug/1608959
965dda8
@mjs
upgrades: Remove old apiserver charm get cache 
This takes care of any charm get cache created by the apiserver in an
RC1 or earlier release.

Note the new style of testing upgrade steps (using the new findSteps
helper). This approach both ensures that the step is correctly wired up
and does what it's supposed to in the one test, and doesn't require
much in export_test.go.

The old test helpers have been removed.
c795129
@mjs
Contributor

mjs commented Sep 22, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Sep 22, 2016

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

jujubot commented Sep 22, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9302
@mjs
Contributor

mjs commented Sep 22, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Sep 22, 2016

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

@jujubot jujubot merged commit 609d4dc into juju:master Sep 22, 2016

@mjs mjs deleted the mjs:1608959-remove-apiserver-charm-cache branch Sep 22, 2016

@mjs mjs referenced this pull request Sep 23, 2016

Merged

apiserver: Remove charm get cache #6309

jujubot added a commit that referenced this pull request Sep 23, 2016

@jujubot
Merge pull request #6309 from mjs/1626304-remove-charm-get-cache-1.25 
apiserver: Remove charm get cache

(this is a backport from 2.0 - see also: #6302)

The filesystem cache used when retrieving charms has been removed. It was implemented when charms lived in a remote S3 bucket and is no longer necessary now that charm live in MongoDB's GridFS store. This helps to reduce disk space usage in the controllers and is one less thing that needs cleaning up when models are removed.

An upgrade step was also added. Now that the apiserver's charm get cache is no longer used it is removed to free up disk space.

This is part of the fix for https://bugs.launchpad.net/juju/+bug/1626304
db061a3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment