Skip to content

Commit ae85986

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

7 files changed

Lines changed: 214 additions & 14 deletions

File tree

‎integration/container_update_resources_test.go‎

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

60+
func checkMemoryLimitInContainerStatus(t *testing.T, status *runtime.ContainerStatus, memLimit int64) {
61+
t.Helper()
62+
require.NotNil(t, status)
63+
require.NotNil(t, status.Resources)
64+
require.NotNil(t, status.Resources.Linux)
65+
require.NotNil(t, status.Resources.Linux.MemoryLimitInBytes)
66+
assert.Equal(t, memLimit, status.Resources.Linux.MemoryLimitInBytes)
67+
}
68+
6069
func getCgroupSwapLimitForTask(t *testing.T, task containerd.Task) uint64 {
6170
if cgroups.Mode() == cgroups.Unified {
6271
groupPath, err := cgroupsv2.PidGroupPath(int(task.Pid()))
@@ -282,3 +291,51 @@ func TestUpdateContainerResources_MemoryLimit(t *testing.T) {
282291
require.NoError(t, err)
283292
assert.Equal(t, uint64(800*1024*1024), stat.Memory.Usage.Limit)
284293
}
294+
295+
func TestUpdateContainerResources_StatusUpdated(t *testing.T) {
296+
t.Log("Create a sandbox")
297+
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "update-container-resources")
298+
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
@@ -142,6 +142,9 @@ func TestContainerdImage(t *testing.T) {
142142
if err != nil {
143143
return false, err
144144
}
145+
if s.Resources == nil || (s.Resources.Linux == nil && s.Resources.Windows == nil) {
146+
return false, fmt.Errorf("No Resource field in container status: %+v", s)
147+
}
145148
return s.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil
146149
}
147150
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
@@ -258,6 +258,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
258258
}()
259259

260260
status := containerstore.Status{CreatedAt: time.Now().UnixNano()}
261+
status = copyResourcesToStatus(spec, status)
261262
container, err := containerstore.NewContainer(meta,
262263
containerstore.WithStatus(status, containerRootDir),
263264
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
@@ -37,6 +37,7 @@ import (
3737
"github.com/containerd/containerd/runtime/linux/runctypes"
3838
runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
3939
"github.com/containerd/typeurl"
40+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
4041
"github.com/sirupsen/logrus"
4142

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

‎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)