deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

feat(lock): change cleaner code to listen to k8s namespsce events #209

Closed smothiki closed 8 years ago

smothiki commented 8 years ago

removes locking completely for cleaner. Deletes an App folder asap when there is a namespace deletion

arschles commented 8 years ago

@smothiki tests now fail:

ENG000656:builder aaronschlesinger$ make test
docker run --rm -e GO15VENDOREXPERIMENT=1 -v /Users/aaronschlesinger/gocode/src/github.com/deis/builder:/go/src/github.com/deis/builder -w /go/src/github.com/deis/builder quay.io/deis/go-dev:0.8.0 sh -c 'go test $(glide nv)'
?       github.com/deis/builder/pkg [no test files]
ok      github.com/deis/builder/pkg/cleaner 0.016s
?       github.com/deis/builder/pkg/conf    [no test files]
ok      github.com/deis/builder/pkg/controller  0.003s
ok      github.com/deis/builder/pkg/env 0.002s
?       github.com/deis/builder/pkg/git [no test files]
ok      github.com/deis/builder/pkg/gitreceive  0.005s
ok      github.com/deis/builder/pkg/gitreceive/git  0.002s
ok      github.com/deis/builder/pkg/gitreceive/storage  0.004s
ok      github.com/deis/builder/pkg/healthsrv   0.006s
?       github.com/deis/builder/pkg/k8s [no test files]
[info] 2016/02/29 23:00:03 Listening on 127.0.0.1:2244
[info] 2016/02/29 23:00:03 Accepting new connections.
[info] 2016/02/29 23:00:03 Checking closer.
[info] 2016/02/29 23:00:03 Checking closer.
[info] 2016/02/29 23:00:03 Accepted connection.
[info] 2016/02/29 23:00:03 Channel type: session
Key='HELLO', Value='world'
[info] 2016/02/29 23:00:03 PING
[info] 2016/02/29 23:00:03 Channel type: session
[warning] 2016/02/29 23:00:03 Illegal command is 'illegal'
[info] 2016/02/29 23:00:03 Listening on 127.0.0.1:2245
[info] 2016/02/29 23:00:03 Accepting new connections.
[info] 2016/02/29 23:00:03 Checking closer.
[info] 2016/02/29 23:00:03 Checking closer.
[info] 2016/02/29 23:00:03 Accepted connection.
[info] 2016/02/29 23:00:04 Channel type: session
[warning] 2016/02/29 23:00:04 Expected two-part command.
[info] 2016/02/29 23:00:04 Channel type: session
[info] 2016/02/29 23:00:06 Channel type: session
[error] 2016/02/29 23:00:06 Another git push is ongoing
[info] 2016/02/29 23:00:06 Listening on 127.0.0.1:2246
[info] 2016/02/29 23:00:06 Accepting new connections.
[info] 2016/02/29 23:00:06 Checking closer.
[info] 2016/02/29 23:00:06 Checking closer.
[info] 2016/02/29 23:00:06 Accepted connection.
[info] 2016/02/29 23:00:06 Channel type: session
[warning] 2016/02/29 23:00:06 Expected two-part command.
[info] 2016/02/29 23:00:06 Channel type: session
[error] 2016/02/29 23:00:06 Another git push is ongoing
--- FAIL: TestDelete (0.21s)
    server_test.go:207: expected no error, got ssh: command git-upload-pack /demo.git failed
FAIL
FAIL    github.com/deis/builder/pkg/sshd    3.638s
?       github.com/deis/builder [no test files]
make: *** [test] Error 1
arschles commented 8 years ago

@smothiki just tested. A concurrent git push to the same app results in this:

ENG000656:example-go aaronschlesinger$ git push deis master
exec request failed on channel 0
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Otherwise, deleting an app, re-creating it, and pushing a different repo to the same app name seems to work.

arschles commented 8 years ago

Also, CI will say this in a few, but tests don't compile anymore:

ENG000656:builder aaronschlesinger$ make test
docker run --rm -e GO15VENDOREXPERIMENT=1 -v /Users/aaronschlesinger/gocode/src/github.com/deis/builder:/go/src/github.com/deis/builder -w /go/src/github.com/deis/builder quay.io/deis/go-dev:0.8.0 sh -c 'go test $(glide nv)'
?       github.com/deis/builder/pkg [no test files]
?       github.com/deis/builder/pkg/cleaner [no test files]
?       github.com/deis/builder/pkg/conf    [no test files]
ok      github.com/deis/builder/pkg/controller  0.003s
ok      github.com/deis/builder/pkg/env 0.002s
?       github.com/deis/builder/pkg/git [no test files]
ok      github.com/deis/builder/pkg/gitreceive  0.007s
ok      github.com/deis/builder/pkg/gitreceive/git  0.002s
ok      github.com/deis/builder/pkg/gitreceive/storage  0.003s
ok      github.com/deis/builder/pkg/healthsrv   0.004s
?       github.com/deis/builder/pkg/k8s [no test files]
# github.com/deis/builder/pkg/sshd
pkg/sshd/server_test.go:273: undefined: cleaner.NewRef
pkg/sshd/server_test.go:275: too many arguments in call to Serve
FAIL    github.com/deis/builder/pkg/sshd [build failed]
?       github.com/deis/builder [no test files]
make: *** [test] Error 2
jchauncey commented 8 years ago

Ok take 2:

Got the same error as above -

╰─± git push deis master
exec request failed on channel 0
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
smothiki commented 8 years ago

this issue is already present in the master branch . As we have avoided global lock for each exec req we haven;t seen this till now https://github.com/deis/builder/issues/217

arschles commented 8 years ago

@smothiki ok, thanks. that being said, this PR looks ok to merge. I'm LGTMing

arschles commented 8 years ago

@smothiki I forgot about my previous comment about writing tests for the cleaner, sorry. removing my LGTM for now. please let me know when tests are done and I'll re-review

smothiki commented 8 years ago

refer https://github.com/deis/builder/issues/228

arschles commented 8 years ago

@smothiki does this need another manual test since the last one?

smothiki commented 8 years ago

This is working no need of testing Just tested the behavior

mboersma commented 8 years ago

Code looks good on first pass, testing it in k8s now.

mboersma commented 8 years ago

This works great. I had three simultaneous pushes going from a combination of Dockerfile and Procfile apps, and nothing ever blocked or errored. A shell inside the builder container shows everything was cleaned up immediately under /home/git. I kept creating, pushing, deleting simultaneously but wasn't able to cause a failure.

Nice work!