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

Ignore containers #45

Merged
merged 19 commits into from
Nov 3, 2018
Merged

Ignore containers #45

merged 19 commits into from
Nov 3, 2018

Conversation

tlkamp
Copy link
Contributor

@tlkamp tlkamp commented Oct 30, 2018

Adds the ability to ignore select running containers.

Closes #35

@codecov-io
Copy link

codecov-io commented Oct 30, 2018

Codecov Report

Merging #45 into master will increase coverage by 3.57%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #45      +/-   ##
========================================
+ Coverage   96.42%   100%   +3.57%     
========================================
  Files           7      7              
  Lines         140    145       +5     
========================================
+ Hits          135    145      +10     
+ Misses          5      0       -5
Impacted Files Coverage Δ
ouroboros/cli.py 100% <100%> (ø) ⬆️
ouroboros/main.py 100% <100%> (+13.33%) ⬆️
ouroboros/image.py 100% <100%> (ø) ⬆️
ouroboros/container.py 100% <100%> (+2.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33f92b1...dc23134. Read the comment docs.

@tlkamp tlkamp changed the title Ignore containers WIP: Ignore containers Oct 30, 2018
@tlkamp
Copy link
Contributor Author

tlkamp commented Oct 30, 2018

@circa10a How do you think we should handle a situation in which a user does the following:

$ ouroboros --monitor test1 test2 --ignore test1

Should we throw an error and refuse to do anything, or should one flag take precedence over the other?

@circa10a
Copy link
Member

circa10a commented Oct 30, 2018

@circa10a How do you think we should handle a situation in which a user does the following:

$ ouroboros --monitor test1 test2 --ignore test1

Should we throw an error and refuse to do anything, or should one flag take precedence over the other?

I think an error should be thrown for a couple of reasons:

  • Logging to display what the problem is
  • I wouldn't want something to cancel out and proceed in case the intention is for test1 to absolutely NOT be upgraded

The only issue I have with the reasons above is I would hate for the application not to start because of user error, but I think it's more vital not to update a potentially import container...

@tlkamp
Copy link
Contributor Author

tlkamp commented Oct 30, 2018

So what if we do this:

  • Ouroboros still starts, but --ignore takes precedence
  • Ouroboros logs that it is ignoring container because it was specified in both --monitor and --ignore

@circa10a
Copy link
Member

circa10a commented Oct 30, 2018

So what if we do this:

  • Ouroboros still starts, but --ignore takes precedence
  • Ouroboros logs that it is ignoring container because it was specified in both --monitor and --ignore

I like it 😄

@circa10a
Copy link
Member

TIL intersection!

Ready to merge?

@tlkamp
Copy link
Contributor Author

tlkamp commented Oct 31, 2018

Nope still WIP

@tlkamp
Copy link
Contributor Author

tlkamp commented Nov 3, 2018

@circa10a Gonna update the integration test and then it'll be ready to merge

@tlkamp tlkamp changed the title WIP: Ignore containers Ignore containers Nov 3, 2018
@tlkamp tlkamp requested a review from circa10a November 3, 2018 21:13
@tlkamp
Copy link
Contributor Author

tlkamp commented Nov 3, 2018

@circa10a It's ready when you are my d00d

@tlkamp
Copy link
Contributor Author

tlkamp commented Nov 3, 2018

$ docker pull busybox:1.28
1.28: Pulling from library/busybox
07a152489297: Pull complete
Digest: sha256:141c253bc4c3fd0a201d32dc1f493bcf3fff003b6df416dea4f41046e0f37d47
Status: Downloaded newer image for busybox:1.28

$ docker run -d --name bb busybox:1.28 tail -f /dev/null
b44a2839e91141468c223115f7bf8e2f1f00afde22b56a49a8731ced6933c173

$ docker run -d --name ignore busybox:1.28 tail -f /dev/null
70dd2426b27b12c5eebba8b3cbabab26236204f7791429c079cc395ee08491f3

$ docker ps
CONTAINER ID        IMAGE               COMMAND               CREATED             STATUS              PORTS               NAMES
70dd2426b27b        busybox:1.28        "tail -f /dev/null"   2 seconds ago       Up 2 seconds                            ignore
b44a2839e911        busybox:1.28        "tail -f /dev/null"   25 seconds ago      Up 24 seconds                           bb

$ ouroboros --interval 5 --monitor bb ignore --ignore ignore --runonce --cleanup
[INFO] 2018-11-03 16:11:27 Running job Every 5 seconds do main(args=Namespace(cleanup=True, ignore=['ignore'], interval=5, loglevel='info', monitor=['bb', 'ignore'], run_once=True, url='unix://var/run/docker.sock'), api_client=<docker.api.client.APIClient object at 0x100e8b320>) (last run: [never], next run: 2018-11-03 16:11:27)
[INFO] 2018-11-03 16:11:27 Ignoring container(s): ignore
[INFO] 2018-11-03 16:11:29 bb will be updated
[INFO] 2018-11-03 16:11:40 Removing image: busybox:1.28
[CRITICAL] 2018-11-03 16:11:40 Could not clean up image: busybox:1.28, reason:
409 Client Error: Conflict ("conflict: unable to delete 8c811b4aec35 (cannot be forced) - image is being used by running container 70dd2426b27b")
[INFO] 2018-11-03 16:11:40 1 container(s) updated

$ docker ps
CONTAINER ID        IMAGE               COMMAND               CREATED             STATUS              PORTS               NAMES
a462e84a78d9        busybox:latest      "tail -f /dev/null"   23 seconds ago      Up 21 seconds                           bb
70dd2426b27b        busybox:1.28        "tail -f /dev/null"   2 minutes ago       Up 2 minutes                            ignore

@circa10a circa10a merged commit d9dd230 into master Nov 3, 2018
@circa10a circa10a deleted the ignore-containers branch November 6, 2018 23:51
@dirtycajunrice dirtycajunrice added the enhancement New feature or request label Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants