1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00
Commit graph

26 commits

Author SHA1 Message Date
Yong Tang
3f6e3a0885 Fix docker rename with linked containers
This fix tries to address issue raised in #23973 where the
`docker rename` does not update namedIndex and linkIndex,
thus resulting in failures when the linked containers are
referenced later on.

This fix updates the namedIndex and linkIndex during the
`docker rename` and fixes the issue.

An integration test has been added to cover the changes
in this fix.

This fix fixes #23973.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
2016-06-27 19:58:05 -07:00
Vincent Demeester
7e1ec8d2bd
No need for container.Lock if rename same name
During the renaming of a container, no need to call `container.Lock()`
if `oldName == newName`.

This is a follow-up from #23360 (commit 88d1ee6c11)

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2016-06-24 19:55:48 +02:00
Sainath Grandhi
3e8c16ef6d docker rename fix to address the issue of renaming with the same name issue #23319
Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.com>
2016-06-08 15:32:40 -07:00
Daniel Zhang
be072a8954 Embedded DNS problem after renaming container. Step2:change in docker/daemon side and add integration test
Signed-off-by: Daniel Zhang <jmzwcn@gmail.com>
2016-05-26 12:59:44 +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
Arnaud Porterie
70c5e96cb8 Merge pull request #19604 from Microsoft/jjh/testrename
Windows CI: Fix TestRename*
2016-01-26 19:03:31 -08:00
John Howard
f21fb2162e Windows CI: Fix TestRename*
Signed-off-by: John Howard <jhoward@microsoft.com>
2016-01-23 09:25:10 -08:00
Vincent Demeester
1d8ccc6ae7 Add the possibility to log event with specific attributes
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2016-01-17 12:14:01 +01:00
Brian Goff
0f9f99500c Build names and links at runtime
Don't rely on sqlite db for name registration and linking.
Instead register names and links when the daemon starts to an in-memory
store.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2016-01-07 14:10:42 -05:00
David Calavera
d7d512bb92 Rename Daemon.Get to Daemon.GetContainer.
This is more aligned with `Daemon.GetImage` and less confusing.

Signed-off-by: David Calavera <david.calavera@gmail.com>
2015-12-11 12:39:28 -05: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
David Calavera
ca5ede2d0a Decouple daemon and container to log events.
Create a supervisor interface to let the container monitor to emit events.

Signed-off-by: David Calavera <david.calavera@gmail.com>
2015-11-04 12:27:48 -05:00
Santhosh Manohar
8e0bbb2898 Add libnetwork call on daemon rename
Signed-off-by: Santhosh Manohar <santhosh@docker.com>
2015-10-23 16:26:24 -07:00
Tibor Vass
b08f071e18 Revert "Merge pull request #16228 from duglin/ContextualizeEvents"
Although having a request ID available throughout the codebase is very
valuable, the impact of requiring a Context as an argument to every
function in the codepath of an API request, is too significant and was
not properly understood at the time of the review.

Furthermore, mixing API-layer code with non-API-layer code makes the
latter usable only by API-layer code (one that has a notion of Context).

This reverts commit de41640435, reversing
changes made to 7daeecd42d.

Signed-off-by: Tibor Vass <tibor@docker.com>

Conflicts:
	api/server/container.go
	builder/internals.go
	daemon/container_unix.go
	daemon/create.go
2015-09-29 14:26:51 -04:00
Doug Davis
26b1064967 Add context.RequestID to event stream
This PR adds a "request ID" to each event generated, the 'docker events'
stream now looks like this:

```
2015-09-10T15:02:50.000000000-07:00 [reqid: c01e3534ddca] de7c5d4ca927253cf4e978ee9c4545161e406e9b5a14617efb52c658b249174a: (from ubuntu) create
```
Note the `[reqID: c01e3534ddca]` part, that's new.

Each HTTP request will generate its own unique ID. So, if you do a
`docker build` you'll see a series of events all with the same reqID.
This allow for log processing tools to determine which events are all related
to the same http request.

I didn't propigate the context to all possible funcs in the daemon,
I decided to just do the ones that needed it in order to get the reqID
into the events. I'd like to have people review this direction first, and
if we're ok with it then I'll make sure we're consistent about when
we pass around the context - IOW, make sure that all funcs at the same level
have a context passed in even if they don't call the log funcs - this will
ensure we're consistent w/o passing it around for all calls unnecessarily.

ping @icecrime @calavera @crosbymichael

Signed-off-by: Doug Davis <dug@us.ibm.com>
2015-09-24 11:56:37 -07:00
Doug Davis
0a734182eb Move more 'daemon' errors to the new error package
Signed-off-by: Doug Davis <dug@us.ibm.com>
2015-09-23 09:51:45 -07:00
Doug Davis
848792c42e Fix 'rename' error msg and error checking
`docker rename foo ''` would result in:
```
usage: docker rename OLD_NAME NEW_NAME
```
which is the old engine's way of return errors - yes that's in the
daemon code.  So I fixed that error msg to just be normal.

While doing that I noticed that using an empty string for the
source container name failed but didn't print any error message at all.
This is because we would generate a URL like: ../containers//rename/..
which would cause a 301 redirect to ../containers/rename/..
however the CLI code doesn't actually deal with 301's - it just ignores
them and returns back to the CLI code/caller.

Rather than changing the CLI to deal with 3xx error codes, which would
probably be a good thing to do in a follow-on PR, for this immediate
issue I just added a cli-side check for empty strings for both old and
new names. This way we catch it even before we hit the daemon.

API callers will get a 404, assuming they follow the 301, for the
case of the src being empty, and the new error msg when the destination
is empty - so we should be good now.

Add tests for both cases too.

Signed-off-by: Doug Davis <dug@us.ibm.com>
2015-09-18 11:12:22 -07:00
Morgan Bauer
abd72d4008
golint fixes for daemon/ package
- some method names were changed to have a 'Locking' suffix, as the
 downcased versions already existed, and the existing functions simply
 had locks around the already downcased version.
 - deleting unused functions
 - package comment
 - magic numbers replaced by golang constants
 - comments all over

Signed-off-by: Morgan Bauer <mbauer@us.ibm.com>
2015-08-27 22:07:42 -07:00
Doug Davis
8232312c1e Cleanup container LogEvent calls
Move some calls to container.LogEvent down lower so that there's
less of a chance of them being missed. Also add a few more events
that appear to have been missed.

Added testcases for new events: commit, copy, resize, attach, rename, top

Signed-off-by: Doug Davis <dug@us.ibm.com>
2015-06-01 12:39:28 -07:00
Hu Keping
49c4de4aeb Remove Job from rename
A part of ISSUE#12151-Remove engine.Job mechanism

Signed-off-by: Hu Keping <hukeping@huawei.com>
2015-04-10 01:52:55 +08:00
Antonio Murdaca
c79b9bab54 Remove engine.Status and replace it with standard go error
Signed-off-by: Antonio Murdaca <me@runcom.ninja>
2015-03-25 22:32:08 +01:00
Brian Goff
c5c72cf151 Persist container to disk after rename
Fixes #11315

After rename occured the graphdb was updated but the container struct
was never commited back to disk, so on daemon restart it loads the old
name again.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
2015-03-11 12:39:31 -07:00
Srini Brahmaroutu
caaae78247 Prefix / to the container name is ignored when container is renamed
Closes #10996

Signed-off-by: Srini Brahmaroutu <srbrahma@us.ibm.com>
2015-02-27 22:40:04 +00:00
Andrew C. Bodine
d25a65375c Closes #9311 Handles container id/name collisions against daemon functionalities according to #8069
Signed-off-by: Andrew C. Bodine <acbodine@us.ibm.com>
2015-01-21 17:11:31 -08:00
Jessica Frazelle
a92281637f Renaming a container with an invalid name should fail
Docker-DCO-1.1-Signed-off-by: Jessica Frazelle <jess@docker.com> (github: jfrazelle)
2015-01-14 12:54:23 -08:00
Srini Brahmaroutu
21a809d9ae rename a existing container
Closes #3036

Signed-off-by: Srini Brahmaroutu <srbrahma@us.ibm.com>
2015-01-13 03:27:17 +00:00