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 migration diffid atomic write #20372

Merged
merged 1 commit into from Feb 17, 2016

Conversation

Projects
None yet
8 participants
@tonistiigi
Copy link
Member

commented Feb 16, 2016

Fixes #20267

From the #20267 debug info it seems that empty diffid file was created during the calculation phase. This makes sure that if calculation is cancelled it can not leave a layer in a broken state. There is also added validation that we don't create broken image configurations but error out if a layer diffid is not a valid digest.

cc @dmcgowan @aaronlehmann

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

Fix migration diffid atomic write
Fixes #20267

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@@ -160,7 +160,12 @@ func calculateLayerChecksum(graphDir, id string, ls checksumCalculator) error {
return err
}

if err := ioutil.WriteFile(filepath.Join(graphDir, id, migrationDiffIDFileName), []byte(diffID), 0600); err != nil {
tmpFile := filepath.Join(graphDir, id, migrationDiffIDFileName+".tmp")

This comment has been minimized.

Copy link
@aaronlehmann

aaronlehmann Feb 16, 2016

Contributor

I wonder if we should use an extension like .migrate to make it clearer where a stray file came from.

This comment has been minimized.

Copy link
@tonistiigi

tonistiigi Feb 16, 2016

Author Member

All these files already have .migration- prefix

@aaronlehmann

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2016

LGTM

@tonistiigi tonistiigi added this to the 1.10.2 milestone Feb 16, 2016

@dmcgowan

This comment has been minimized.

Copy link
Member

commented Feb 16, 2016

LGTM

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Feb 17, 2016

LGTM

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Feb 17, 2016

WindowsTP4 https://jenkins.dockerproject.org/job/Docker-PRs-WoW-TP4/1138/console 😢

fatal: unable to access 'https://github.com/docker/docker.git/': transfer closed with outstanding read data remaining
21:52:38 + ec=128
21:52:38 + set +x
@cpuguy83

This comment has been minimized.

Copy link
Contributor

commented Feb 17, 2016

LGTM

calavera added a commit that referenced this pull request Feb 17, 2016

@calavera calavera merged commit 61aa025 into moby:master Feb 17, 2016

8 checks passed

docker/dco-signed All commits signed
Details
documentation success 1 tests run, 1 skipped, 0 failed.
Details
experimental Jenkins build Docker-PRs-experimental 15030 has succeeded
Details
gccgo Jenkins build Docker-PRs-gccgo 1942 has succeeded
Details
janky Jenkins build Docker-PRs 23818 has succeeded
Details
userns Jenkins build Docker-PRs-userns 6275 has succeeded
Details
win2lin Jenkins build Docker-PRs-Win2Lin 21805 has succeeded
Details
windowsTP4 Jenkins build Docker-PRs-WoW-TP4 1140 has succeeded
Details

@calavera calavera deleted the tonistiigi:fix-empty-diffid branch Feb 17, 2016

@tiborvass tiborvass referenced this pull request Feb 20, 2016

Merged

1.10.2 cherrypicks #20526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.