Skip to content
This repository was archived by the owner on Jan 25, 2022. It is now read-only.

Commit d180354

Browse files
glynglyn
committed
Honour Docker image environment metadata
Invert the order in which environment variables are accumulated in Docker layers so that the upper layers take precedence over lower layers. A $PATH in the environment now overrides wshd's default. Represent environments using process.Env instead of []string. This change brings some additional error checking to the format of the environment variables. Duplicate environment variables are removed as soon as they occur rather than being passed down and removed later causing confusion. [finishes #86540096] Signed-off-by: Chris Brown <[email protected]>
1 parent 7e24eee commit d180354

25 files changed

+187
-154
lines changed

Diff for: ‎fences/fake_fences/fake_fences.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ package fake_fences
22

33
import (
44
"encoding/json"
5-
"fmt"
65
"net"
76

87
"github.com/cloudfoundry-incubator/garden"
98
"github.com/cloudfoundry-incubator/garden-linux/fences"
109
"github.com/cloudfoundry-incubator/garden-linux/old/sysconfig"
10+
"github.com/cloudfoundry-incubator/garden-linux/process"
1111
)
1212

1313
type FakeFences struct {
@@ -83,8 +83,8 @@ func (f *FakeAllocation) Dismantle() error {
8383
func (f *FakeAllocation) Info(i *garden.ContainerInfo) {
8484
}
8585

86-
func (f *FakeAllocation) ConfigureProcess(env *[]string) error {
87-
*env = append(*env, fmt.Sprintf("fake_fences_env=%s", f.Subnet))
86+
func (f *FakeAllocation) ConfigureProcess(env process.Env) error {
87+
env["fake_fences_env"] = f.Subnet
8888
return nil
8989
}
9090

Diff for: ‎fences/netfence/fence.go

+12-10
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import (
55
"encoding/json"
66
"fmt"
77
"net"
8+
"strconv"
89
"strings"
910

1011
"github.com/cloudfoundry-incubator/garden"
1112
"github.com/cloudfoundry-incubator/garden-linux/fences"
1213
"github.com/cloudfoundry-incubator/garden-linux/fences/netfence/network/subnets"
1314
"github.com/cloudfoundry-incubator/garden-linux/old/sysconfig"
15+
"github.com/cloudfoundry-incubator/garden-linux/process"
1416
"github.com/pivotal-golang/lager"
1517
)
1618

@@ -41,7 +43,7 @@ type FlatFence struct {
4143
// address is statically allocated. In all cases, if an IP cannot be allocated which
4244
// meets the requirements, an error is returned.
4345
//
44-
// The given allocation is stored in the returned fence.
46+
// The given fence builder is stored in the returned fence.
4547
func (f *fenceBuilder) Build(spec string, sysconfig *sysconfig.Config, containerID string) (fences.Fence, error) {
4648
var ipSelector subnets.IPSelector = subnets.DynamicIPSelector
4749
var subnetSelector subnets.SubnetSelector = subnets.DynamicSubnetSelector
@@ -171,17 +173,17 @@ func (a *Fence) MarshalJSON() ([]byte, error) {
171173
return json.Marshal(ff)
172174
}
173175

174-
func (a *Fence) ConfigureProcess(env *[]string) error {
176+
func (a *Fence) ConfigureProcess(env process.Env) error {
175177
suff, _ := a.IPNet.Mask.Size()
176178

177-
*env = append(*env, fmt.Sprintf("network_host_ip=%s", subnets.GatewayIP(a.IPNet)),
178-
fmt.Sprintf("network_container_ip=%s", a.containerIP),
179-
fmt.Sprintf("network_cidr_suffix=%d", suff),
180-
fmt.Sprintf("container_iface_mtu=%d", a.fenceBldr.mtu),
181-
fmt.Sprintf("subnet_shareable=%v", a.subnetShareable),
182-
fmt.Sprintf("network_cidr=%s", a.IPNet.String()),
183-
fmt.Sprintf("external_ip=%s", a.fenceBldr.externalIP.String()),
184-
fmt.Sprintf("network_ip_hex=%s", hexIP(a.IPNet.IP))) // suitable for short bridge interface names
179+
env["network_host_ip"] = subnets.GatewayIP(a.IPNet).String()
180+
env["network_container_ip"] = a.containerIP.String()
181+
env["network_cidr_suffix"] = strconv.Itoa(suff)
182+
env["container_iface_mtu"] = strconv.FormatUint(uint64(a.fenceBldr.mtu), 10)
183+
env["subnet_shareable"] = strconv.FormatBool(a.subnetShareable)
184+
env["network_cidr"] = a.IPNet.String()
185+
env["external_ip"] = a.fenceBldr.externalIP.String()
186+
env["network_ip_hex"] = hexIP(a.IPNet.IP) // suitable for short bridge interface names
185187

186188
return nil
187189
}

Diff for: ‎fences/netfence/fence_test.go

+10-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/cloudfoundry-incubator/garden"
1010
"github.com/cloudfoundry-incubator/garden-linux/fences/netfence/network/subnets"
1111
"github.com/cloudfoundry-incubator/garden-linux/old/sysconfig"
12+
"github.com/cloudfoundry-incubator/garden-linux/process"
1213
. "github.com/onsi/ginkgo"
1314
. "github.com/onsi/gomega"
1415
"github.com/pivotal-golang/lager"
@@ -320,7 +321,7 @@ var _ = Describe("Fence", func() {
320321
Describe("ConfigureProcess", func() {
321322
Context("With a /29", func() {
322323
var (
323-
env []string
324+
env process.Env
324325
)
325326

326327
BeforeEach(func() {
@@ -329,33 +330,33 @@ var _ = Describe("Fence", func() {
329330

330331
fence.mtu = 123
331332

332-
env = []string{"foo", "bar"}
333+
env = process.Env{"foo": "bar"}
333334
allocation := &Fence{ipn, net.ParseIP("4.5.6.1"), "", "host", false, "bridge", fence, lagertest.NewTestLogger("allocation")}
334-
allocation.ConfigureProcess(&env)
335+
allocation.ConfigureProcess(env)
335336
})
336337

337338
It("configures with the correct network_cidr", func() {
338-
Ω(env).Should(ContainElement("network_cidr=4.5.6.0/29"))
339+
Ω(env.Array()).Should(ContainElement("network_cidr=4.5.6.0/29"))
339340
})
340341

341342
It("configures with the correct gateway ip", func() {
342-
Ω(env).Should(ContainElement("network_host_ip=4.5.6.6"))
343+
Ω(env.Array()).Should(ContainElement("network_host_ip=4.5.6.6"))
343344
})
344345

345346
It("configures with the correct container ip", func() {
346-
Ω(env).Should(ContainElement("network_container_ip=4.5.6.1"))
347+
Ω(env.Array()).Should(ContainElement("network_container_ip=4.5.6.1"))
347348
})
348349

349350
It("configures with the correct cidr suffix", func() {
350-
Ω(env).Should(ContainElement("network_cidr_suffix=29"))
351+
Ω(env.Array()).Should(ContainElement("network_cidr_suffix=29"))
351352
})
352353

353354
It("configures with the correct MTU size", func() {
354-
Ω(env).Should(ContainElement("container_iface_mtu=123"))
355+
Ω(env.Array()).Should(ContainElement("container_iface_mtu=123"))
355356
})
356357

357358
It("configures with the correct external IP", func() {
358-
Ω(env).Should(ContainElement("external_ip=1.2.3.4"))
359+
Ω(env.Array()).Should(ContainElement("external_ip=1.2.3.4"))
359360
})
360361
})
361362
})

Diff for: ‎fences/registry.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/cloudfoundry-incubator/garden"
99
"github.com/cloudfoundry-incubator/garden-linux/old/sysconfig"
10+
"github.com/cloudfoundry-incubator/garden-linux/process"
1011
)
1112

1213
var ErrNoFencesRegistered = errors.New("no fences have been registered")
@@ -32,7 +33,7 @@ type Builder interface {
3233

3334
type Fence interface {
3435
json.Marshaler
35-
ConfigureProcess(*[]string) error
36+
ConfigureProcess(process.Env) error
3637
Dismantle() error
3738
Info(*garden.ContainerInfo)
3839
String() string

Diff for: ‎fences/registry_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/cloudfoundry-incubator/garden"
99
. "github.com/cloudfoundry-incubator/garden-linux/fences"
1010
"github.com/cloudfoundry-incubator/garden-linux/old/sysconfig"
11+
"github.com/cloudfoundry-incubator/garden-linux/process"
1112

1213
. "github.com/onsi/ginkgo"
1314
. "github.com/onsi/gomega"
@@ -300,7 +301,7 @@ func (a *FakeAllocation) MarshalJSON() ([]byte, error) {
300301
return nil, nil
301302
}
302303

303-
func (a *FakeAllocation) ConfigureProcess(env *[]string) error {
304+
func (a *FakeAllocation) ConfigureProcess(env process.Env) error {
304305
return nil
305306
}
306307

Diff for: ‎integration/lifecycle/lifecycle_test.go

+11-8
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,21 @@ var _ = Describe("Creating a container", func() {
134134
})
135135
})
136136

137-
FContext("when the docker image specifies $HOME and $PATH", func() {
137+
Context("when the docker image specifies $PATH", func() {
138138
BeforeEach(func() {
139-
// dockerfile contains `ENV /usr/local/bin:/usr/bin:/bin:/from-ENV`,
139+
// Dockerfile contains:
140+
// ENV PATH /usr/local/bin:/usr/bin:/bin:/from-dockerfile
141+
// ENV TEST test-from-dockerfile
142+
// ENV TEST second-test-from-dockerfile:$TEST
140143
// see diego-dockerfiles/with-volume
141144
rootfs = "docker:///cloudfoundry/with-volume"
142145
})
143146

144-
It("$HOME is taken from the docker image", func() {
147+
It("$PATH is taken from the docker image", func() {
145148
stdout := gbytes.NewBuffer()
146149
process, err := container.Run(garden.ProcessSpec{
147150
Path: "/bin/sh",
148-
Args: []string{"-c", "echo $HOME"},
151+
Args: []string{"-c", "echo $PATH"},
149152
}, garden.ProcessIO{
150153
Stdout: io.MultiWriter(GinkgoWriter, stdout),
151154
Stderr: GinkgoWriter,
@@ -154,14 +157,14 @@ var _ = Describe("Creating a container", func() {
154157
Ω(err).ShouldNot(HaveOccurred())
155158

156159
process.Wait()
157-
Ω(stdout).Should(gbytes.Say("/home-from-ENV"))
160+
Ω(stdout).Should(gbytes.Say("/usr/local/bin:/usr/bin:/bin:/from-dockerfile"))
158161
})
159162

160-
PIt("$PATH is taken from the docker image", func() {
163+
It("$TEST is taken from the docker image", func() {
161164
stdout := gbytes.NewBuffer()
162165
process, err := container.Run(garden.ProcessSpec{
163166
Path: "/bin/sh",
164-
Args: []string{"-c", "echo $PATH"},
167+
Args: []string{"-c", "echo $TEST"},
165168
}, garden.ProcessIO{
166169
Stdout: io.MultiWriter(GinkgoWriter, stdout),
167170
Stderr: GinkgoWriter,
@@ -170,7 +173,7 @@ var _ = Describe("Creating a container", func() {
170173
Ω(err).ShouldNot(HaveOccurred())
171174

172175
process.Wait()
173-
Ω(stdout).Should(gbytes.Say("/usr/local/bin:/usr/bin:/bin:/from-ENV"))
176+
Ω(stdout).Should(gbytes.Say("second-test-from-dockerfile:test-from-dockerfile"))
174177
})
175178
})
176179
})

Diff for: ‎old/integration/wshd/main_test.go

-3
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,6 @@ setup_fs
452452
ls := exec.Command(wsh,
453453
"--socket", socketPath,
454454
"--user", "vcap",
455-
"--env", "PATH=nada",
456455
"ls",
457456
)
458457

@@ -464,7 +463,6 @@ setup_fs
464463
onlyInSbin := exec.Command(wsh,
465464
"--socket", socketPath,
466465
"--user", "vcap",
467-
"--env", "PATH=nada",
468466
"ifconfig",
469467
)
470468

@@ -537,7 +535,6 @@ setup_fs
537535
onlyInSbin := exec.Command(wsh,
538536
"--socket", socketPath,
539537
"--user", "root",
540-
"--env", "PATH=nada",
541538
"ifconfig",
542539
)
543540

Diff for: ‎old/linux_backend/container_pool/container_pool.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -250,23 +250,22 @@ func (p *LinuxContainerPool) Create(spec garden.ContainerSpec) (c linux_backend.
250250
return nil, err
251251
}
252252

253-
rootFSEnvVars, err := p.acquireSystemResources(id, containerPath, spec.RootFSPath, resources, spec.BindMounts, pLog)
253+
rootFSEnv, err := p.acquireSystemResources(id, containerPath, spec.RootFSPath, resources, spec.BindMounts, pLog)
254254
if err != nil {
255255
return nil, err
256256
}
257257

258258
pLog.Info("created")
259259

260-
ctrEnv, err := process.NewEnv(rootFSEnvVars)
261-
if err != nil {
262-
return nil, err
263-
}
264-
265260
specEnv, err := process.NewEnv(spec.Env)
266261
if err != nil {
267262
return nil, err
268263
}
269264

265+
pLog.Debug("calculate-environment", lager.Data{
266+
"rootfs-env": rootFSEnv,
267+
"create-env": specEnv,
268+
})
270269
return linux_backend.NewLinuxContainer(
271270
pLog,
272271
id,
@@ -281,7 +280,7 @@ func (p *LinuxContainerPool) Create(spec garden.ContainerSpec) (c linux_backend.
281280
p.quotaManager,
282281
bandwidth_manager.New(containerPath, id, p.runner),
283282
process_tracker.New(containerPath, p.runner),
284-
ctrEnv.Merge(specEnv),
283+
rootFSEnv.Merge(specEnv),
285284
p.filterProvider.ProvideFilter(id),
286285
), nil
287286
}
@@ -531,7 +530,7 @@ func (p *LinuxContainerPool) releasePoolResources(resources *linux_backend.Resou
531530
}
532531
}
533532

534-
func (p *LinuxContainerPool) acquireSystemResources(id, containerPath, rootFSPath string, resources *linux_backend.Resources, bindMounts []garden.BindMount, pLog lager.Logger) ([]string, error) {
533+
func (p *LinuxContainerPool) acquireSystemResources(id, containerPath, rootFSPath string, resources *linux_backend.Resources, bindMounts []garden.BindMount, pLog lager.Logger) (process.Env, error) {
535534
rootfsURL, err := url.Parse(rootFSPath)
536535
if err != nil {
537536
pLog.Error("parse-rootfs-path-failed", err, lager.Data{
@@ -556,14 +555,15 @@ func (p *LinuxContainerPool) acquireSystemResources(id, containerPath, rootFSPat
556555

557556
createCmd := path.Join(p.binPath, "create.sh")
558557
create := exec.Command(createCmd, containerPath)
559-
create.Env = []string{
560-
"id=" + id,
561-
"rootfs_path=" + rootfsPath,
562-
fmt.Sprintf("user_uid=%d", resources.UserUID),
563-
fmt.Sprintf("root_uid=%d", resources.RootUID),
564-
"PATH=" + os.Getenv("PATH"),
565-
}
566-
resources.Network.ConfigureProcess(&create.Env)
558+
env := process.Env{
559+
"id": id,
560+
"rootfs_path": rootfsPath,
561+
"user_uid": strconv.FormatUint(uint64(resources.UserUID), 10),
562+
"root_uid": strconv.FormatUint(uint64(resources.RootUID), 10),
563+
"PATH": os.Getenv("PATH"),
564+
}
565+
resources.Network.ConfigureProcess(env)
566+
create.Env = env.Array()
567567

568568
pRunner := logging.Runner{
569569
CommandRunner: p.runner,

0 commit comments

Comments
 (0)