Skip to content

Commit 6dcde1d

Browse files
majabojarskamajabojarska
committed
feat: impl. container mount controller
Implements the ContainerMountController, which enables userVolume and hostPath mounts in containers. Signed-off-by: Maja Bojarska <maja.bojarska@siderolabs.com>
1 parent 48fe469 commit 6dcde1d

29 files changed

Lines changed: 2283 additions & 477 deletions

File tree

‎api/resource/definitions/containers/containers.proto‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,20 @@ message ContainerMountSpec {
9090
string destination = 4;
9191
// Size of a tmpfs mount, in bytes; zero means the kernel default.
9292
uint64 size = 5;
93-
// Options with the read-only default already applied.
93+
// Options with the writable default already applied.
9494
repeated string options = 6;
9595
}
9696

97+
// ContainerMountStatusSpec is the spec for ContainerMountStatus.
98+
message ContainerMountStatusSpec {
99+
// Ready is true once every mount the container declares is available.
100+
bool ready = 1;
101+
// Mounts are the resolved mounts, with host source paths filled in. Only meaningful when Ready.
102+
repeated ResolvedMountSpec mounts = 2;
103+
// Error describes why the mounts are not ready.
104+
string error = 3;
105+
}
106+
97107
// ContainerNetworkSpec is the resolved network configuration.
98108
message ContainerNetworkSpec {
99109
// HostNetwork shares the host network namespace instead of creating an empty one.
@@ -144,10 +154,12 @@ message ContainerSpecSpec {
144154
// ResolvedMountSpec is a mount with its host-side source resolved.
145155
message ResolvedMountSpec {
146156
string kind = 1;
147-
// Source is the host path to bind from; empty for tmpfs.
157+
// Source is the host path to bind from; empty for tmpfs and userVolume.
148158
string source = 2;
149159
string destination = 3;
150160
uint64 size = 4;
151161
repeated string options = 5;
162+
// VolumeID is the resolved userVolume's ID; empty for tmpfs and hostPath.
163+
string volume_id = 6;
152164
}
153165

‎internal/app/machined/pkg/controllers/containers/config_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ func (suite *ConfigSuite) TestResolvesMounts() {
128128

129129
asrt.Equal(containers.MountKindHostPath, mounts[3].Kind)
130130
asrt.Equal("/dev", mounts[3].Source)
131-
// Read-only by default.
132-
asrt.Equal([]string{"ro"}, mounts[3].Options)
131+
// Writable by default.
132+
asrt.NotContains(mounts[3].Options, "ro")
133133
})
134134
}
135135

‎internal/app/machined/pkg/controllers/containers/containerd_runner.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,10 @@ func MountsResolvedToOCI(mounts []containersres.ResolvedMountSpec) []specs.Mount
368368
Destination: mount.Destination,
369369
Options: options,
370370
})
371-
case containersres.MountKindHostPath:
371+
// A user volume is a bind of the path the volume is mounted at: by the time a mount reaches
372+
// here, MountController has resolved its source to that path, so the two kinds are the same
373+
// operation and differ only in who decided the source.
374+
case containersres.MountKindHostPath, containersres.MountKindUserVolume:
372375
out = append(out, specs.Mount{
373376
Type: "bind",
374377
Source: mount.Source,

‎internal/app/machined/pkg/controllers/containers/containerd_runner_test.go‎

Lines changed: 63 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ package containers_test
77
import (
88
"testing"
99

10+
specs "github.com/opencontainers/runtime-spec/specs-go"
1011
"github.com/stretchr/testify/assert"
11-
"github.com/stretchr/testify/require"
1212

1313
containersctrl "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/containers"
1414
"github.com/siderolabs/talos/pkg/machinery/resources/containers"
@@ -87,72 +87,96 @@ func TestParseNumericUser(t *testing.T) {
8787
}
8888
}
8989

90+
// TestMountsResolvedToOCI covers the translation of every resolved mount kind.
91+
//
92+
// Every kind has to be handled: a kind that falls through is dropped silently, and the container then
93+
// runs without a mount it declared, which is only visible once something inside it looks for the
94+
// destination.
9095
func TestMountsResolvedToOCI(t *testing.T) {
91-
tests := []struct {
92-
name string
93-
input []containers.ResolvedMountSpec
94-
check func(t *testing.T, mounts any)
96+
t.Parallel()
97+
98+
for _, test := range []struct {
99+
name string
100+
mounts []containers.ResolvedMountSpec
101+
expected []specs.Mount
95102
}{
96103
{
97-
name: "empty mounts",
98-
input: []containers.ResolvedMountSpec{},
99-
check: func(t *testing.T, mounts any) {
100-
assert.Nil(t, mounts)
101-
},
104+
name: "none",
102105
},
103106
{
104-
name: "tmpfs mount",
105-
input: []containers.ResolvedMountSpec{
107+
name: "tmpfs with a size",
108+
mounts: []containers.ResolvedMountSpec{
106109
{
107110
Kind: containers.MountKindTmpfs,
108-
Destination: "/tmp",
109-
Size: 1024 * 1024,
110-
Options: []string{"noexec"},
111+
Destination: "/scratch",
112+
Size: 1024,
111113
},
112114
},
113-
check: func(t *testing.T, mounts any) {
114-
// Just verify it's not nil and has 1 element
115-
require.NotNil(t, mounts)
116-
// Can't easily introspect the mount without importing specs
115+
expected: []specs.Mount{
116+
{
117+
Type: "tmpfs",
118+
Source: "tmpfs",
119+
Destination: "/scratch",
120+
Options: []string{"nosuid", "nodev", "size=1024"},
121+
},
117122
},
118123
},
119124
{
120-
name: "hostpath mount",
121-
input: []containers.ResolvedMountSpec{
125+
name: "host path, read-only",
126+
mounts: []containers.ResolvedMountSpec{
122127
{
123128
Kind: containers.MountKindHostPath,
124-
Source: "/host/path",
125-
Destination: "/container/path",
129+
Source: "/var/log",
130+
Destination: "/host-log",
126131
Options: []string{"ro"},
127132
},
128133
},
129-
check: func(t *testing.T, mounts any) {
130-
require.NotNil(t, mounts)
134+
expected: []specs.Mount{
135+
{
136+
Type: "bind",
137+
Source: "/var/log",
138+
Destination: "/host-log",
139+
Options: []string{"rbind", "ro"},
140+
},
131141
},
132142
},
133143
{
134-
name: "mixed mounts",
135-
input: []containers.ResolvedMountSpec{
144+
// The source is the path the volume is mounted at, filled in by MountController.
145+
name: "user volume",
146+
mounts: []containers.ResolvedMountSpec{
136147
{
137-
Kind: containers.MountKindTmpfs,
138-
Destination: "/tmp",
148+
Kind: containers.MountKindUserVolume,
149+
Source: "/var/mnt/data",
150+
Destination: "/mnt/data",
139151
},
152+
},
153+
expected: []specs.Mount{
140154
{
141-
Kind: containers.MountKindHostPath,
142-
Source: "/host/data",
143-
Destination: "/data",
155+
Type: "bind",
156+
Source: "/var/mnt/data",
157+
Destination: "/mnt/data",
158+
Options: []string{"rbind"},
144159
},
145160
},
146-
check: func(t *testing.T, mounts any) {
147-
require.NotNil(t, mounts)
161+
},
162+
{
163+
name: "all kinds keep their declared order",
164+
mounts: []containers.ResolvedMountSpec{
165+
{Kind: containers.MountKindUserVolume, Source: "/var/mnt/data", Destination: "/mnt/data"},
166+
{Kind: containers.MountKindTmpfs, Destination: "/scratch"},
167+
{Kind: containers.MountKindHostPath, Source: "/var/log", Destination: "/host-log"},
168+
},
169+
expected: []specs.Mount{
170+
{Type: "bind", Source: "/var/mnt/data", Destination: "/mnt/data", Options: []string{"rbind"}},
171+
{Type: "tmpfs", Source: "tmpfs", Destination: "/scratch", Options: []string{"nosuid", "nodev"}},
172+
{Type: "bind", Source: "/var/log", Destination: "/host-log", Options: []string{"rbind"}},
148173
},
149174
},
150-
}
175+
} {
176+
t.Run(test.name, func(t *testing.T) {
177+
t.Parallel()
151178

152-
for _, tt := range tests {
153-
t.Run(tt.name, func(t *testing.T) {
154-
mounts := containersctrl.MountsResolvedToOCI(tt.input)
155-
tt.check(t, mounts)
179+
assert.Equal(t, test.expected, containersctrl.MountsResolvedToOCI(test.mounts))
156180
})
157181
}
158182
}

‎internal/app/machined/pkg/controllers/containers/instance.go‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ func (ctrl *InstanceController) Inputs() []controller.Input {
4646
Type: containers.ContainerImageStatusType,
4747
Kind: controller.InputWeak,
4848
},
49+
{
50+
Namespace: containers.NamespaceName,
51+
Type: containers.ContainerMountStatusType,
52+
Kind: controller.InputWeak,
53+
},
4954
{
5055
Namespace: containers.NamespaceName,
5156
Type: containers.ContainerInstanceStatusType,
@@ -180,21 +185,21 @@ func (ctrl *InstanceController) reconcileInstance(
180185
ctx context.Context,
181186
r controller.Runtime,
182187
logger *zap.Logger,
183-
spec *containers.ContainerSpec,
188+
containerSpec *containers.ContainerSpec,
184189
instances []*containers.ContainerInstanceSpec,
185190
) (optional.Optional[time.Duration], error) {
186191
var currentInstance *containers.ContainerInstanceSpec
187192
if len(instances) > 0 {
188193
currentInstance = instances[len(instances)-1]
189194
}
190195

191-
containerID := spec.Metadata().ID()
196+
containerSpecID := containerSpec.Metadata().ID()
192197

193198
nextGeneration := uint64(0)
194199

195200
// Babysit the existing instance until the spec changes.
196201
if currentInstance != nil {
197-
wasDestroyed, wakeUpAfter, err := ctrl.reconcileExistingInstance(ctx, r, logger, spec, currentInstance)
202+
wasDestroyed, wakeUpAfter, err := ctrl.reconcileExistingInstance(ctx, r, logger, containerSpec, currentInstance)
198203
if err != nil {
199204
return optional.None[time.Duration](), err
200205
}
@@ -207,33 +212,36 @@ func (ctrl *InstanceController) reconcileInstance(
207212
}
208213

209214
// No container exists now, but dependencies may be unmet.
210-
waitingFor, wakeUpAfter, err := spec.TypedSpec().Ready(ctx, r, containerID)
215+
waitingFor, wakeUpAfter, err := containerSpec.TypedSpec().Ready(ctx, r, containerSpecID)
211216
if err != nil {
212217
return optional.None[time.Duration](), err
213218
}
214219

215220
if len(waitingFor) > 0 {
216221
logger.Debug("container is waiting on dependencies",
217-
zap.String("container", containerID),
222+
zap.String("container", containerSpecID),
218223
zap.Strings("waitingFor", waitingFor),
219224
)
220225

221226
return wakeUpAfter, nil
222227
}
223228

224229
// We're good to create a new instance.
225-
imageDigest, err := containers.GetImageDigest(ctx, r, containerID, spec.TypedSpec().Image.Ref)
230+
imageDigest, err := containers.GetImageDigest(ctx, r, containerSpecID, containerSpec.TypedSpec().Image.Ref)
226231
if err != nil {
227232
return optional.None[time.Duration](), err
228233
}
229234

230-
mounts, _ := containers.ResolveInstanceMounts(spec.TypedSpec().Mounts)
235+
resolvedMounts, err := containerSpec.TypedSpec().GetResolvedMounts(ctx, r, containerSpecID)
236+
if err != nil {
237+
return optional.None[time.Duration](), err
238+
}
231239

232-
if err := ctrl.createInstanceSpec(ctx, r, containerID, nextGeneration, spec, imageDigest, mounts); err != nil {
240+
if err := ctrl.createInstanceSpec(ctx, r, containerSpec, nextGeneration, imageDigest, resolvedMounts); err != nil {
233241
return optional.None[time.Duration](), err
234242
}
235243

236-
logger.Info("container instance created", zap.String("container", containerID), zap.Uint64("generation", nextGeneration), zap.String("image", imageDigest))
244+
logger.Info("container instance created", zap.String("container", containerSpecID), zap.Uint64("generation", nextGeneration), zap.String("image", imageDigest))
237245

238246
return optional.None[time.Duration](), nil
239247
}
@@ -410,31 +418,31 @@ func (ctrl *InstanceController) checkRestartDue(
410418
// createInstanceSpec creates a new ContainerInstanceSpec with all fields populated from the spec and resolved values.
411419
func (ctrl *InstanceController) createInstanceSpec(
412420
ctx context.Context,
413-
r controller.Runtime,
414-
containerID string,
421+
runtime controller.Runtime,
422+
containerSpec *containers.ContainerSpec,
415423
generation uint64,
416-
spec *containers.ContainerSpec,
417424
digest string,
418425
mounts []containers.ResolvedMountSpec,
419426
) error {
420-
instanceID := containers.InstanceID(containerID, generation)
427+
containerSpecID := containerSpec.Metadata().ID()
428+
instanceID := containers.InstanceID(containerSpecID, generation)
421429

422-
return safe.WriterModify(ctx, r,
430+
return safe.WriterModify(ctx, runtime,
423431
containers.NewContainerInstanceSpec(containers.NamespaceName, instanceID),
424432
func(res *containers.ContainerInstanceSpec) error {
425433
instanceSpec := res.TypedSpec()
426-
instanceSpec.ContainerID = containerID
434+
instanceSpec.ContainerID = containerSpecID
427435
instanceSpec.Generation = generation
428436
instanceSpec.Image = digest
429-
instanceSpec.Entrypoint = spec.TypedSpec().Entrypoint
430-
instanceSpec.Args = spec.TypedSpec().Args
431-
instanceSpec.WorkingDir = spec.TypedSpec().WorkingDir
432-
instanceSpec.RunAs = spec.TypedSpec().RunAs
433-
instanceSpec.Environment = spec.TypedSpec().Environment
437+
instanceSpec.Entrypoint = containerSpec.TypedSpec().Entrypoint
438+
instanceSpec.Args = containerSpec.TypedSpec().Args
439+
instanceSpec.WorkingDir = containerSpec.TypedSpec().WorkingDir
440+
instanceSpec.RunAs = containerSpec.TypedSpec().RunAs
441+
instanceSpec.Environment = containerSpec.TypedSpec().Environment
434442
instanceSpec.Mounts = mounts
435-
instanceSpec.Security = spec.TypedSpec().Security
436-
instanceSpec.Network = spec.TypedSpec().Network
437-
instanceSpec.Resources = spec.TypedSpec().Resources
443+
instanceSpec.Security = containerSpec.TypedSpec().Security
444+
instanceSpec.Network = containerSpec.TypedSpec().Network
445+
instanceSpec.Resources = containerSpec.TypedSpec().Resources
438446

439447
return nil
440448
},

0 commit comments

Comments
 (0)