diff --git a/server/server.go b/server/server.go index 3b4165a9..8e53b001 100644 --- a/server/server.go +++ b/server/server.go @@ -134,7 +134,7 @@ func (s *Server) removeSandbox(id string) error { } func (s *Server) addContainer(c *oci.Container) error { - return s.state.AddContainer(c, c.Sandbox()) + return s.state.AddContainer(c) } func (s *Server) getContainer(id string) (*oci.Container, error) { diff --git a/server/state/file_state.go b/server/state/file_state.go index f42f581d..69f5f735 100644 --- a/server/state/file_state.go +++ b/server/state/file_state.go @@ -128,7 +128,6 @@ func (s *FileState) forceSyncWithDisk() error { } for _, sb := range oldSandboxes { if !newState.HasSandbox(sb.ID()) && sb.NetNs() != nil { - if err := sb.NetNsRemove(); err != nil { return fmt.Errorf("error closing network namespace of sandbox %v: %v", sb.ID(), err) } @@ -656,7 +655,7 @@ func NewFileState(statePath string, runtime *oci.Runtime) (Store, error) { // Perform an initial sync with the disk if err := state.forceSyncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return state, nil @@ -668,18 +667,18 @@ func (s *FileState) AddSandbox(sb *sandbox.Sandbox) error { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return fmt.Errorf("error syncing with on-disk state: %v", err) + return errSyncWithDisk(err) } if err := s.memoryState.AddSandbox(sb); err != nil { - return fmt.Errorf("error adding sandbox %v to in-memory state: %v", sb.ID(), err) + return err } if err := s.putSandboxToDisk(sb); err != nil { if err2 := s.memoryState.DeleteSandbox(sb.ID()); err2 != nil { logrus.Errorf("error removing sandbox %s from in-memory state, states are desynced: %v", sb.ID(), err) } - return err + return fmt.Errorf("error saving sandbox %s to disk: %v", sb.ID(), err) } return nil @@ -705,15 +704,15 @@ func (s *FileState) DeleteSandbox(id string) error { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return fmt.Errorf("error syncing with on-disk state: %v", err) + return errSyncWithDisk(err) } if err := s.memoryState.DeleteSandbox(id); err != nil { - return fmt.Errorf("error removing sandbox %v from in-memory state: %v", id, err) + return err } if err := s.removeSandboxFromDisk(id); err != nil { - return err + return fmt.Errorf("error removing sandbox ID %s from disk: %v", id, err) } return nil @@ -725,7 +724,7 @@ func (s *FileState) GetSandbox(id string) (*sandbox.Sandbox, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.GetSandbox(id) @@ -737,7 +736,7 @@ func (s *FileState) LookupSandboxByName(name string) (*sandbox.Sandbox, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.LookupSandboxByName(name) @@ -750,7 +749,7 @@ func (s *FileState) LookupSandboxByID(id string) (*sandbox.Sandbox, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.LookupSandboxByID(id) @@ -762,30 +761,30 @@ func (s *FileState) GetAllSandboxes() ([]*sandbox.Sandbox, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.GetAllSandboxes() } // AddContainer adds a container to the state -func (s *FileState) AddContainer(c *oci.Container, sandboxID string) error { +func (s *FileState) AddContainer(c *oci.Container) error { s.lockfile.Lock() defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return fmt.Errorf("error syncing with on-disk state: %v", err) + return errSyncWithDisk(err) } - if err := s.memoryState.AddContainer(c, sandboxID); err != nil { - return fmt.Errorf("error adding container %v to in-memory state: %v", c.ID(), err) + if err := s.memoryState.AddContainer(c); err != nil { + return err } if err := s.putContainerToDisk(c, true); err != nil { - if err2 := s.memoryState.DeleteContainer(c.ID(), sandboxID); err2 != nil { + if err2 := s.memoryState.DeleteContainer(c.ID(), c.Sandbox()); err2 != nil { logrus.Errorf("error removing container %v from in-memory state, states are desynced: %v", c.ID(), err2) } - return err + return fmt.Errorf("error adding container %s to on-disk state: %v", c.ID(), err) } return nil @@ -811,15 +810,15 @@ func (s *FileState) DeleteContainer(id, sandboxID string) error { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return fmt.Errorf("error syncing with on-disk state: %v", err) + return errSyncWithDisk(err) } if err := s.memoryState.DeleteContainer(id, sandboxID); err != nil { - return fmt.Errorf("error removing container %v from in-memory state: %v", id, err) + return err } if err := s.removeContainerFromDisk(id, sandboxID); err != nil { - return err + return fmt.Errorf("error removing container %s from on-disk state: %v", id, err) } return nil @@ -831,7 +830,7 @@ func (s *FileState) GetContainer(id, sandboxID string) (*oci.Container, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.GetContainer(id, sandboxID) @@ -843,7 +842,7 @@ func (s *FileState) GetContainerSandbox(id string) (string, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return "", fmt.Errorf("error syncing with on-disk state: %v", err) + return "", errSyncWithDisk(err) } return s.memoryState.GetContainerSandbox(id) @@ -855,7 +854,7 @@ func (s *FileState) LookupContainerByName(name string) (*oci.Container, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.LookupContainerByName(name) @@ -868,7 +867,7 @@ func (s *FileState) LookupContainerByID(id string) (*oci.Container, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.LookupContainerByID(id) @@ -881,8 +880,12 @@ func (s *FileState) GetAllContainers() ([]*oci.Container, error) { defer s.lockfile.Unlock() if err := s.syncWithDisk(); err != nil { - return nil, fmt.Errorf("error syncing with on-disk state: %v", err) + return nil, errSyncWithDisk(err) } return s.memoryState.GetAllContainers() } + +func errSyncWithDisk(err error) error { + return fmt.Errorf("error syncing on-disk state: %v", err) +} diff --git a/server/state/in_memory_state.go b/server/state/in_memory_state.go index c563485f..b2d8d798 100644 --- a/server/state/in_memory_state.go +++ b/server/state/in_memory_state.go @@ -42,6 +42,10 @@ func (s *InMemoryState) AddSandbox(sandbox *sandbox.Sandbox) error { s.lock.Lock() defer s.lock.Unlock() + if sandbox == nil { + return fmt.Errorf("nil passed as sandbox to AddSandbox") + } + if _, exist := s.sandboxes[sandbox.ID()]; exist { return fmt.Errorf("sandbox with ID %v already exists", sandbox.ID()) } @@ -93,7 +97,7 @@ func (s *InMemoryState) DeleteSandbox(id string) error { defer s.lock.Unlock() if _, exist := s.sandboxes[id]; !exist { - return fmt.Errorf("no sandbox with ID %v exists, cannot delete", id) + return errNoSuchSandbox(id) } name := s.sandboxes[id].Name() @@ -128,7 +132,7 @@ func (s *InMemoryState) GetSandbox(id string) (*sandbox.Sandbox, error) { sandbox, ok := s.sandboxes[id] if !ok { - return nil, fmt.Errorf("no sandbox with id %v exists", id) + return nil, errNoSuchSandbox(id) } return sandbox, nil @@ -146,7 +150,7 @@ func (s *InMemoryState) LookupSandboxByName(name string) (*sandbox.Sandbox, erro sandbox, ok := s.sandboxes[id] if !ok { - // This should never happen + // This should never happen - our internal state must be desynced return nil, fmt.Errorf("cannot find sandbox %v in sandboxes map", id) } @@ -166,7 +170,7 @@ func (s *InMemoryState) LookupSandboxByID(id string) (*sandbox.Sandbox, error) { sandbox, ok := s.sandboxes[fullID] if !ok { - // This should never happen + // This should never happen, internal state must be desynced return nil, fmt.Errorf("cannot find sandbox %v in sandboxes map", fullID) } @@ -187,25 +191,25 @@ func (s *InMemoryState) GetAllSandboxes() ([]*sandbox.Sandbox, error) { } // AddContainer adds a single container to a given sandbox in the state -func (s *InMemoryState) AddContainer(c *oci.Container, sandboxID string) error { +func (s *InMemoryState) AddContainer(c *oci.Container) error { s.lock.Lock() defer s.lock.Unlock() - if c.Sandbox() != sandboxID { - return fmt.Errorf("cannot add container to sandbox %v as it is part of sandbox %v", sandboxID, c.Sandbox()) + if c == nil { + return fmt.Errorf("nil passed as container to AddContainer") } - sandbox, ok := s.sandboxes[sandboxID] + sandbox, ok := s.sandboxes[c.Sandbox()] if !ok { - return fmt.Errorf("sandbox with ID %v does not exist, cannot add container", sandboxID) + return errNoSuchSandbox(c.Sandbox()) } if ctr := sandbox.GetContainer(c.ID()); ctr != nil { - return fmt.Errorf("container with ID %v already exists in sandbox %v", c.ID(), sandboxID) + return fmt.Errorf("container with ID %v already exists in sandbox %v", c.ID(), c.Sandbox()) } if sandbox.InfraContainer().ID() == c.ID() { - return fmt.Errorf("container is infra container of sandbox %s, refusing to add to containers list", sandboxID) + return fmt.Errorf("container is infra container of sandbox %s, refusing to add to containers list", c.Sandbox()) } sandbox.AddContainer(c) @@ -260,12 +264,12 @@ func (s *InMemoryState) DeleteContainer(id, sandboxID string) error { sandbox, ok := s.sandboxes[sandboxID] if !ok { - return fmt.Errorf("sandbox with ID %v does not exist", sandboxID) + return errNoSuchSandbox(sandboxID) } ctr := sandbox.GetContainer(id) if ctr == nil { - return fmt.Errorf("sandbox %v has no container with ID %v", sandboxID, id) + return errNoSuchContainerInSandbox(id, sandboxID) } sandbox.RemoveContainer(id) @@ -306,7 +310,7 @@ func (s *InMemoryState) GetContainerSandbox(id string) (string, error) { ctr := s.containers.Get(id) if ctr == nil { - return "", fmt.Errorf("no container with ID %v found", id) + return "", errNoSuchContainer(id) } return ctr.Sandbox(), nil @@ -350,7 +354,7 @@ func (s *InMemoryState) GetAllContainers() ([]*oci.Container, error) { func (s *InMemoryState) getContainer(id string) (*oci.Container, error) { ctr := s.containers.Get(id) if ctr == nil { - return nil, fmt.Errorf("cannot find container with ID %v", id) + return nil, errNoSuchContainer(id) } return s.getContainerFromSandbox(id, ctr.Sandbox()) @@ -361,13 +365,25 @@ func (s *InMemoryState) getContainer(id string) (*oci.Container, error) { func (s *InMemoryState) getContainerFromSandbox(id, sandboxID string) (*oci.Container, error) { sandbox, ok := s.sandboxes[sandboxID] if !ok { - return nil, fmt.Errorf("sandbox with ID %v does not exist", sandboxID) + return nil, errNoSuchSandbox(sandboxID) } ctr := sandbox.GetContainer(id) if ctr == nil { - return nil, fmt.Errorf("cannot find container %v in sandbox %v", id, sandboxID) + return nil, errNoSuchContainerInSandbox(id, sandboxID) } return ctr, nil } + +func errNoSuchSandbox(id string) error { + return fmt.Errorf("no sandbox with ID %s found", id) +} + +func errNoSuchContainer(id string) error { + return fmt.Errorf("no container with ID %s found", id) +} + +func errNoSuchContainerInSandbox(id string, sandboxID string) error { + return fmt.Errorf("no container with ID %s in sandbox %s", id, sandboxID) +} diff --git a/server/state/state_store.go b/server/state/state_store.go index d7662285..f96ce0de 100644 --- a/server/state/state_store.go +++ b/server/state/state_store.go @@ -11,7 +11,7 @@ type Store interface { HasSandbox(id string) bool DeleteSandbox(id string) error // These should modify the associated sandbox without prompting - AddContainer(c *oci.Container, sandboxID string) error + AddContainer(c *oci.Container) error HasContainer(id, sandboxID string) bool DeleteContainer(id, sandboxID string) error // These two require full, explicit ID