Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cleaner): remove the usage of watch api #310

Merged
merged 1 commit into from
Apr 29, 2016
Merged

Conversation

kmala
Copy link
Contributor

@kmala kmala commented Apr 26, 2016

Summary of Changes

remove the usage of watch api and instead get the namespaces to know which apps are deleted because the watch api is flaky and also the channel gets closed too often kubernetes/kubernetes#8700

Issue(s) that this PR Closes

closes #287

Associated End To End Test PR(s)

deis/workflow-e2e#182

Associated Documentation PR(s)

Associated Design Document(s)

Testing Instructions

Please provide a detailed list for how to test the changes in this PR.

  1. Create a Deis Cluster
  2. deploy an app using git push
  3. check in the builder pod and the app should be present in the /home/git/ folder
  4. delete the app
  5. again check the builder pod and now it should not be present in the /home/git/ folder

Pull Request Hygiene TODOs

Please make sure the below checklist is complete.

  • Your pull request is concise and to the point (make another PR for refactoring nearby code)
  • Your commits are squashed into logical units of work
  • Your commits follow the commit style guidelines

@kmala kmala added this to the v2.0-beta4 milestone Apr 26, 2016
@kmala kmala self-assigned this Apr 26, 2016
@arschles
Copy link
Member

arschles commented Apr 28, 2016

Historical note: the original implementation of the cleaner used a similar polling approach but suffered from some concurrency issues. See #186 for the initial implementation and #209 for the changes to using the watch API. Added for context

@arschles
Copy link
Member

@kmala code LGTM (although needs rebase). does this need manual testing?

also, not blocking for this PR, but can you create an issue in deis/workflow-e2e to add an end-to-end test to test the cleaner (iirc there isn't one, but if you know of one, then ignore)

@smothiki smothiki added the LGTM2 label Apr 29, 2016
@kmala kmala merged commit 1743208 into deis:master Apr 29, 2016
@kmala kmala deleted the cleaner branch April 29, 2016 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High CPU usage on AWS cluster
4 participants