From c1e9a85008b2a2d56ca0eb477a5ad4fa04ebe455 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Thu, 30 Apr 2015 11:53:13 +0200 Subject: [PATCH 1/2] store: define specific error for key not found. It's needed when you want to know if the key exists. --- store/store.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/store/store.go b/store/store.go index 8a9c257..a4f54dc 100644 --- a/store/store.go +++ b/store/store.go @@ -20,6 +20,7 @@ import ( "crypto/sha512" "database/sql" "encoding/json" + "errors" "fmt" "hash" "io" @@ -59,6 +60,10 @@ var diskvStores = [...]string{ "imageManifest", } +var ( + ErrKeyNotFound = errors.New("no keys found") +) + // Store encapsulates a content-addressable-storage for storing ACIs on disk. type Store struct { base string @@ -220,7 +225,7 @@ func (s Store) ResolveKey(key string) (string, error) { keyCount := len(aciInfos) if keyCount == 0 { - return "", fmt.Errorf("no keys found") + return "", ErrKeyNotFound } if keyCount != 1 { return "", fmt.Errorf("ambiguous key: %q", key) From 7c69744267eea9a88043912029598b03bcc6aaf0 Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Thu, 30 Apr 2015 11:53:13 +0200 Subject: [PATCH 2/2] rkt: add command to remove one or more images from the store. This adds a new command `rkt rmimage` to remove one or more images from the store providing a key uniquely resolvable. The logic is to firstly remove transactional data (the rows in the aciinfo and remote db tables for the given key) and then remove non transactional data (the diskv blob and imageManifest stores). The tree store is removed separately and only if an image hash related to the tree store key isn't referenced by any preparing/prepared/running pod. This means that an image can be removed (from the db and the blob/imageManifest stores) but not from the treestore. As removal of non transactional data can fail for multiple reasons some data can remain stale, the same applies if the treestore can't be removed because referenced by a pod. A future patch will provide a cas gc to clean the stale files and unreferenced treestores. Example: ``` $ rkt rmimage sha512-aa244c542b1b631bbd616bc266909b sha512-aaa wronghash rkt: successfully removed aci for key: "sha512-aa244c542b1b631bbd616bc266909b" rkt: key "sha512-aaa" not valid: no keys found rkt: wrong key "wronghash": badly formatted hash string rkt: 1 image(s) successfully removed rkt: 2 image(s) cannot be removed ``` --- rkt/rmimage.go | 151 ++++++++++++++++++++++++++++++++++++++++++++ store/aciinfo.go | 32 ++++++++++ store/remote.go | 9 +++ store/store.go | 87 +++++++++++++++++++++++-- store/store_test.go | 94 +++++++++++++++++++++++++++ 5 files changed, 369 insertions(+), 4 deletions(-) create mode 100644 rkt/rmimage.go diff --git a/rkt/rmimage.go b/rkt/rmimage.go new file mode 100644 index 0000000..b3fc962 --- /dev/null +++ b/rkt/rmimage.go @@ -0,0 +1,151 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "flag" + "fmt" + + "github.com/coreos/rkt/store" + + "github.com/coreos/rkt/Godeps/_workspace/src/github.com/appc/spec/schema/types" +) + +var ( + cmdRmImage = &Command{ + Name: "rmimage", + Summary: "Remove image(s) with the given key(s) from the local store", + Usage: "IMAGEID...", + Run: runRmImage, + Flags: &rmImageFlags, + } + rmImageFlags flag.FlagSet +) + +func init() { + commands = append(commands, cmdRmImage) +} + +func runRmImage(args []string) (exit int) { + if len(args) < 1 { + stderr("rkt: Must provide at least one image key") + return 1 + } + + s, err := store.NewStore(globalFlags.Dir) + if err != nil { + stderr("rkt: cannot open store: %v\n", err) + return 1 + } + + referencedImgs, err := getReferencedImgs(s) + if err != nil { + stderr("rkt: cannot get referenced images: %v\n", err) + return 1 + } + + //TODO(sgotti) Which return code to use when the removal fails only for some images? + done := 0 + errors := 0 + staleErrors := 0 + for _, pkey := range args { + errors++ + h, err := types.NewHash(pkey) + if err != nil { + stderr("rkt: wrong imageID %q: %v\n", pkey, err) + continue + } + key, err := s.ResolveKey(h.String()) + if err != nil { + stderr("rkt: imageID %q not valid: %v\n", pkey, err) + continue + } + if key == "" { + stderr("rkt: imageID %q doesn't exists\n", pkey) + continue + } + + err = s.RemoveACI(key) + if err != nil { + if serr, ok := err.(*store.StoreRemovalError); ok { + staleErrors++ + stderr("rkt: some files cannot be removed for imageID %q: %v\n", pkey, serr) + } else { + stderr("rkt: error removing aci for imageID %q: %v\n", pkey, err) + } + continue + } + stdout("rkt: successfully removed aci for imageID: %q\n", pkey) + + // Remove the treestore only if the image isn't referenced by + // some containers + // TODO(sgotti) there's a windows between getting refenced + // images and this check where a new container could be + // prepared/runned with this image. To avoid this a global lock + // is needed. + if _, ok := referencedImgs[key]; ok { + stderr("rkt: imageID is referenced by some containers, cannot remove the tree store") + continue + } else { + err = s.RemoveTreeStore(key) + if err != nil { + staleErrors++ + stderr("rkt: error removing treestore for imageID %q: %v\n", pkey, err) + continue + } + } + errors-- + done++ + } + + if done > 0 { + stdout("rkt: %d image(s) successfully removed\n", done) + } + if errors > 0 { + stdout("rkt: %d image(s) cannot be removed\n", errors) + } + if staleErrors > 0 { + stdout("rkt: %d image(s) removed but left some stale files\n", staleErrors) + } + return 0 +} + +func getReferencedImgs(s *store.Store) (map[string]struct{}, error) { + imgs := map[string]struct{}{} + walkErrors := []error{} + // Consider pods in preparing, prepared, run, exitedgarbage state + if err := walkPods(includeMostDirs, func(p *pod) { + appImgs, err := p.getAppsHashes() + if err != nil { + // Ignore errors reading/parsing pod file + return + } + for _, appImg := range appImgs { + key, err := s.ResolveKey(appImg.String()) + if err != nil && err != store.ErrKeyNotFound { + walkErrors = append(walkErrors, fmt.Errorf("bad imageID %q in pod definition: %v", appImg.String(), err)) + return + } + imgs[key] = struct{}{} + } + }); err != nil { + return nil, fmt.Errorf("failed to get pod handles: %v", err) + } + if len(walkErrors) > 0 { + return nil, fmt.Errorf("errors occured walking pods. errors: %v", walkErrors) + + } + return imgs, nil +} diff --git a/store/aciinfo.go b/store/aciinfo.go index a2db29f..f6fe03f 100644 --- a/store/aciinfo.go +++ b/store/aciinfo.go @@ -72,6 +72,29 @@ func GetACIInfosWithAppName(tx *sql.Tx, appname string) ([]*ACIInfo, bool, error return aciinfos, found, err } +// GetAciInfosWithBlobKey returns the ACIInfo with the given blobKey. found will be +// false if no aciinfo exists. +func GetACIInfoWithBlobKey(tx *sql.Tx, blobKey string) (*ACIInfo, bool, error) { + aciinfo := &ACIInfo{} + found := false + rows, err := tx.Query("SELECT * from aciinfo WHERE blobkey == $1", blobKey) + if err != nil { + return nil, false, err + } + for rows.Next() { + found = true + if err := rows.Scan(&aciinfo.BlobKey, &aciinfo.AppName, &aciinfo.ImportTime, &aciinfo.Latest); err != nil { + return nil, false, err + } + // No more than one row for blobkey must exist. + break + } + if err := rows.Err(); err != nil { + return nil, false, err + } + return aciinfo, found, err +} + // GetAllACIInfos returns all the ACIInfos sorted by optional sortfields and // with ascending or descending order. func GetAllACIInfos(tx *sql.Tx, sortfields []string, ascending bool) ([]*ACIInfo, error) { @@ -117,3 +140,12 @@ func WriteACIInfo(tx *sql.Tx, aciinfo *ACIInfo) error { return nil } + +// RemoveACIInfo removes the ACIInfo with the given blobKey. +func RemoveACIInfo(tx *sql.Tx, blobKey string) error { + _, err := tx.Exec("DELETE from aciinfo where blobkey == $1", blobKey) + if err != nil { + return err + } + return nil +} diff --git a/store/remote.go b/store/remote.go index 4345889..69fd444 100644 --- a/store/remote.go +++ b/store/remote.go @@ -70,3 +70,12 @@ func WriteRemote(tx *sql.Tx, remote *Remote) error { } return nil } + +// RemoveRemote removes the remote with the given blobKey. +func RemoveRemote(tx *sql.Tx, blobKey string) error { + _, err := tx.Exec("DELETE FROM remote WHERE blobkey == $1", blobKey) + if err != nil { + return err + } + return nil +} diff --git a/store/store.go b/store/store.go index a4f54dc..719c692 100644 --- a/store/store.go +++ b/store/store.go @@ -64,6 +64,22 @@ var ( ErrKeyNotFound = errors.New("no keys found") ) +// StoreRemovalError defines an error removing a non transactional store (like +// a diskv store or the tree store). +// When this happen there's the possibility that the store is left in an +// unclean state (for example with some stale files). +type StoreRemovalError struct { + errors []error +} + +func (e *StoreRemovalError) Error() string { + s := fmt.Sprintf("some aci disk entries cannot be removed: ") + for _, err := range e.errors { + s = s + fmt.Sprintf("[%v]", err) + } + return s +} + // Store encapsulates a content-addressable-storage for storing ACIs on disk. type Store struct { base string @@ -331,6 +347,57 @@ func (s Store) WriteACI(r io.Reader, latest bool) (string, error) { return key, nil } +// RemoveACI removes the ACI with the given key. It firstly removes the aci +// infos inside the db, then it tries to remove the non transactional data. +// If some error occurs removing some non transactional data a +// StoreRemovalError is returned. +func (ds Store) RemoveACI(key string) error { + imageKeyLock, err := lock.ExclusiveKeyLock(ds.imageLockDir, key) + if err != nil { + return fmt.Errorf("error locking image: %v", err) + } + defer imageKeyLock.Close() + + // Firstly remove aciinfo and remote from the db in an unique transaction. + // remote needs to be removed or a GetRemote will return a blobKey not + // referenced by any ACIInfo. + err = ds.db.Do(func(tx *sql.Tx) error { + if _, found, err := GetACIInfoWithBlobKey(tx, key); err != nil { + return fmt.Errorf("error getting aciinfo: %v", err) + } else if !found { + return fmt.Errorf("cannot find image with key: %s", key) + } + + if err := RemoveACIInfo(tx, key); err != nil { + return err + } + if err := RemoveRemote(tx, key); err != nil { + return err + } + return nil + }) + if err != nil { + return fmt.Errorf("cannot remove image with key: %s from db: %v", key, err) + } + + // Then remove non transactional entries from the blob, imageManifest + // and tree store. + // TODO(sgotti). Now that the ACIInfo is removed the image doesn't + // exists anymore, but errors removing non transactional entries can + // leave stale data that will require a cas GC to be implemented. + storeErrors := []error{} + for _, s := range ds.stores { + if err := s.Erase(key); err != nil { + // If there's an error save it and continue with the other stores + storeErrors = append(storeErrors, err) + } + } + if len(storeErrors) > 0 { + return &StoreRemovalError{errors: storeErrors} + } + return nil +} + // RenderTreeStore renders a treestore for the given image key if it's not // already fully rendered. // Users of treestore should call s.RenderTreeStore before using it to ensure @@ -358,12 +425,10 @@ func (s Store) RenderTreeStore(key string, rebuild bool) error { // Firstly remove a possible partial treestore if existing. // This is needed as a previous ACI removal operation could have failed // cleaning the tree store leaving some stale files. - err = s.treestore.Remove(key) - if err != nil { + if err := s.treestore.Remove(key); err != nil { return err } - err = s.treestore.Write(key, &s) - if err != nil { + if err := s.treestore.Write(key, &s); err != nil { return err } return nil @@ -395,6 +460,20 @@ func (s Store) GetTreeStoreRootFS(key string) string { return s.treestore.GetRootFS(key) } +// RemoveTreeStore removes the rendered image in tree store with the given key. +func (ds Store) RemoveTreeStore(key string) error { + treeStoreKeyLock, err := lock.ExclusiveKeyLock(ds.treeStoreLockDir, key) + if err != nil { + return fmt.Errorf("error locking tree store: %v", err) + } + defer treeStoreKeyLock.Close() + + if err := ds.treestore.Remove(key); err != nil { + return fmt.Errorf("error removing the tree store: %v", err) + } + return nil +} + // GetRemote tries to retrieve a remote with the given ACIURL. found will be // false if remote doesn't exist. func (s Store) GetRemote(aciURL string) (*Remote, bool, error) { diff --git a/store/store_test.go b/store/store_test.go index c87316e..0c492d5 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -449,5 +449,99 @@ func TestTreeStore(t *testing.T) { if err == nil { t.Fatalf("unexpected error: %v", err) } +} + +func TestRemoveACI(t *testing.T) { + dir, err := ioutil.TempDir("", tstprefix) + if err != nil { + t.Fatalf("error creating tempdir: %v", err) + } + defer os.RemoveAll(dir) + ds, err := NewStore(dir) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + imj := `{ + "acKind": "ImageManifest", + "acVersion": "0.4.0", + "name": "example.com/test01" + }` + + aciFile, err := aci.NewACI(dir, imj, nil) + if err != nil { + t.Fatalf("error creating test tar: %v", err) + } + // Rewind the ACI + if _, err := aciFile.Seek(0, 0); err != nil { + t.Fatalf("unexpected error %v", err) + } + key, err := ds.WriteACI(aciFile, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + aciURL := "http://example.com/test01.aci" + // Create our first Remote, and simulate Store() to create row in the table + na := NewRemote(aciURL, "") + na.BlobKey = key + ds.WriteRemote(na) + + err = ds.RemoveACI(key) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify that no remote for the specified key exists + _, found, err := ds.GetRemote(aciURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if found { + t.Fatalf("expected to find no remote, but a remote was found") + } + + // Try to remove a non-existent key + err = ds.RemoveACI("sha512-aaaaaaaaaaaaaaaaa") + if err == nil { + t.Fatalf("expected error") + } + + // Simulate error removing from the + imj = `{ + "acKind": "ImageManifest", + "acVersion": "0.3.0", + "name": "example.com/test01" + }` + + aciFile, err = aci.NewACI(dir, imj, nil) + if err != nil { + t.Fatalf("error creating test tar: %v", err) + } + // Rewind the ACI + if _, err := aciFile.Seek(0, 0); err != nil { + t.Fatalf("unexpected error %v", err) + } + key, err = ds.WriteACI(aciFile, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + aciURL = "http://example.com/test02.aci" + // Create our first Remote, and simulate Store() to create row in the table + na = NewRemote(aciURL, "") + na.BlobKey = key + ds.WriteRemote(na) + + err = os.Remove(filepath.Join(dir, "cas", "blob", blockTransform(key)[0], blockTransform(key)[1], key)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + err = ds.RemoveACI(key) + if err == nil { + t.Fatalf("expected error: %v", err) + } + if _, ok := err.(*StoreRemovalError); !ok { + t.Fatalf("expected StoreRemovalError got: %v", err) + } }