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

Add default value "3m" to image_delay, making it consistent with docs. #6078

Conversation

zhiguangwang
Copy link
Contributor

Currently, image_delay defaults to 0, causing nomad client to delete docker images the moment they become unused.

This is not the expected behavior according to docs:

https://www.nomadproject.io/docs/drivers/docker.html#image_delay

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 6, 2019

CLA assistant check
All committers have signed the CLA.

@langmartin langmartin self-requested a review August 16, 2019 18:23
Copy link
Contributor

@langmartin langmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! It does match the documentation. I'm going to ask for one more confirmation, but it looks good to me.

@langmartin langmartin requested a review from notnoop August 30, 2019 23:44
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I think i've been hit by nomad deleting images prematurely without fully understanding why. Thanks for catching this.

Copy link
Contributor

@langmartin langmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change just needs to be rebased on master in order for the tests to pass

@zhiguangwang zhiguangwang force-pushed the fix-docker-image-delay-default-value branch from fb2ff8b to e7eede5 Compare September 2, 2019 08:42
@zhiguangwang
Copy link
Contributor Author

I think this change just needs to be rebased on master in order for the tests to pass

Done :)

Copy link
Contributor

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - This was actually catching me a bunch last week but I didn't get a chance to debug

@endocrimes endocrimes merged commit 9a44545 into hashicorp:master Sep 2, 2019
@github-actions
Copy link

github-actions bot commented Feb 1, 2023

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants