Commit Graph

21 Commits

Author SHA1 Message Date
Brian Goff eaad3ee3cf Make sure timers are stopped after use.
`time.After` keeps a timer running until the specified duration is
completed. It also allocates a new timer on each call. This can wind up
leaving lots of uneccessary timers running in the background that are
not needed and consume resources.

Instead of `time.After`, use `time.NewTimer` so the timer can actually
be stopped.
In some of these cases it's not a big deal since the duraiton is really
short, but in others it is much worse.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2019-01-16 14:32:53 -08:00
Daniel Nephin 4f0d95fa6e Add canonical import comment
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-02-05 16:51:57 -05:00
Derek McGowan 1009e6a40b
Update logrus to v1.0.1
Fixes case sensitivity issue

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
2017-07-31 13:16:46 -07:00
Brian Goff 5ea75bb6bf Move `StreamConfig` out of `runconfig`
`StreamConfig` carries with it a dep on libcontainerd, which is used by
other projects, but libcontainerd doesn't compile on all platforms, so
move it to `github.com/docker/docker/container/stream`

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-11-14 15:15:09 -05:00
Antonio Murdaca 44ccbb317c *: fix logrus.Warn[f]
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
2016-06-11 19:42:38 +02:00
Alexander Morozov b9966f3a81 daemon: remove some unused code
Signed-off-by: Alexander Morozov <lk4d4@docker.com>
2016-03-31 11:24:12 -07:00
Tonis Tiigi 9c4570a958 Replace execdrivers with containerd implementation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Kenfe-Mickael Laventure <mickael.laventure@gmail.com>
Signed-off-by: Anusha Ragunathan <anusha@docker.com>
2016-03-18 13:38:32 -07:00
Alexander Morozov 8706c5124a Remove obsolete comment
There is no more race

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
2016-03-09 09:38:39 -08:00
David Calavera a793564b25 Remove static errors from errors package.
Moving all strings to the errors package wasn't a good idea after all.

Our custom implementation of Go errors predates everything that's nice
and good about working with errors in Go. Take as an example what we
have to do to get an error message:

```go
func GetErrorMessage(err error) string {
	switch err.(type) {
	case errcode.Error:
		e, _ := err.(errcode.Error)
		return e.Message

	case errcode.ErrorCode:
		ec, _ := err.(errcode.ErrorCode)
		return ec.Message()

	default:
		return err.Error()
	}
}
```

This goes against every good practice for Go development. The language already provides a simple, intuitive and standard way to get error messages, that is calling the `Error()` method from an error. Reinventing the error interface is a mistake.

Our custom implementation also makes very hard to reason about errors, another nice thing about Go. I found several (>10) error declarations that we don't use anywhere. This is a clear sign about how little we know about the errors we return. I also found several error usages where the number of arguments was different than the parameters declared in the error, another clear example of how difficult is to reason about errors.

Moreover, our custom implementation didn't really make easier for people to return custom HTTP status code depending on the errors. Again, it's hard to reason about when to set custom codes and how. Take an example what we have to do to extract the message and status code from an error before returning a response from the API:

```go
	switch err.(type) {
	case errcode.ErrorCode:
		daError, _ := err.(errcode.ErrorCode)
		statusCode = daError.Descriptor().HTTPStatusCode
		errMsg = daError.Message()

	case errcode.Error:
		// For reference, if you're looking for a particular error
		// then you can do something like :
		//   import ( derr "github.com/docker/docker/errors" )
		//   if daError.ErrorCode() == derr.ErrorCodeNoSuchContainer { ... }

		daError, _ := err.(errcode.Error)
		statusCode = daError.ErrorCode().Descriptor().HTTPStatusCode
		errMsg = daError.Message

	default:
		// This part of will be removed once we've
		// converted everything over to use the errcode package

		// FIXME: this is brittle and should not be necessary.
		// If we need to differentiate between different possible error types,
		// we should create appropriate error types with clearly defined meaning
		errStr := strings.ToLower(err.Error())
		for keyword, status := range map[string]int{
			"not found":             http.StatusNotFound,
			"no such":               http.StatusNotFound,
			"bad parameter":         http.StatusBadRequest,
			"conflict":              http.StatusConflict,
			"impossible":            http.StatusNotAcceptable,
			"wrong login/password":  http.StatusUnauthorized,
			"hasn't been activated": http.StatusForbidden,
		} {
			if strings.Contains(errStr, keyword) {
				statusCode = status
				break
			}
		}
	}
```

You can notice two things in that code:

1. We have to explain how errors work, because our implementation goes against how easy to use Go errors are.
2. At no moment we arrived to remove that `switch` statement that was the original reason to use our custom implementation.

This change removes all our status errors from the errors package and puts them back in their specific contexts.
IT puts the messages back with their contexts. That way, we know right away when errors used and how to generate their messages.
It uses custom interfaces to reason about errors. Errors that need to response with a custom status code MUST implementent this simple interface:

```go
type errorWithStatus interface {
	HTTPErrorStatusCode() int
}
```

This interface is very straightforward to implement. It also preserves Go errors real behavior, getting the message is as simple as using the `Error()` method.

I included helper functions to generate errors that use custom status code in `errors/errors.go`.

By doing this, we remove the hard dependency we have eeverywhere to our custom errors package. Yes, you can use it as a helper to generate error, but it's still very easy to generate errors without it.

Please, read this fantastic blog post about errors in Go: http://dave.cheney.net/2014/12/24/inspecting-errors

Signed-off-by: David Calavera <david.calavera@gmail.com>
2016-02-26 15:49:09 -05:00
Zhang Wei ff3ea4c90f Update RestartPolicy of container
Add `--restart` flag for `update` command, so we can change restart
policy for a container no matter it's running or stopped.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
2016-02-20 17:06:32 +08:00
Zhang Wei 155714c596 Lock container when set state to restarting
After exec driver run, container lock is lost, so we should lock
container when changing its state to `restarting`

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
2016-02-02 19:50:06 +08:00
Daniel Dao 84e14754e1 only close LogDriver after LogCopier is done
this prevents the copier from sending messages in the buffer to the closed
driver. If the copied took longer than the timeout to drain the buffer, this
aborts the copier read loop and return back so we can cleanup resources
properly.

Signed-off-by: Daniel Dao <dqminh@cloudflare.com>
2016-01-18 17:47:57 +00:00
Brian Goff f093e1273d Revert "Break big lock into some tiny locks"
This reverts commit 1326f0cba5.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-01-14 13:38:09 -05:00
Sebastiaan van Stijn 967acd56c1 Merge pull request #18512 from euank/18510-fixOomKilled
Set OOMKilled state on any OOM event
2016-01-11 00:09:26 +01:00
Sebastiaan van Stijn a082f80832 Merge pull request #18736 from WeiZhang555/tiny-lock
Break big lock into some tiny locks for containerStart
2016-01-09 00:35:26 +01:00
David Calavera 907407d0b2 Modify import paths to point to the new engine-api package.
Signed-off-by: David Calavera <david.calavera@gmail.com>
2016-01-06 19:48:59 -05:00
Zhang Wei 1326f0cba5 Break big lock into some tiny locks
Don't involve code waiting for blocking channel in locked critical
section because it has potential risk of hanging forever.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
2015-12-23 13:23:23 +08:00
David Calavera 7ac4232e70 Move Config and HostConfig from runconfig to types/container.
- Make the API client library completely standalone.
- Move windows partition isolation detection to the client, so the
  driver doesn't use external types.

Signed-off-by: David Calavera <david.calavera@gmail.com>
2015-12-22 13:34:30 -05:00
Euan 0b5131444d Set OOMKilled state on any OOM event
This restores the behavior that existed prior to #16235 for setting
OOMKilled, while retaining the additional benefits it introduced around
emitting the oom event.

This also adds a test for the most obvious OOM cases which would have
caught this regression.

Fixes #18510

Signed-off-by: Euan <euank@amazon.com>
2015-12-15 19:27:57 +00:00
Justas Brazauskas 927b334ebf Fix typos found across repository
Signed-off-by: Justas Brazauskas <brazauskasjustas@gmail.com>
2015-12-13 18:04:12 +02:00
David Calavera 6bb0d1816a Move Container to its own package.
So other packages don't need to import the daemon package when they
want to use this struct.

Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
2015-12-03 17:39:49 +01:00