Skip to content

Commit f266f13

Browse files
thaJeztahthaJeztah
authored
Merge pull request #41636 from TBBle/37352-test-and-fix
Set 127GB default sandbox size for WCOW, and ensure storage-opts is honoured on all paths under WCOW and LCOW
2 parents b865beb + 1571e93 commit f266f13

7 files changed

Lines changed: 108 additions & 49 deletions

File tree

‎builder/dockerfile/internals.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"os"
1212
"path"
1313
"path/filepath"
14-
"runtime"
1514
"strings"
1615

1716
"github.com/docker/docker/api/types"
@@ -448,8 +447,7 @@ func (b *Builder) probeAndCreate(dispatchState *dispatchState, runConfig *contai
448447
func (b *Builder) create(runConfig *container.Config) (string, error) {
449448
logrus.Debugf("[BUILDER] Command to be executed: %v", runConfig.Cmd)
450449

451-
isWCOW := runtime.GOOS == "windows" && b.platform != nil && b.platform.OS == "windows"
452-
hostConfig := hostConfigFromOptions(b.options, isWCOW)
450+
hostConfig := hostConfigFromOptions(b.options)
453451
container, err := b.containerManager.Create(runConfig, hostConfig)
454452
if err != nil {
455453
return "", err
@@ -462,7 +460,7 @@ func (b *Builder) create(runConfig *container.Config) (string, error) {
462460
return container.ID, nil
463461
}
464462

465-
func hostConfigFromOptions(options *types.ImageBuildOptions, isWCOW bool) *container.HostConfig {
463+
func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConfig {
466464
resources := container.Resources{
467465
CgroupParent: options.CgroupParent,
468466
CPUShares: options.CPUShares,
@@ -485,16 +483,6 @@ func hostConfigFromOptions(options *types.ImageBuildOptions, isWCOW bool) *conta
485483
LogConfig: defaultLogConfig,
486484
ExtraHosts: options.ExtraHosts,
487485
}
488-
489-
// For WCOW, the default of 20GB hard-coded in the platform
490-
// is too small for builder scenarios where many users are
491-
// using RUN statements to install large amounts of data.
492-
// Use 127GB as that's the default size of a VHD in Hyper-V.
493-
if isWCOW {
494-
hc.StorageOpt = make(map[string]string)
495-
hc.StorageOpt["size"] = "127GB"
496-
}
497-
498486
return hc
499487
}
500488

‎daemon/create.go‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -187,23 +187,6 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr
187187

188188
ctr.HostConfig.StorageOpt = opts.params.HostConfig.StorageOpt
189189

190-
// Fixes: https://github.com/moby/moby/issues/34074 and
191-
// https://github.com/docker/for-win/issues/999.
192-
// Merge the daemon's storage options if they aren't already present. We only
193-
// do this on Windows as there's no effective sandbox size limit other than
194-
// physical on Linux.
195-
if isWindows {
196-
if ctr.HostConfig.StorageOpt == nil {
197-
ctr.HostConfig.StorageOpt = make(map[string]string)
198-
}
199-
for _, v := range daemon.configStore.GraphOptions {
200-
opt := strings.SplitN(v, "=", 2)
201-
if _, ok := ctr.HostConfig.StorageOpt[opt[0]]; !ok {
202-
ctr.HostConfig.StorageOpt[opt[0]] = opt[1]
203-
}
204-
}
205-
}
206-
207190
// Set RWLayer for container after mount labels have been set
208191
rwLayer, err := daemon.imageService.CreateLayer(ctr, setupInitLayer(daemon.idMapping))
209192
if err != nil {

‎daemon/graphdriver/lcow/lcow.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ type Driver struct {
124124
cachedScratchMutex sync.Mutex // Protects race conditions from multiple threads creating the cached scratch.
125125
options []string // Graphdriver options we are initialised with.
126126
globalMode bool // Indicates if running in an unsafe/global service VM mode.
127+
defaultSandboxSize uint64 // The default sandbox size to use if one is not specified
127128

128129
// NOTE: It is OK to use a cache here because Windows does not support
129130
// restoring containers when the daemon dies.
@@ -163,7 +164,8 @@ func InitDriver(dataRoot string, options []string, _, _ []idtools.IDMap) (graphd
163164
serviceVms: &serviceVMMap{
164165
svms: make(map[string]*serviceVMMapItem),
165166
},
166-
globalMode: false,
167+
globalMode: false,
168+
defaultSandboxSize: client.DefaultVhdxSizeGB,
167169
}
168170

169171
// Looks for relevant options
@@ -178,6 +180,16 @@ func InitDriver(dataRoot string, options []string, _, _ []idtools.IDMap) (graphd
178180
return nil, fmt.Errorf("%s failed to parse value for 'lcow.globalmode' - must be 'true' or 'false'", title)
179181
}
180182
break
183+
case "lcow.sandboxsize":
184+
var err error
185+
d.defaultSandboxSize, err = strconv.ParseUint(opt[1], 10, 32)
186+
if err != nil {
187+
return nil, fmt.Errorf("%s failed to parse value '%s' for 'lcow.sandboxsize'", title, v)
188+
}
189+
if d.defaultSandboxSize < client.DefaultVhdxSizeGB {
190+
return nil, fmt.Errorf("%s 'lcow.sandboxsize' option cannot be less than %d", title, client.DefaultVhdxSizeGB)
191+
}
192+
break
181193
}
182194
}
183195
}
@@ -517,7 +529,7 @@ func (d *Driver) CreateReadWrite(id, parent string, opts *graphdriver.CreateOpts
517529
}
518530

519531
// Look for an explicit sandbox size option.
520-
sandboxSize := uint64(client.DefaultVhdxSizeGB)
532+
sandboxSize := d.defaultSandboxSize
521533
for k, v := range opts.StorageOpt {
522534
switch strings.ToLower(k) {
523535
case "lcow.sandboxsize":

‎daemon/graphdriver/windows/windows.go‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,15 @@ import (
3838
"golang.org/x/sys/windows"
3939
)
4040

41-
// filterDriver is an HCSShim driver type for the Windows Filter driver.
42-
const filterDriver = 1
41+
const (
42+
// filterDriver is an HCSShim driver type for the Windows Filter driver.
43+
filterDriver = 1
44+
// For WCOW, the default of 20GB hard-coded in the platform
45+
// is too small for builder scenarios where many users are
46+
// using RUN or COPY statements to install large amounts of data.
47+
// Use 127GB as that's the default size of a VHD in Hyper-V.
48+
defaultSandboxSize = "127GB"
49+
)
4350

4451
var (
4552
// mutatedFiles is a list of files that are mutated by the import process
@@ -73,15 +80,20 @@ func (c *checker) IsMounted(path string) bool {
7380
return false
7481
}
7582

83+
type storageOptions struct {
84+
size uint64
85+
}
86+
7687
// Driver represents a windows graph driver.
7788
type Driver struct {
7889
// info stores the shim driver information
7990
info hcsshim.DriverInfo
8091
ctr *graphdriver.RefCounter
8192
// it is safe for windows to use a cache here because it does not support
8293
// restoring containers when the daemon dies.
83-
cacheMu sync.Mutex
84-
cache map[string]string
94+
cacheMu sync.Mutex
95+
cache map[string]string
96+
defaultStorageOpts *storageOptions
8597
}
8698

8799
// InitFilter returns a new Windows storage filter driver.
@@ -100,13 +112,27 @@ func InitFilter(home string, options []string, uidMaps, gidMaps []idtools.IDMap)
100112
return nil, fmt.Errorf("windowsfilter failed to create '%s': %v", home, err)
101113
}
102114

115+
storageOpt := make(map[string]string)
116+
storageOpt["size"] = defaultSandboxSize
117+
118+
for _, v := range options {
119+
opt := strings.SplitN(v, "=", 2)
120+
storageOpt[strings.ToLower(opt[0])] = opt[1]
121+
}
122+
123+
storageOptions, err := parseStorageOpt(storageOpt)
124+
if err != nil {
125+
return nil, fmt.Errorf("windowsfilter failed to parse default storage options - %s", err)
126+
}
127+
103128
d := &Driver{
104129
info: hcsshim.DriverInfo{
105130
HomeDir: home,
106131
Flavour: filterDriver,
107132
},
108-
cache: make(map[string]string),
109-
ctr: graphdriver.NewRefCounter(&checker{}),
133+
cache: make(map[string]string),
134+
ctr: graphdriver.NewRefCounter(&checker{}),
135+
defaultStorageOpts: storageOptions,
110136
}
111137
return d, nil
112138
}
@@ -231,8 +257,13 @@ func (d *Driver) create(id, parent, mountLabel string, readOnly bool, storageOpt
231257
return fmt.Errorf("Failed to parse storage options - %s", err)
232258
}
233259

260+
sandboxSize := d.defaultStorageOpts.size
234261
if storageOptions.size != 0 {
235-
if err := hcsshim.ExpandSandboxSize(d.info, id, storageOptions.size); err != nil {
262+
sandboxSize = storageOptions.size
263+
}
264+
265+
if sandboxSize != 0 {
266+
if err := hcsshim.ExpandSandboxSize(d.info, id, sandboxSize); err != nil {
236267
return err
237268
}
238269
}
@@ -935,10 +966,6 @@ func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) {
935966
return &fileGetCloserWithBackupPrivileges{d.dir(id)}, nil
936967
}
937968

938-
type storageOptions struct {
939-
size uint64
940-
}
941-
942969
func parseStorageOpt(storageOpt map[string]string) (*storageOptions, error) {
943970
options := storageOptions{}
944971

‎hack/make.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ Function Run-IntegrationTests() {
363363
$pinfo.FileName = "gotestsum.exe"
364364
$pinfo.WorkingDirectory = "$($PWD.Path)"
365365
$pinfo.UseShellExecute = $false
366-
$pinfo.Arguments = "--format=standard-verbose --jsonfile=$jsonFilePath --junitfile=$xmlFilePath -- $env:INTEGRATION_TESTFLAGS"
366+
$pinfo.Arguments = "--format=standard-verbose --jsonfile=$jsonFilePath --junitfile=$xmlFilePath -- -test.timeout=60m $env:INTEGRATION_TESTFLAGS"
367367
$p = New-Object System.Diagnostics.Process
368368
$p.StartInfo = $pinfo
369369
$p.Start() | Out-Null

‎integration/build/build_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,55 @@ RUN for g in $(seq 0 8); do dd if=/dev/urandom of=rnd bs=1K count=1 seek=$((1024
523523
assert.Check(t, is.Contains(out.String(), "Successfully built"))
524524
}
525525

526+
func TestBuildWCOWSandboxSize(t *testing.T) {
527+
skip.If(t, testEnv.DaemonInfo.OSType != "windows", "only Windows has sandbox size control")
528+
ctx := context.TODO()
529+
defer setupTest(t)()
530+
531+
dockerfile := `FROM busybox AS intermediate
532+
WORKDIR C:\\stuff
533+
# Create and delete a 21GB file
534+
RUN fsutil file createnew C:\\stuff\\bigfile_0.txt 22548578304 && del bigfile_0.txt
535+
# Create three 7GB files
536+
RUN fsutil file createnew C:\\stuff\\bigfile_1.txt 7516192768
537+
RUN fsutil file createnew C:\\stuff\\bigfile_2.txt 7516192768
538+
RUN fsutil file createnew C:\\stuff\\bigfile_3.txt 7516192768
539+
# Copy that 21GB of data out into a new target
540+
FROM busybox
541+
COPY --from=intermediate C:\\stuff C:\\stuff
542+
`
543+
544+
buf := bytes.NewBuffer(nil)
545+
w := tar.NewWriter(buf)
546+
writeTarRecord(t, w, "Dockerfile", dockerfile)
547+
err := w.Close()
548+
assert.NilError(t, err)
549+
550+
apiclient := testEnv.APIClient()
551+
resp, err := apiclient.ImageBuild(ctx,
552+
buf,
553+
types.ImageBuildOptions{
554+
Remove: true,
555+
ForceRemove: true,
556+
})
557+
558+
out := bytes.NewBuffer(nil)
559+
assert.NilError(t, err)
560+
_, err = io.Copy(out, resp.Body)
561+
resp.Body.Close()
562+
assert.NilError(t, err)
563+
// The test passes if either:
564+
// - the image build succeeded; or
565+
// - The "COPY --from=intermediate" step ran out of space during re-exec'd writing of the transport layer information to hcsshim's temp directory
566+
// The latter case means we finished the COPY operation, so the sandbox must have been larger than 20GB, which was the test,
567+
// and _then_ ran out of space on the host during `importLayer` in the WindowsFilter graph driver, while committing the layer.
568+
// See https://github.com/moby/moby/pull/41636#issuecomment-723038517 for more details on the operations being done here.
569+
// Specifically, this happens on the Docker Jenkins CI Windows-RS5 build nodes.
570+
// The two parts of the acceptable-failure case are on different lines, so we need two regexp checks.
571+
assert.Check(t, is.Regexp("Successfully built|COPY --from=intermediate", out.String()))
572+
assert.Check(t, is.Regexp("Successfully built|re-exec error: exit status 1: output: write.*daemon\\\\\\\\tmp\\\\\\\\hcs.*bigfile_[1-3].txt: There is not enough space on the disk.", out.String()))
573+
}
574+
526575
func TestBuildWithEmptyDockerfile(t *testing.T) {
527576
skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "broken in earlier versions")
528577
ctx := context.TODO()

‎integration/container/mounts_linux_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,15 +285,15 @@ func TestContainerVolumesMountedAsShared(t *testing.T) {
285285

286286
// Convert this directory into a shared mount point so that we do
287287
// not rely on propagation properties of parent mount.
288-
if err := mount.Mount(tmpDir1.Path(), tmpDir1.Path(), "none", "bind,private"); err != nil {
288+
if err := mount.MakePrivate(tmpDir1.Path()); err != nil {
289289
t.Fatal(err)
290290
}
291291
defer func() {
292292
if err := mount.Unmount(tmpDir1.Path()); err != nil {
293293
t.Fatal(err)
294294
}
295295
}()
296-
if err := mount.Mount("none", tmpDir1.Path(), "none", "shared"); err != nil {
296+
if err := mount.MakeShared(tmpDir1.Path()); err != nil {
297297
t.Fatal(err)
298298
}
299299

@@ -342,15 +342,15 @@ func TestContainerVolumesMountedAsSlave(t *testing.T) {
342342

343343
// Convert this directory into a shared mount point so that we do
344344
// not rely on propagation properties of parent mount.
345-
if err := mount.Mount(tmpDir1.Path(), tmpDir1.Path(), "none", "bind,private"); err != nil {
345+
if err := mount.MakePrivate(tmpDir1.Path()); err != nil {
346346
t.Fatal(err)
347347
}
348348
defer func() {
349349
if err := mount.Unmount(tmpDir1.Path()); err != nil {
350350
t.Fatal(err)
351351
}
352352
}()
353-
if err := mount.Mount("none", tmpDir1.Path(), "none", "shared"); err != nil {
353+
if err := mount.MakeShared(tmpDir1.Path()); err != nil {
354354
t.Fatal(err)
355355
}
356356

0 commit comments

Comments
 (0)