Skip to content

Commit b1b8523

Browse files
authored
campaigns: fix cidfile lifecycle on Windows (#368)
Fixes #367.
1 parent 58838e8 commit b1b8523

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ All notable changes to `src-cli` are documented in this file.
1818
### Fixed
1919

2020
- Campaign steps run in a container that does not run as root could fail on systems that do not map the running user ID to the container, most notably desktop Linux. This has been fixed: temporary files and workspaces mounted into the container now have sufficient permissions to allow the container user to execute the step. [#366](https://github.com/sourcegraph/src-cli/pull/366)
21+
- Executing campaigns on Windows would fail due to obscure `--cidfile` errors: namely, the temporary cidfile would not be removed before `docker run` was invoked. This has been fixed. [#368](https://github.com/sourcegraph/src-cli/pull/368)
2122

2223
## 3.21.5
2324

internal/campaigns/run_steps.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,24 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor
6868
stepContext.PreviousStep = results[i-1]
6969
}
7070

71+
// Find a location that we can use for a cidfile, which will contain the
72+
// container ID that is used below. We can then use this to remove the
73+
// container on a successful run, rather than leaving it dangling.
7174
cidFile, err := ioutil.TempFile(tempDir, repo.Slug()+"-container-id")
7275
if err != nil {
7376
return nil, errors.Wrap(err, "Creating a CID file failed")
7477
}
75-
_ = os.Remove(cidFile.Name()) // docker exits if this file exists upon `docker run` starting
78+
79+
// However, Docker will fail if the cidfile actually exists, so we need
80+
// to remove it. Because Windows can't remove open files, we'll first
81+
// close it, even though that's unnecessary elsewhere.
82+
cidFile.Close()
83+
if err = os.Remove(cidFile.Name()); err != nil {
84+
return nil, errors.Wrap(err, "removing cidfile")
85+
}
86+
87+
// Since we went to all that effort, we can now defer a function that
88+
// uses the cidfile to clean up after this function is done.
7689
defer func() {
7790
cid, err := ioutil.ReadFile(cidFile.Name())
7891
_ = os.Remove(cidFile.Name())

0 commit comments

Comments
 (0)