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

Commit f0b5665

Browse files
Random-LiuRandom-Liu
authored
Merge pull request #1048 from Random-Liu/cherrypick-#1045-release-1.0
Cherrypick #1045 release 1.0
2 parents eedb9f8 + 5edec1d commit f0b5665

6 files changed

Lines changed: 150 additions & 21 deletions

File tree

‎pkg/server/container_create.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
3737
"github.com/opencontainers/runc/libcontainer/devices"
3838
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
39-
"github.com/opencontainers/runtime-tools/generate"
4039
"github.com/opencontainers/runtime-tools/validate"
4140
"github.com/opencontainers/selinux/go-selinux/label"
4241
"github.com/pkg/errors"
@@ -77,6 +76,7 @@ func init() {
7776
// CreateContainer creates a new container in the given PodSandbox.
7877
func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateContainerRequest) (_ *runtime.CreateContainerResponse, retErr error) {
7978
config := r.GetConfig()
79+
logrus.Debugf("Container config %+v", config)
8080
sandboxConfig := r.GetSandboxConfig()
8181
sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId())
8282
if err != nil {
@@ -507,7 +507,7 @@ func (c *criService) generateContainerMounts(sandboxID string, config *runtime.C
507507

508508
// setOCIProcessArgs sets process args. It returns error if the final arg list
509509
// is empty.
510-
func setOCIProcessArgs(g *generate.Generator, config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) error {
510+
func setOCIProcessArgs(g *generator, config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) error {
511511
command, args := config.GetCommand(), config.GetArgs()
512512
// The following logic is migrated from https://github.com/moby/moby/blob/master/daemon/commit.go
513513
// TODO(random-liu): Clearly define the commands overwrite behavior.
@@ -529,7 +529,7 @@ func setOCIProcessArgs(g *generate.Generator, config *runtime.ContainerConfig, i
529529

530530
// addImageEnvs adds environment variables from image config. It returns error if
531531
// an invalid environment variable is encountered.
532-
func addImageEnvs(g *generate.Generator, imageEnvs []string) error {
532+
func addImageEnvs(g *generator, imageEnvs []string) error {
533533
for _, e := range imageEnvs {
534534
kv := strings.SplitN(e, "=", 2)
535535
if len(kv) != 2 {
@@ -540,7 +540,7 @@ func addImageEnvs(g *generate.Generator, imageEnvs []string) error {
540540
return nil
541541
}
542542

543-
func setOCIPrivileged(g *generate.Generator, config *runtime.ContainerConfig) error {
543+
func setOCIPrivileged(g *generator, config *runtime.ContainerConfig) error {
544544
// Add all capabilities in privileged mode.
545545
g.SetupPrivileged(true)
546546
setOCIBindMountsPrivileged(g)
@@ -561,7 +561,7 @@ func clearReadOnly(m *runtimespec.Mount) {
561561
}
562562

563563
// addDevices set device mapping without privilege.
564-
func (c *criService) addOCIDevices(g *generate.Generator, devs []*runtime.Device) error {
564+
func (c *criService) addOCIDevices(g *generator, devs []*runtime.Device) error {
565565
spec := g.Spec()
566566
for _, device := range devs {
567567
path, err := c.os.ResolveSymbolicLink(device.HostPath)
@@ -593,7 +593,7 @@ func (c *criService) addOCIDevices(g *generate.Generator, devs []*runtime.Device
593593
}
594594

595595
// addDevices set device mapping with privilege.
596-
func setOCIDevicesPrivileged(g *generate.Generator) error {
596+
func setOCIDevicesPrivileged(g *generator) error {
597597
spec := g.Spec()
598598
hostDevices, err := devices.HostDevices()
599599
if err != nil {
@@ -624,7 +624,7 @@ func setOCIDevicesPrivileged(g *generate.Generator) error {
624624
}
625625

626626
// addOCIBindMounts adds bind mounts.
627-
func (c *criService) addOCIBindMounts(g *generate.Generator, mounts []*runtime.Mount, mountLabel string) error {
627+
func (c *criService) addOCIBindMounts(g *generator, mounts []*runtime.Mount, mountLabel string) error {
628628
// Sort mounts in number of parts. This ensures that high level mounts don't
629629
// shadow other mounts.
630630
sort.Sort(orderedMounts(mounts))
@@ -719,7 +719,7 @@ func (c *criService) addOCIBindMounts(g *generate.Generator, mounts []*runtime.M
719719
return nil
720720
}
721721

722-
func setOCIBindMountsPrivileged(g *generate.Generator) {
722+
func setOCIBindMountsPrivileged(g *generator) {
723723
spec := g.Spec()
724724
// clear readonly for /sys and cgroup
725725
for i, m := range spec.Mounts {
@@ -734,8 +734,8 @@ func setOCIBindMountsPrivileged(g *generate.Generator) {
734734
spec.Linux.MaskedPaths = nil
735735
}
736736

737-
// setOCILinuxResource set container resource limit.
738-
func setOCILinuxResource(g *generate.Generator, resources *runtime.LinuxContainerResources) {
737+
// setOCILinuxResource set container cgroup resource limit.
738+
func setOCILinuxResource(g *generator, resources *runtime.LinuxContainerResources) {
739739
if resources == nil {
740740
return
741741
}
@@ -761,7 +761,7 @@ func getOCICapabilitiesList() []string {
761761
}
762762

763763
// setOCICapabilities adds/drops process capabilities.
764-
func setOCICapabilities(g *generate.Generator, capabilities *runtime.Capability) error {
764+
func setOCICapabilities(g *generator, capabilities *runtime.Capability) error {
765765
if capabilities == nil {
766766
return nil
767767
}
@@ -807,7 +807,7 @@ func setOCICapabilities(g *generate.Generator, capabilities *runtime.Capability)
807807
}
808808

809809
// setOCINamespaces sets namespaces.
810-
func setOCINamespaces(g *generate.Generator, namespaces *runtime.NamespaceOption, sandboxPid uint32) {
810+
func setOCINamespaces(g *generator, namespaces *runtime.NamespaceOption, sandboxPid uint32) {
811811
g.AddOrReplaceLinuxNamespace(string(runtimespec.NetworkNamespace), getNetworkNamespace(sandboxPid)) // nolint: errcheck
812812
g.AddOrReplaceLinuxNamespace(string(runtimespec.IPCNamespace), getIPCNamespace(sandboxPid)) // nolint: errcheck
813813
g.AddOrReplaceLinuxNamespace(string(runtimespec.UTSNamespace), getUTSNamespace(sandboxPid)) // nolint: errcheck

‎pkg/server/container_create_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ func TestContainerSpecCommand(t *testing.T) {
451451
} {
452452

453453
config, _, imageConfig, _ := getCreateContainerTestData()
454-
g := generate.New()
454+
g := newCustomGenerator(generate.New())
455455
config.Command = test.criEntrypoint
456456
config.Args = test.criArgs
457457
imageConfig.Entrypoint = test.imageEntrypoint
@@ -671,7 +671,7 @@ func TestPrivilegedBindMount(t *testing.T) {
671671
},
672672
} {
673673
t.Logf("TestCase %q", desc)
674-
g := generate.New()
674+
g := newCustomGenerator(generate.New())
675675
g.SetRootReadonly(test.readonlyRootFS)
676676
c := newTestCRIService()
677677
c.addOCIBindMounts(&g, nil, "")
@@ -780,7 +780,7 @@ func TestMountPropagation(t *testing.T) {
780780
},
781781
} {
782782
t.Logf("TestCase %q", desc)
783-
g := generate.New()
783+
g := newCustomGenerator(generate.New())
784784
c := newTestCRIService()
785785
c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn
786786
err := c.addOCIBindMounts(&g, []*runtime.Mount{test.criMount}, "")

‎pkg/server/helpers.go‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,10 +438,52 @@ func buildLabels(configLabels map[string]string, containerType string) map[strin
438438
}
439439

440440
// newSpecGenerator creates a new spec generator for the runtime spec.
441-
func newSpecGenerator(spec *runtimespec.Spec) generate.Generator {
441+
func newSpecGenerator(spec *runtimespec.Spec) generator {
442442
g := generate.NewFromSpec(spec)
443443
g.HostSpecific = true
444-
return g
444+
return newCustomGenerator(g)
445+
}
446+
447+
// generator is a custom generator with some functions overridden
448+
// used by the cri plugin.
449+
// TODO(random-liu): Upstream this fix.
450+
type generator struct {
451+
generate.Generator
452+
envCache map[string]int
453+
}
454+
455+
func newCustomGenerator(g generate.Generator) generator {
456+
cg := generator{
457+
Generator: g,
458+
envCache: make(map[string]int),
459+
}
460+
spec := g.Spec()
461+
if spec != nil && spec.Process != nil {
462+
for i, env := range spec.Process.Env {
463+
kv := strings.SplitN(env, "=", 2)
464+
cg.envCache[kv[0]] = i
465+
}
466+
}
467+
return cg
468+
}
469+
470+
// AddProcessEnv overrides the original AddProcessEnv. It uses
471+
// a map to cache and override envs.
472+
func (g *generator) AddProcessEnv(key, value string) {
473+
if len(g.envCache) == 0 {
474+
// Call AddProccessEnv once to initialize the spec.
475+
g.Generator.AddProcessEnv(key, value)
476+
g.envCache[key] = 0
477+
return
478+
}
479+
spec := g.Spec()
480+
env := fmt.Sprintf("%s=%s", key, value)
481+
if idx, ok := g.envCache[key]; !ok {
482+
spec.Process.Env = append(spec.Process.Env, env)
483+
g.envCache[key] = len(spec.Process.Env) - 1
484+
} else {
485+
spec.Process.Env[idx] = env
486+
}
445487
}
446488

447489
func getPodCNILabels(id string, config *runtime.PodSandboxConfig) map[string]string {

‎pkg/server/helpers_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/containerd/containerd/containers"
2525
"github.com/containerd/containerd/linux/runctypes"
2626
imagedigest "github.com/opencontainers/go-digest"
27+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
2728
"github.com/stretchr/testify/assert"
2829
"golang.org/x/net/context"
2930
runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
@@ -213,3 +214,87 @@ func TestOrderedMounts(t *testing.T) {
213214
sort.Stable(orderedMounts(mounts))
214215
assert.Equal(t, expected, mounts)
215216
}
217+
218+
func TestCustomGenerator(t *testing.T) {
219+
for desc, test := range map[string]struct {
220+
existing []string
221+
kv [][2]string
222+
expected []string
223+
expectNil bool
224+
}{
225+
"empty": {
226+
expectNil: true,
227+
},
228+
"single env": {
229+
kv: [][2]string{
230+
{"a", "b"},
231+
},
232+
expected: []string{"a=b"},
233+
},
234+
"multiple envs": {
235+
kv: [][2]string{
236+
{"a", "b"},
237+
{"c", "d"},
238+
{"e", "f"},
239+
},
240+
expected: []string{
241+
"a=b",
242+
"c=d",
243+
"e=f",
244+
},
245+
},
246+
"env override": {
247+
kv: [][2]string{
248+
{"k1", "v1"},
249+
{"k2", "v2"},
250+
{"k3", "v3"},
251+
{"k3", "v4"},
252+
{"k1", "v5"},
253+
{"k4", "v6"},
254+
},
255+
expected: []string{
256+
"k1=v5",
257+
"k2=v2",
258+
"k3=v4",
259+
"k4=v6",
260+
},
261+
},
262+
"existing env": {
263+
existing: []string{
264+
"k1=v1",
265+
"k2=v2",
266+
"k3=v3",
267+
},
268+
kv: [][2]string{
269+
{"k3", "v4"},
270+
{"k2", "v5"},
271+
{"k4", "v6"},
272+
},
273+
expected: []string{
274+
"k1=v1",
275+
"k2=v5",
276+
"k3=v4",
277+
"k4=v6",
278+
},
279+
},
280+
} {
281+
t.Logf("TestCase %q", desc)
282+
var spec *runtimespec.Spec
283+
if len(test.existing) > 0 {
284+
spec = &runtimespec.Spec{
285+
Process: &runtimespec.Process{
286+
Env: test.existing,
287+
},
288+
}
289+
}
290+
g := newSpecGenerator(spec)
291+
for _, kv := range test.kv {
292+
g.AddProcessEnv(kv[0], kv[1])
293+
}
294+
if test.expectNil {
295+
assert.Nil(t, g.Spec())
296+
} else {
297+
assert.Equal(t, test.expected, g.Spec().Process.Env)
298+
}
299+
}
300+
}

‎pkg/server/instrumented_service.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (in *instrumentedService) RunPodSandbox(ctx context.Context, r *runtime.Run
5252
if err := in.checkInitialized(); err != nil {
5353
return nil, err
5454
}
55-
logrus.Infof("RunPodSandbox with config %+v", r.GetConfig())
55+
logrus.Infof("RunPodsandbox for %+v", r.GetConfig().GetMetadata())
5656
defer func() {
5757
if err != nil {
5858
logrus.WithError(err).Errorf("RunPodSandbox for %+v failed, error", r.GetConfig().GetMetadata())
@@ -142,8 +142,8 @@ func (in *instrumentedService) CreateContainer(ctx context.Context, r *runtime.C
142142
if err := in.checkInitialized(); err != nil {
143143
return nil, err
144144
}
145-
logrus.Infof("CreateContainer within sandbox %q with container config %+v and sandbox config %+v",
146-
r.GetPodSandboxId(), r.GetConfig(), r.GetSandboxConfig())
145+
logrus.Infof("CreateContainer within sandbox %q for container %+v",
146+
r.GetPodSandboxId(), r.GetConfig().GetMetadata())
147147
defer func() {
148148
if err != nil {
149149
logrus.WithError(err).Errorf("CreateContainer within sandbox %q for %+v failed",

‎pkg/server/sandbox_run.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/containerd/containerd/oci"
2929
cni "github.com/containerd/go-cni"
3030
"github.com/containerd/typeurl"
31+
"github.com/davecgh/go-spew/spew"
3132
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
3233
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
3334
"github.com/pkg/errors"
@@ -54,6 +55,7 @@ func init() {
5455
// the sandbox is in ready state.
5556
func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandboxRequest) (_ *runtime.RunPodSandboxResponse, retErr error) {
5657
config := r.GetConfig()
58+
logrus.Debugf("Sandbox config %+v", config)
5759

5860
// Generate unique id and name for the sandbox and reserve the name.
5961
id := util.GenerateID()
@@ -142,7 +144,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
142144
if err != nil {
143145
return nil, errors.Wrap(err, "failed to generate sandbox container spec")
144146
}
145-
logrus.Debugf("Sandbox container spec: %+v", spec)
147+
logrus.Debugf("Sandbox container %q spec: %#+v", id, spew.NewFormatter(spec))
146148

147149
var specOpts []oci.SpecOpts
148150
userstr, err := generateUserString(

0 commit comments

Comments
 (0)