Remove string test for container conflict #3513
Conversation
It appears that the upstream changed the error text returned on container conflict. This caused Nomad to not be able to create a new condition when docker stopped the container without cleaning up the conainter manually.
|
Addresses #3419 and ZD-5338 |
client/driver/docker.go
Outdated
| // Adding a / infront of the container name since Docker returns the | ||
| // container names with a / pre-pended to the Nomad generated container names | ||
| containerName := "/" + config.Name | ||
| d.logger.Printf("[DEBUG] driver.docker: searching for container name %q to purge", containerName) |
There was a problem hiding this comment.
Can you remove to purge from this log line? I know you didn't add it, but it seems grossly misleading as if we find a running container we do not purge it.
client/driver/docker.go
Outdated
| if err != nil { | ||
| d.logger.Printf("[ERR] driver.docker: failed to purge container %s", container.ID) | ||
| return nil, recoverableErrTimeouts(fmt.Errorf("Failed to purge container %s: %s", container.ID, err)) | ||
| } else if err == nil { |
There was a problem hiding this comment.
Mind dropping the else if err == nil part? It's not necessary since the if err != nil bit returns.
I know this isn't your change, but I'd love to clean up some of this ugliness while we're in here.
|
This generally LGTM, please wait for @schmichael to see if this fixes the problem via the repro step of the linked issue before merging |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
It appears that the upstream changed the error text returned on container conflict. This caused Nomad to not be able to create a new condition when docker stopped the container without cleaning up the container manually.