Skip to content

Commit b7b1200

Browse files
ruiwen-zhaoruiwen-zhao
committed
ContainerStatus to return container resources
Signed-off-by: ruiwen-zhao <[email protected]>
1 parent 6e4b683 commit b7b1200

7 files changed

Lines changed: 215 additions & 14 deletions

File tree

‎integration/container_update_resources_test.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ func checkMemorySwapLimit(t *testing.T, spec *runtimespec.Spec, memLimit *int64)
5858
}
5959
}
6060

61+
func checkMemoryLimitInContainerStatus(t *testing.T, status *runtime.ContainerStatus, memLimit int64) {
62+
t.Helper()
63+
require.NotNil(t, status)
64+
require.NotNil(t, status.Resources)
65+
require.NotNil(t, status.Resources.Linux)
66+
require.NotNil(t, status.Resources.Linux.MemoryLimitInBytes)
67+
assert.Equal(t, memLimit, status.Resources.Linux.MemoryLimitInBytes)
68+
}
69+
6170
func getCgroupSwapLimitForTask(t *testing.T, task containerd.Task) uint64 {
6271
if cgroups.Mode() == cgroups.Unified {
6372
groupPath, err := cgroupsv2.PidGroupPath(int(task.Pid()))
@@ -281,3 +290,52 @@ func TestUpdateContainerResources_MemoryLimit(t *testing.T) {
281290
memLimit = getCgroupMemoryLimitForTask(t, task)
282291
assert.Equal(t, uint64(800*1024*1024), memLimit)
283292
}
293+
294+
func TestUpdateContainerResources_StatusUpdated(t *testing.T) {
295+
t.Log("Create a sandbox")
296+
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "update-container-resources")
297+
298+
pauseImage := images.Get(images.Pause)
299+
EnsureImageExists(t, pauseImage)
300+
301+
t.Log("Create a container with memory limit")
302+
cnConfig := ContainerConfig(
303+
"container",
304+
pauseImage,
305+
WithResources(&runtime.LinuxContainerResources{
306+
MemoryLimitInBytes: 200 * 1024 * 1024,
307+
}),
308+
)
309+
cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
310+
require.NoError(t, err)
311+
312+
t.Log("Check memory limit in container status")
313+
status, err := runtimeService.ContainerStatus(cn)
314+
checkMemoryLimitInContainerStatus(t, status, 200*1024*1024)
315+
require.NoError(t, err)
316+
317+
t.Log("Update container memory limit after created")
318+
err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{
319+
MemoryLimitInBytes: 400 * 1024 * 1024,
320+
}, nil)
321+
require.NoError(t, err)
322+
323+
t.Log("Check memory limit in container status")
324+
status, err = runtimeService.ContainerStatus(cn)
325+
checkMemoryLimitInContainerStatus(t, status, 400*1024*1024)
326+
require.NoError(t, err)
327+
328+
t.Log("Start the container")
329+
require.NoError(t, runtimeService.StartContainer(cn))
330+
331+
t.Log("Update container memory limit after started")
332+
err = runtimeService.UpdateContainerResources(cn, &runtime.LinuxContainerResources{
333+
MemoryLimitInBytes: 800 * 1024 * 1024,
334+
}, nil)
335+
require.NoError(t, err)
336+
337+
t.Log("Check memory limit in container status")
338+
status, err = runtimeService.ContainerStatus(cn)
339+
checkMemoryLimitInContainerStatus(t, status, 800*1024*1024)
340+
require.NoError(t, err)
341+
}

‎integration/containerd_image_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ func TestContainerdImage(t *testing.T) {
143143
if err != nil {
144144
return false, err
145145
}
146+
if s.Resources == nil || (s.Resources.Linux == nil && s.Resources.Windows == nil) {
147+
return false, fmt.Errorf("No Resource field in container status: %+v", s)
148+
}
146149
return s.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil
147150
}
148151
require.NoError(t, Eventually(checkContainer, 100*time.Millisecond, 10*time.Second))

‎pkg/cri/server/container_create.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
261261
}()
262262

263263
status := containerstore.Status{CreatedAt: time.Now().UnixNano()}
264+
status = copyResourcesToStatus(spec, status)
264265
container, err := containerstore.NewContainer(meta,
265266
containerstore.WithStatus(status, containerRootDir),
266267
containerstore.WithContainer(cntr),

‎pkg/cri/server/container_status.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
6060
}
6161
}
6262
status := toCRIContainerStatus(container, spec, imageRef)
63+
6364
if status.GetCreatedAt() == 0 {
6465
// CRI doesn't allow CreatedAt == 0.
6566
info, err := container.Container.Info(ctx)
@@ -119,6 +120,7 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
119120
Annotations: meta.Config.GetAnnotations(),
120121
Mounts: meta.Config.GetMounts(),
121122
LogPath: meta.LogPath,
123+
Resources: status.Resources,
122124
}
123125
}
124126

‎pkg/cri/server/container_update_resources.go‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
4545
// Update resources in status update transaction, so that:
4646
// 1) There won't be race condition with container start.
4747
// 2) There won't be concurrent resource update to the same container.
48-
if err := container.Status.Update(func(status containerstore.Status) (containerstore.Status, error) {
49-
return status, c.updateContainerResources(ctx, container, r, status)
48+
if err := container.Status.UpdateSync(func(status containerstore.Status) (containerstore.Status, error) {
49+
return c.updateContainerResources(ctx, container, r, status)
5050
}); err != nil {
5151
return nil, fmt.Errorf("failed to update resources: %w", err)
5252
}
@@ -56,11 +56,13 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
5656
func (c *criService) updateContainerResources(ctx context.Context,
5757
cntr containerstore.Container,
5858
r *runtime.UpdateContainerResourcesRequest,
59-
status containerstore.Status) (retErr error) {
59+
status containerstore.Status) (newStatus containerstore.Status, retErr error) {
60+
61+
newStatus = status
6062
id := cntr.ID
6163
// Do not update the container when there is a removal in progress.
6264
if status.Removing {
63-
return fmt.Errorf("container %q is in removing state", id)
65+
return newStatus, fmt.Errorf("container %q is in removing state", id)
6466
}
6567

6668
// Update container spec. If the container is not started yet, updating
@@ -69,15 +71,15 @@ func (c *criService) updateContainerResources(ctx context.Context,
6971
// the spec will become our source of truth for resource limits.
7072
oldSpec, err := cntr.Container.Spec(ctx)
7173
if err != nil {
72-
return fmt.Errorf("failed to get container spec: %w", err)
74+
return newStatus, fmt.Errorf("failed to get container spec: %w", err)
7375
}
7476
newSpec, err := updateOCIResource(ctx, oldSpec, r, c.config)
7577
if err != nil {
76-
return fmt.Errorf("failed to update resource in spec: %w", err)
78+
return newStatus, fmt.Errorf("failed to update resource in spec: %w", err)
7779
}
7880

7981
if err := updateContainerSpec(ctx, cntr.Container, newSpec); err != nil {
80-
return err
82+
return newStatus, err
8183
}
8284
defer func() {
8385
if retErr != nil {
@@ -87,32 +89,35 @@ func (c *criService) updateContainerResources(ctx context.Context,
8789
if err := updateContainerSpec(deferCtx, cntr.Container, oldSpec); err != nil {
8890
log.G(ctx).WithError(err).Errorf("Failed to update spec %+v for container %q", oldSpec, id)
8991
}
92+
} else {
93+
// Update container status only when the spec is updated
94+
newStatus = copyResourcesToStatus(newSpec, status)
9095
}
9196
}()
9297

9398
// If container is not running, only update spec is enough, new resource
9499
// limit will be applied when container start.
95100
if status.State() != runtime.ContainerState_CONTAINER_RUNNING {
96-
return nil
101+
return newStatus, nil
97102
}
98103

99104
task, err := cntr.Container.Task(ctx, nil)
100105
if err != nil {
101106
if errdefs.IsNotFound(err) {
102107
// Task exited already.
103-
return nil
108+
return newStatus, nil
104109
}
105-
return fmt.Errorf("failed to get task: %w", err)
110+
return newStatus, fmt.Errorf("failed to get task: %w", err)
106111
}
107112
// newSpec.Linux / newSpec.Windows won't be nil
108113
if err := task.Update(ctx, containerd.WithResources(getResources(newSpec))); err != nil {
109114
if errdefs.IsNotFound(err) {
110115
// Task exited already.
111-
return nil
116+
return newStatus, nil
112117
}
113-
return fmt.Errorf("failed to update resources: %w", err)
118+
return newStatus, fmt.Errorf("failed to update resources: %w", err)
114119
}
115-
return nil
120+
return newStatus, nil
116121
}
117122

118123
// updateContainerSpec updates container spec.

‎pkg/cri/server/helpers.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"github.com/containerd/containerd/runtime/linux/runctypes"
3939
runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
4040
"github.com/containerd/typeurl"
41+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
4142
"github.com/sirupsen/logrus"
4243

4344
runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
@@ -429,3 +430,83 @@ func getPassthroughAnnotations(podAnnotations map[string]string,
429430
}
430431
return passthroughAnnotations
431432
}
433+
434+
// copyResourcesToStatus copys container resource contraints from spec to
435+
// container status.
436+
// This will need updates when new fields are added to ContainerResources.
437+
func copyResourcesToStatus(spec *runtimespec.Spec, status containerstore.Status) containerstore.Status {
438+
status.Resources = &runtime.ContainerResources{}
439+
if spec.Linux != nil {
440+
status.Resources.Linux = &runtime.LinuxContainerResources{}
441+
442+
if spec.Process != nil && spec.Process.OOMScoreAdj != nil {
443+
status.Resources.Linux.OomScoreAdj = int64(*spec.Process.OOMScoreAdj)
444+
}
445+
446+
if spec.Linux.Resources == nil {
447+
return status
448+
}
449+
450+
if spec.Linux.Resources.CPU != nil {
451+
if spec.Linux.Resources.CPU.Period != nil {
452+
status.Resources.Linux.CpuPeriod = int64(*spec.Linux.Resources.CPU.Period)
453+
}
454+
if spec.Linux.Resources.CPU.Quota != nil {
455+
status.Resources.Linux.CpuQuota = *spec.Linux.Resources.CPU.Quota
456+
}
457+
if spec.Linux.Resources.CPU.Shares != nil {
458+
status.Resources.Linux.CpuShares = int64(*spec.Linux.Resources.CPU.Shares)
459+
}
460+
status.Resources.Linux.CpusetCpus = spec.Linux.Resources.CPU.Cpus
461+
status.Resources.Linux.CpusetMems = spec.Linux.Resources.CPU.Mems
462+
}
463+
464+
if spec.Linux.Resources.Memory != nil {
465+
if spec.Linux.Resources.Memory.Limit != nil {
466+
status.Resources.Linux.MemoryLimitInBytes = *spec.Linux.Resources.Memory.Limit
467+
}
468+
if spec.Linux.Resources.Memory.Swap != nil {
469+
status.Resources.Linux.MemorySwapLimitInBytes = *spec.Linux.Resources.Memory.Swap
470+
}
471+
}
472+
473+
if spec.Linux.Resources.HugepageLimits != nil {
474+
hugepageLimits := make([]*runtime.HugepageLimit, len(spec.Linux.Resources.HugepageLimits))
475+
for _, l := range spec.Linux.Resources.HugepageLimits {
476+
hugepageLimits = append(hugepageLimits, &runtime.HugepageLimit{
477+
PageSize: l.Pagesize,
478+
Limit: l.Limit,
479+
})
480+
}
481+
status.Resources.Linux.HugepageLimits = hugepageLimits
482+
}
483+
484+
if spec.Linux.Resources.Unified != nil {
485+
status.Resources.Linux.Unified = spec.Linux.Resources.Unified
486+
}
487+
}
488+
489+
if spec.Windows != nil {
490+
status.Resources.Windows = &runtime.WindowsContainerResources{}
491+
if spec.Windows.Resources == nil {
492+
return status
493+
}
494+
495+
if spec.Windows.Resources.CPU != nil {
496+
if spec.Windows.Resources.CPU.Shares != nil {
497+
status.Resources.Windows.CpuShares = int64(*spec.Windows.Resources.CPU.Shares)
498+
status.Resources.Windows.CpuCount = int64(*spec.Windows.Resources.CPU.Count)
499+
status.Resources.Windows.CpuMaximum = int64(*spec.Windows.Resources.CPU.Maximum)
500+
}
501+
}
502+
503+
if spec.Windows.Resources.Memory != nil {
504+
if spec.Windows.Resources.Memory.Limit != nil {
505+
status.Resources.Windows.MemoryLimitInBytes = int64(*spec.Windows.Resources.Memory.Limit)
506+
}
507+
}
508+
509+
// TODO: Figure out how to get RootfsSizeInBytes
510+
}
511+
return status
512+
}

‎pkg/cri/store/container/status.go‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ type Status struct {
9797
// Unknown indicates that the container status is not fully loaded.
9898
// This field doesn't need to be checkpointed.
9999
Unknown bool `json:"-"`
100+
// Resources has container runtime resource constraints
101+
Resources *runtime.ContainerResources
100102
}
101103

102104
// State returns current state of the container based on the container status.
@@ -203,7 +205,56 @@ type statusStorage struct {
203205
func (s *statusStorage) Get() Status {
204206
s.RLock()
205207
defer s.RUnlock()
206-
return s.status
208+
// Deep copy is needed in case some fields in Status are updated after Get()
209+
// is called.
210+
return deepCopyOf(s.status)
211+
}
212+
213+
func deepCopyOf(s Status) Status {
214+
copy := s
215+
// Resources is the only field that is a pointer, and therefore needs
216+
// a manual deep copy.
217+
// This will need updates when new fields are added to ContainerResources.
218+
if s.Resources == nil {
219+
return copy
220+
}
221+
copy.Resources = &runtime.ContainerResources{}
222+
if s.Resources != nil && s.Resources.Linux != nil {
223+
hugepageLimits := make([]*runtime.HugepageLimit, len(s.Resources.Linux.HugepageLimits))
224+
for _, l := range s.Resources.Linux.HugepageLimits {
225+
hugepageLimits = append(hugepageLimits, &runtime.HugepageLimit{
226+
PageSize: l.PageSize,
227+
Limit: l.Limit,
228+
})
229+
}
230+
copy.Resources = &runtime.ContainerResources{
231+
Linux: &runtime.LinuxContainerResources{
232+
CpuPeriod: s.Resources.Linux.CpuPeriod,
233+
CpuQuota: s.Resources.Linux.CpuQuota,
234+
CpuShares: s.Resources.Linux.CpuShares,
235+
CpusetCpus: s.Resources.Linux.CpusetCpus,
236+
CpusetMems: s.Resources.Linux.CpusetMems,
237+
MemoryLimitInBytes: s.Resources.Linux.MemoryLimitInBytes,
238+
MemorySwapLimitInBytes: s.Resources.Linux.MemorySwapLimitInBytes,
239+
OomScoreAdj: s.Resources.Linux.OomScoreAdj,
240+
Unified: s.Resources.Linux.Unified,
241+
HugepageLimits: hugepageLimits,
242+
},
243+
}
244+
}
245+
246+
if s.Resources != nil && s.Resources.Windows != nil {
247+
copy.Resources = &runtime.ContainerResources{
248+
Windows: &runtime.WindowsContainerResources{
249+
CpuShares: s.Resources.Windows.CpuShares,
250+
CpuCount: s.Resources.Windows.CpuCount,
251+
CpuMaximum: s.Resources.Windows.CpuMaximum,
252+
MemoryLimitInBytes: s.Resources.Windows.MemoryLimitInBytes,
253+
RootfsSizeInBytes: s.Resources.Windows.RootfsSizeInBytes,
254+
},
255+
}
256+
}
257+
return copy
207258
}
208259

209260
// UpdateSync updates the container status and the on disk checkpoint.

0 commit comments

Comments
 (0)