diff --git a/builder/dockerfile/add_copy.go b/builder/dockerfile/add_copy.go index a498dbd20f1af2a76737a457fd444c03f3d73185..c5d966e21aa3ca1276bf0777d99b0ca892e7f9ac 100644 --- a/builder/dockerfile/add_copy.go +++ b/builder/dockerfile/add_copy.go @@ -158,11 +158,8 @@ func (c *cmdBuilder) getCopyContextDir(from string) (string, func(), error) { } cleanup := func() { - if _, uerr := c.stage.localStore.Unmount(imgDesc.ContainerDesc.ContainerID, false); uerr != nil { - logrus.Warnf("Unmount layer[%s] for COPY from[%s] failed: %v", imgDesc.ContainerDesc.ContainerID, from, uerr) - } - if derr := c.stage.localStore.DeleteContainer(imgDesc.ContainerDesc.ContainerID); derr != nil { - logrus.Warnf("Delete layer[%s] for COPY from[%s] failed: %v", imgDesc.ContainerDesc.ContainerID, from, derr) + if cerr := c.stage.localStore.CleanContainer(imgDesc.ContainerDesc.ContainerID); cerr != nil { + logrus.Warnf("Clean layer[%s] for COPY from[%s] failed: %v", imgDesc.ContainerDesc.ContainerID, from, cerr) } } diff --git a/builder/dockerfile/stage_builder.go b/builder/dockerfile/stage_builder.go index 38cc0bf3b0f32ce387cea1a4067a60d0b2300a4b..ad77eceafe1612cf76702b8b678d74f74a08f9ea 100644 --- a/builder/dockerfile/stage_builder.go +++ b/builder/dockerfile/stage_builder.go @@ -353,19 +353,11 @@ func (s *stageBuilder) commit(ctx context.Context) (string, error) { // delete cleans up temporary resources which are created during stage building. func (s *stageBuilder) delete() error { - var retErr error - - if s.containerID != "" { - if _, err := s.localStore.Unmount(s.containerID, false); err != nil { - s.builder.Logger().Warnf("Unmount mountpoint of containerID[%q] for stage[%q] failed: %v", s.containerID, s.name, err) - retErr = errors.Wrapf(err, "unmount mountpoint of containerID[%q] for stage[%q] failed", s.containerID, s.name) - } - if err := s.localStore.DeleteContainer(s.containerID); err != nil { - s.builder.Logger().Warnf("Delete mountpoint of containerID[%q] for stage[%q] failed: %v", s.containerID, s.name, err) - retErr = errors.Wrapf(err, "delete mountpoint of containerID[%q] for stage[%q] failed", s.containerID, s.name) - } - s.mountpoint = "" + if s.containerID == "" { + return nil } - return retErr + s.mountpoint = "" + + return s.localStore.CleanContainer(s.containerID) } diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index c4f2ea80b33493a638b37ddd12df797c241a7f79..876cbfc1876b35e4cffb279a4682411c1a8cc59d 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -88,7 +88,7 @@ func runDaemon(cmd *cobra.Command, args []string) error { return err } // cleanup the residual container store if it exists - store.CleanContainerStore() + store.CleanContainers() // Ensure we have only one daemon running at the same time lock, err := util.SetDaemonLock(daemonOpts.RunRoot, lockFileName) if err != nil { diff --git a/daemon/daemon.go b/daemon/daemon.go index 504d192db6513a2009ec0a7bb2957568ee52d55b..64243d275520dd5c1c9475b947351bbb30c483e5 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -36,7 +36,6 @@ import ( "isula.org/isula-build/util" ) - // Options carries the options configured to daemon type Options struct { Debug bool @@ -86,8 +85,8 @@ func (d *Daemon) Run() (err error) { gc := gc.NewGC() gc.StartGC(ctx) - if err := d.registerSubReaper(gc); err != nil { - return err + if rerr := d.registerSubReaper(gc); rerr != nil { + return rerr } logrus.Debugf("Daemon start with option %#v", d.opts) @@ -189,7 +188,7 @@ func (d *Daemon) Cleanup() error { logrus.Info("Delete key failed") } d.deleteAllBuilders() - d.localStore.CleanContainerStore() + d.localStore.CleanContainers() _, err := d.localStore.Shutdown(false) return err } @@ -225,4 +224,3 @@ func (d *Daemon) registerSubReaper(g *gc.GarbageCollector) error { return g.RegisterGC(opt) } - diff --git a/store/store.go b/store/store.go index 9da2c8a751f5f40e29ca007a06f070d9f01d65e3..263d69e8956e8aaf66607fd3e8fa474f964d01a1 100644 --- a/store/store.go +++ b/store/store.go @@ -15,6 +15,8 @@ package store import ( + "sync" + is "github.com/containers/image/v5/storage" "github.com/containers/storage" "github.com/sirupsen/logrus" @@ -36,6 +38,7 @@ var ( type Store struct { // storage.Store wraps up the various types of file-based stores storage.Store + sync.RWMutex } // GetDefaultStoreOptions returns default store options. @@ -103,11 +106,11 @@ func GetStore() (Store, error) { is.Transport.SetStore(store) - return Store{store}, nil + return Store{Store: store}, nil } -// CleanContainerStore unmount the containers and delete them -func (s *Store) CleanContainerStore() { +// CleanContainers unmount the containers and delete them +func (s *Store) CleanContainers() { containers, err := s.Containers() if err != nil { logrus.Warn("Failed to get containers while cleaning the container store") @@ -115,11 +118,28 @@ func (s *Store) CleanContainerStore() { } for _, container := range containers { - if _, err := s.Unmount(container.ID, false); err != nil { - logrus.Warnf("Unmount container store failed while cleaning %q", container.ID) - } - if err := s.DeleteContainer(container.ID); err != nil { - logrus.Warnf("Delete container store failed while cleaning %q", container.ID) + if cerr := s.CleanContainer(container.ID); cerr != nil { + logrus.Warnf("Clean container %q failed", container.ID) } } } + +// CleanContainer cleans the container in store +func (s *Store) CleanContainer(id string) error { + s.Lock() + defer s.Unlock() + + // Do not care about all the errors whiling cleaning the container, + // just return one if the error occurs. + var err error + if _, uerr := s.Unmount(id, false); uerr != nil { + err = uerr + logrus.Warnf("Unmount container store failed while cleaning %q", id) + } + if derr := s.DeleteContainer(id); derr != nil { + err = derr + logrus.Warnf("Delete container store failed while cleaning %q", id) + } + + return err +}