diff options
-rw-r--r-- | .github/workflows/stale.yml | 25 | ||||
-rw-r--r-- | libpod/container.go | 9 | ||||
-rw-r--r-- | libpod/container_api.go | 44 | ||||
-rw-r--r-- | libpod/container_internal.go | 45 | ||||
-rw-r--r-- | libpod/networking_linux.go | 28 | ||||
-rw-r--r-- | libpod/runtime_volume_linux.go | 9 | ||||
-rw-r--r-- | pkg/adapter/containers.go | 6 | ||||
-rw-r--r-- | pkg/network/devices.go | 17 | ||||
-rw-r--r-- | pkg/network/files.go | 26 |
9 files changed, 148 insertions, 61 deletions
diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml new file mode 100644 index 000000000..44cb82ff0 --- /dev/null +++ b/.github/workflows/stale.yml @@ -0,0 +1,25 @@ +name: Mark stale issues and pull requests + +# Please refer to https://github.com/actions/stale/blob/master/action.yml +# to see all config knobs of the stale action. + +on: + schedule: + - cron: "0 0 * * *" + +jobs: + stale: + + runs-on: ubuntu-latest + + steps: + - uses: actions/stale@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + stale-issue-message: 'This issue had no activity for 30 days. In the absence of activity or the "do-not-close" label, the issue will be automatically closed within 7 days.' + stale-pr-message: 'This pull request had no activity for 30 days. In the absence of activity or the "do-not-close" label, the pull request will be automatically closed within 7 days.' + stale-issue-label: 'stale-issue' + stale-pr-label: 'stale-pr' + days-before-stale: 30 + days-before-close: 7 + exempt-pr-label: 'do-not-close' diff --git a/libpod/container.go b/libpod/container.go index 7be73b3c3..fc9ef0c86 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -1185,3 +1185,12 @@ func (c *Container) HasHealthCheck() bool { func (c *Container) HealthCheckConfig() *manifest.Schema2HealthConfig { return c.config.HealthCheckConfig } + +// AutoRemove indicates whether the container will be removed after it is executed +func (c *Container) AutoRemove() bool { + spec := c.config.Spec + if spec.Annotations == nil { + return false + } + return c.Spec().Annotations[InspectAnnotationAutoremove] == InspectResponseTrue +} diff --git a/libpod/container_api.go b/libpod/container_api.go index 759a7067e..1b2d52ce3 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -32,9 +32,7 @@ func (c *Container) Init(ctx context.Context) (err error) { } } - if !(c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateStopped || - c.state.State == define.ContainerStateExited) { + if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s has already been created in runtime", c.ID()) } @@ -176,15 +174,12 @@ func (c *Container) StopWithTimeout(timeout uint) error { } } - if c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateUnknown || - c.state.State == define.ContainerStatePaused { - return errors.Wrapf(define.ErrCtrStateInvalid, "can only stop created, running, or stopped containers. %s is in state %s", c.ID(), c.state.State.String()) + if c.ensureState(define.ContainerStateStopped, define.ContainerStateExited) { + return define.ErrCtrStopped } - if c.state.State == define.ContainerStateStopped || - c.state.State == define.ContainerStateExited { - return define.ErrCtrStopped + if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) { + return errors.Wrapf(define.ErrCtrStateInvalid, "can only stop created or running containers. %s is in state %s", c.ID(), c.state.State.String()) } return c.stop(timeout, false) @@ -201,6 +196,7 @@ func (c *Container) Kill(signal uint) error { } } + // TODO: Is killing a paused container OK? if c.state.State != define.ContainerStateRunning { return errors.Wrapf(define.ErrCtrStateInvalid, "can only kill running containers. %s is in state %s", c.ID(), c.state.State.String()) } @@ -234,10 +230,7 @@ func (c *Container) Exec(tty, privileged bool, env map[string]string, cmd []stri } } - conState := c.state.State - - // TODO can probably relax this once we track exec sessions - if conState != define.ContainerStateRunning { + if c.state.State != define.ContainerStateRunning { return define.ExecErrorCodeCannotInvoke, errors.Wrapf(define.ErrCtrStateInvalid, "cannot exec into container that is not running") } @@ -391,11 +384,10 @@ func (c *Container) Attach(streams *AttachStreams, keys string, resize <-chan re c.lock.Unlock() } - if c.state.State != define.ContainerStateCreated && - c.state.State != define.ContainerStateRunning && - c.state.State != define.ContainerStateExited { + if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning) { return errors.Wrapf(define.ErrCtrStateInvalid, "can only attach to created or running containers") } + defer c.newContainerEvent(events.Attach) return c.attach(streams, keys, resize, false, nil) } @@ -432,7 +424,7 @@ func (c *Container) Unmount(force bool) error { return errors.Wrapf(err, "can't determine how many times %s is mounted, refusing to unmount", c.ID()) } if mounted == 1 { - if c.state.State == define.ContainerStateRunning || c.state.State == define.ContainerStatePaused { + if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { return errors.Wrapf(define.ErrCtrStateInvalid, "cannot unmount storage for container %s as it is running or paused", c.ID()) } if len(c.state.ExecSessions) != 0 { @@ -574,7 +566,7 @@ func (c *Container) Cleanup(ctx context.Context) error { } // Check if state is good - if c.state.State == define.ContainerStateRunning || c.state.State == define.ContainerStatePaused { + if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is running or paused, refusing to clean up", c.ID()) } @@ -652,9 +644,7 @@ func (c *Container) Sync() error { // If runtime knows about the container, update its status in runtime // And then save back to disk - if (c.state.State != define.ContainerStateUnknown) && - (c.state.State != define.ContainerStateConfigured) && - (c.state.State != define.ContainerStateExited) { + if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopped) { oldState := c.state.State if err := c.ociRuntime.UpdateContainerStatus(c); err != nil { return err @@ -666,6 +656,7 @@ func (c *Container) Sync() error { } } } + defer c.newContainerEvent(events.Sync) return nil } @@ -840,12 +831,3 @@ func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOpti defer c.newContainerEvent(events.Restore) return c.restore(ctx, options) } - -// AutoRemove indicates whether the container will be removed after it is executed -func (c *Container) AutoRemove() bool { - spec := c.config.Spec - if spec.Annotations == nil { - return false - } - return c.Spec().Annotations[InspectAnnotationAutoremove] == InspectResponseTrue -} diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 0043c9651..028d7601d 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -328,7 +328,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er // Is the container running again? // If so, we don't have to do anything - if c.state.State == define.ContainerStateRunning || c.state.State == define.ContainerStatePaused { + if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { return false, nil } else if c.state.State == define.ContainerStateUnknown { return false, errors.Wrapf(define.ErrInternal, "invalid container state encountered in restart attempt!") @@ -359,8 +359,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er if err := c.reinit(ctx, true); err != nil { return false, err } - } else if c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateExited { + } else if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) { // Initialize the container if err := c.init(ctx, true); err != nil { return false, err @@ -372,6 +371,18 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er return true, nil } +// Ensure that the container is in a specific state or state. +// Returns true if the container is in one of the given states, +// or false otherwise. +func (c *Container) ensureState(states ...define.ContainerStatus) bool { + for _, state := range states { + if state == c.state.State { + return true + } + } + return false +} + // Sync this container with on-disk state and runtime status // Should only be called with container lock held // This function should suffice to ensure a container's state is accurate and @@ -382,9 +393,7 @@ func (c *Container) syncContainer() error { } // If runtime knows about the container, update its status in runtime // And then save back to disk - if (c.state.State != define.ContainerStateUnknown) && - (c.state.State != define.ContainerStateConfigured) && - (c.state.State != define.ContainerStateExited) { + if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopped, define.ContainerStatePaused) { oldState := c.state.State if err := c.checkExitFile(); err != nil { @@ -516,7 +525,7 @@ func (c *Container) setupStorage(ctx context.Context) error { // Tear down a container's storage prior to removal func (c *Container) teardownStorage() error { - if c.state.State == define.ContainerStateRunning || c.state.State == define.ContainerStatePaused { + if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { return errors.Wrapf(define.ErrCtrStateInvalid, "cannot remove storage for container %s as it is running or paused", c.ID()) } @@ -721,10 +730,7 @@ func (c *Container) save() error { // Otherwise, this function will return with error if there are dependencies of this container that aren't running. func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err error) { // Container must be created or stopped to be started - if !(c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateCreated || - c.state.State == define.ContainerStateStopped || - c.state.State == define.ContainerStateExited) { + if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID()) } @@ -755,8 +761,7 @@ func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err err if err := c.reinit(ctx, false); err != nil { return err } - } else if c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateExited { + } else if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) { // Or initialize it if necessary if err := c.init(ctx, false); err != nil { return err @@ -987,7 +992,7 @@ func (c *Container) cleanupRuntime(ctx context.Context) error { // If the container is not ContainerStateStopped or // ContainerStateCreated, do nothing. - if c.state.State != define.ContainerStateStopped && c.state.State != define.ContainerStateCreated { + if !c.ensureState(define.ContainerStateStopped, define.ContainerStateCreated) { return nil } @@ -1078,8 +1083,7 @@ func (c *Container) initAndStart(ctx context.Context) (err error) { if err := c.reinit(ctx, false); err != nil { return err } - } else if c.state.State == define.ContainerStateConfigured || - c.state.State == define.ContainerStateExited { + } else if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) { if err := c.init(ctx, false); err != nil { return err } @@ -1205,7 +1209,7 @@ func (c *Container) unpause() error { // Internal, non-locking function to restart a container func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err error) { - if c.state.State == define.ContainerStateUnknown || c.state.State == define.ContainerStatePaused { + if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopped, define.ContainerStateExited) { return errors.Wrapf(define.ErrCtrStateInvalid, "unable to restart a container in a paused or unknown state") } @@ -1733,9 +1737,8 @@ func (c *Container) checkReadyForRemoval() error { return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is in invalid state", c.ID()) } - if c.state.State == define.ContainerStateRunning || - c.state.State == define.ContainerStatePaused { - return errors.Wrapf(define.ErrCtrStateInvalid, "cannot remove container %s as it is %s - running or paused containers cannot be removed", c.ID(), c.state.State.String()) + if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { + return errors.Wrapf(define.ErrCtrStateInvalid, "cannot remove container %s as it is %s - running or paused containers cannot be removed without force", c.ID(), c.state.State.String()) } if len(c.state.ExecSessions) != 0 { @@ -1816,7 +1819,7 @@ func (c *Container) sortUserVolumes(ctrSpec *spec.Spec) ([]*ContainerNamedVolume // Check for an exit file, and handle one if present func (c *Container) checkExitFile() error { // If the container's not running, nothing to do. - if c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused { + if !c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) { return nil } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 4360c8c15..daa0619a2 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -5,6 +5,7 @@ package libpod import ( "crypto/rand" "fmt" + "io/ioutil" "net" "os" "os/exec" @@ -131,7 +132,7 @@ func checkSlirpFlags(path string) (bool, bool, bool, error) { cmd := exec.Command(path, "--help") out, err := cmd.CombinedOutput() if err != nil { - return false, false, false, err + return false, false, false, errors.Wrapf(err, "slirp4netns %q", out) } return strings.Contains(string(out), "--disable-host-loopback"), strings.Contains(string(out), "--mtu"), strings.Contains(string(out), "--enable-sandbox"), nil } @@ -158,6 +159,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { havePortMapping := len(ctr.Config().PortMappings) > 0 apiSocket := filepath.Join(ctr.runtime.config.TmpDir, fmt.Sprintf("%s.net", ctr.config.ID)) + logPath := filepath.Join(ctr.runtime.config.TmpDir, fmt.Sprintf("slirp4netns-%s.log", ctr.config.ID)) cmdArgs := []string{} if havePortMapping { @@ -165,7 +167,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { } dhp, mtu, sandbox, err := checkSlirpFlags(path) if err != nil { - return errors.Wrapf(err, "error checking slirp4netns binary %s", path) + return errors.Wrapf(err, "error checking slirp4netns binary %s: %q", path, err) } if dhp { cmdArgs = append(cmdArgs, "--disable-host-loopback") @@ -210,6 +212,18 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { // Leak one end of the pipe in slirp4netns, the other will be sent to conmon cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessSlirpSyncR, syncW) + logFile, err := os.Create(logPath) + if err != nil { + return errors.Wrapf(err, "failed to open slirp4netns log file %s", logPath) + } + defer logFile.Close() + // Unlink immediately the file so we won't need to worry about cleaning it up later. + // It is still accessible through the open fd logFile. + if err := os.Remove(logPath); err != nil { + return errors.Wrapf(err, "delete file %s", logPath) + } + cmd.Stdout = logFile + cmd.Stderr = logFile if err := cmd.Start(); err != nil { return errors.Wrapf(err, "failed to start slirp4netns process") } @@ -238,7 +252,15 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) { continue } if status.Exited() { - return errors.New("slirp4netns failed") + // Seek at the beginning of the file and read all its content + if _, err := logFile.Seek(0, 0); err != nil { + logrus.Errorf("could not seek log file: %q", err) + } + logContent, err := ioutil.ReadAll(logFile) + if err != nil { + return errors.Wrapf(err, "slirp4netns failed") + } + return errors.Errorf("slirp4netns failed: %q", logContent) } if status.Signaled() { return errors.New("slirp4netns killed by signal") diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index ba4fff4be..5b05acea4 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -48,6 +48,15 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) } volume.config.CreatedTime = time.Now() + // Check if volume with given name exists. + exists, err := r.state.HasVolume(volume.config.Name) + if err != nil { + return nil, errors.Wrapf(err, "error checking if volume with name %s exists", volume.config.Name) + } + if exists { + return nil, errors.Wrapf(define.ErrVolumeExists, "volume with name %s already exists", volume.config.Name) + } + if volume.config.Driver == define.VolumeDriverLocal { logrus.Debugf("Validating options for local driver") // Validate options diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go index bff93cc9e..73e6dba3a 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -438,7 +438,11 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode if c.IsSet("rm") { if err := r.Runtime.RemoveContainer(ctx, ctr, false, true); err != nil { - logrus.Errorf("Error removing container %s: %v", ctr.ID(), err) + if errors.Cause(err) == define.ErrNoSuchCtr { + logrus.Warnf("Container %s does not exist: %v", ctr.ID(), err) + } else { + logrus.Errorf("Error removing container %s: %v", ctr.ID(), err) + } } } diff --git a/pkg/network/devices.go b/pkg/network/devices.go index 85068a7d1..78e1a5aa5 100644 --- a/pkg/network/devices.go +++ b/pkg/network/devices.go @@ -24,19 +24,26 @@ func GetFreeDeviceName() (string, error) { if err != nil { return "", err } + bridgeNames, err := GetBridgeNamesFromFileSystem() + if err != nil { + return "", err + } for { deviceName = fmt.Sprintf("%s%d", CNIDeviceName, deviceNum) - logrus.Debugf("checking if device name %s exists in other cni networks", deviceName) + logrus.Debugf("checking if device name %q exists in other cni networks", deviceName) if util.StringInSlice(deviceName, networkNames) { deviceNum++ continue } - logrus.Debugf("checking if device name %s exists in live networks", deviceName) - if !util.StringInSlice(deviceName, liveNetworksNames) { + logrus.Debugf("checking if device name %q exists in live networks", deviceName) + if util.StringInSlice(deviceName, liveNetworksNames) { + deviceNum++ + continue + } + logrus.Debugf("checking if device name %q already exists as a bridge name ", deviceName) + if !util.StringInSlice(deviceName, bridgeNames) { break } - // TODO Still need to check the bridge names for a conflict but I dont know - // how to get them yet! deviceNum++ } return deviceName, nil diff --git a/pkg/network/files.go b/pkg/network/files.go index d55ec2dfd..2f3932974 100644 --- a/pkg/network/files.go +++ b/pkg/network/files.go @@ -129,3 +129,29 @@ func GetInterfaceNameFromConfig(path string) (string, error) { } return name, nil } + +// GetBridgeNamesFromFileSystem is a convenience function to get all the bridge +// names from the configured networks +func GetBridgeNamesFromFileSystem() ([]string, error) { + var bridgeNames []string + networks, err := LoadCNIConfsFromDir(CNIConfigDir) + if err != nil { + return nil, err + } + for _, n := range networks { + var name string + // iterate network conflists + for _, cniplugin := range n.Plugins { + // iterate plugins + if cniplugin.Network.Type == "bridge" { + plugin := make(map[string]interface{}) + if err := json.Unmarshal(cniplugin.Bytes, &plugin); err != nil { + continue + } + name = plugin["bridge"].(string) + } + } + bridgeNames = append(bridgeNames, name) + } + return bridgeNames, nil +} |