Skip to content

Commit 5335e56

Browse files
samuelkarpsamuelkarp
committed
sbserver: return resources in ContainerStatus
Port of b7b1200 to sbserver Signed-off-by: Samuel Karp <[email protected]>
1 parent f8f0420 commit 5335e56

4 files changed

Lines changed: 108 additions & 18 deletions

File tree

‎pkg/cri/sbserver/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/sbserver/container_status.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
119119
Annotations: meta.Config.GetAnnotations(),
120120
Mounts: meta.Config.GetMounts(),
121121
LogPath: meta.LogPath,
122+
Resources: status.Resources,
122123
}
123124
}
124125

‎pkg/cri/sbserver/container_update_resources.go‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,14 @@ import (
2424
gocontext "context"
2525
"fmt"
2626

27+
"github.com/containerd/typeurl"
28+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
29+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
30+
2731
"github.com/containerd/containerd"
2832
"github.com/containerd/containerd/containers"
2933
"github.com/containerd/containerd/errdefs"
3034
"github.com/containerd/containerd/log"
31-
"github.com/containerd/typeurl"
32-
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
33-
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
3435

3536
containerstore "github.com/containerd/containerd/pkg/cri/store/container"
3637
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
@@ -45,8 +46,8 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
4546
// Update resources in status update transaction, so that:
4647
// 1) There won't be race condition with container start.
4748
// 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)
49+
if err := container.Status.UpdateSync(func(status containerstore.Status) (containerstore.Status, error) {
50+
return c.updateContainerResources(ctx, container, r, status)
5051
}); err != nil {
5152
return nil, fmt.Errorf("failed to update resources: %w", err)
5253
}
@@ -56,11 +57,13 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
5657
func (c *criService) updateContainerResources(ctx context.Context,
5758
cntr containerstore.Container,
5859
r *runtime.UpdateContainerResourcesRequest,
59-
status containerstore.Status) (retErr error) {
60+
status containerstore.Status) (newStatus containerstore.Status, retErr error) {
61+
62+
newStatus = status
6063
id := cntr.ID
6164
// Do not update the container when there is a removal in progress.
6265
if status.Removing {
63-
return fmt.Errorf("container %q is in removing state", id)
66+
return newStatus, fmt.Errorf("container %q is in removing state", id)
6467
}
6568

6669
// Update container spec. If the container is not started yet, updating
@@ -69,15 +72,15 @@ func (c *criService) updateContainerResources(ctx context.Context,
6972
// the spec will become our source of truth for resource limits.
7073
oldSpec, err := cntr.Container.Spec(ctx)
7174
if err != nil {
72-
return fmt.Errorf("failed to get container spec: %w", err)
75+
return newStatus, fmt.Errorf("failed to get container spec: %w", err)
7376
}
7477
newSpec, err := updateOCIResource(ctx, oldSpec, r, c.config)
7578
if err != nil {
76-
return fmt.Errorf("failed to update resource in spec: %w", err)
79+
return newStatus, fmt.Errorf("failed to update resource in spec: %w", err)
7780
}
7881

7982
if err := updateContainerSpec(ctx, cntr.Container, newSpec); err != nil {
80-
return err
83+
return newStatus, err
8184
}
8285
defer func() {
8386
if retErr != nil {
@@ -87,32 +90,35 @@ func (c *criService) updateContainerResources(ctx context.Context,
8790
if err := updateContainerSpec(deferCtx, cntr.Container, oldSpec); err != nil {
8891
log.G(ctx).WithError(err).Errorf("Failed to update spec %+v for container %q", oldSpec, id)
8992
}
93+
} else {
94+
// Update container status only when the spec is updated
95+
newStatus = copyResourcesToStatus(newSpec, status)
9096
}
9197
}()
9298

9399
// If container is not running, only update spec is enough, new resource
94100
// limit will be applied when container start.
95101
if status.State() != runtime.ContainerState_CONTAINER_RUNNING {
96-
return nil
102+
return newStatus, nil
97103
}
98104

99105
task, err := cntr.Container.Task(ctx, nil)
100106
if err != nil {
101107
if errdefs.IsNotFound(err) {
102108
// Task exited already.
103-
return nil
109+
return newStatus, nil
104110
}
105-
return fmt.Errorf("failed to get task: %w", err)
111+
return newStatus, fmt.Errorf("failed to get task: %w", err)
106112
}
107113
// newSpec.Linux / newSpec.Windows won't be nil
108114
if err := task.Update(ctx, containerd.WithResources(getResources(newSpec))); err != nil {
109115
if errdefs.IsNotFound(err) {
110116
// Task exited already.
111-
return nil
117+
return newStatus, nil
112118
}
113-
return fmt.Errorf("failed to update resources: %w", err)
119+
return newStatus, fmt.Errorf("failed to update resources: %w", err)
114120
}
115-
return nil
121+
return newStatus, nil
116122
}
117123

118124
// updateContainerSpec updates container spec.

‎pkg/cri/sbserver/helpers.go‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ import (
2424
"strconv"
2525
"strings"
2626

27+
"github.com/containerd/typeurl"
28+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
29+
"github.com/sirupsen/logrus"
30+
2731
"github.com/containerd/containerd"
2832
"github.com/containerd/containerd/containers"
2933
"github.com/containerd/containerd/errdefs"
@@ -37,8 +41,6 @@ import (
3741
"github.com/containerd/containerd/reference/docker"
3842
"github.com/containerd/containerd/runtime/linux/runctypes"
3943
runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
40-
"github.com/containerd/typeurl"
41-
"github.com/sirupsen/logrus"
4244

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

0 commit comments

Comments
 (0)