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

Properly handle containers which pre-date the resolv.conf update feature #10018

Merged
merged 1 commit into from
Jan 11, 2015

Conversation

estesp
Copy link
Contributor

@estesp estesp commented Jan 10, 2015

This fixes the container start issue for containers which were started
on a daemon prior to the resolv.conf updater PR. The update code will
now safely ignore these containers (given they don't have a sha256 hash
to compare against) and will not attempt to update the resolv.conf
through their lifetime.

Docker-DCO-1.1-Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com (github: estesp)

// if the user has modified the resolv.conf since container start time, safer
// to just never update the container's resolv.conf during it's lifetime which
// we can control by setting hashBytes to an empty string
hashBytes = []byte("")
Copy link
Contributor

Choose a reason for hiding this comment

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

yayyy comments :D

@jessfraz
Copy link
Contributor

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 10, 2015

@estesp Thanks, also I think worth to mention this in docs about this feature. Just to have place where to point when there will be issue about this :)

// to just never update the container's resolv.conf during it's lifetime which
// we can control by setting hashBytes to an empty string
hashBytes = []byte("")
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

As usual I don't like else :) You can reverse condition and get rid of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch :)

This fixes the container start issue for containers which were started
on a daemon prior to the resolv.conf updater PR. The update code will
now safely ignore these containers (given they don't have a sha256 hash
to compare against) and will not attempt to update the resolv.conf
through their lifetime.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
@estesp
Copy link
Contributor Author

estesp commented Jan 10, 2015

@LK4D4 forgot to add a comment last night after submitting a note to the docs and making the re-order to get rid of else .. PTAL, thx!

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 11, 2015

@estesp Thank you!
LGTM

LK4D4 added a commit that referenced this pull request Jan 11, 2015
Properly handle containers which pre-date the resolv.conf update feature
@LK4D4 LK4D4 merged commit de97839 into moby:master Jan 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants