Skip to content

Commit 65b3922

Browse files
dmcgowandmcgowan
committed
Split streaming config from runtime config
Signed-off-by: Derek McGowan <[email protected]>
1 parent 58ff9d3 commit 65b3922

18 files changed

+433
-412
lines changed

‎pkg/cri/config/config.go‎

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/containerd/log"
2727
"github.com/pelletier/go-toml/v2"
2828
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
29+
"k8s.io/kubelet/pkg/cri/streaming"
2930

3031
runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
3132
runcoptions "github.com/containerd/containerd/v2/core/runtime/v2/runc/options"
@@ -312,31 +313,18 @@ type ImageConfig struct {
312313
StatsCollectPeriod int `toml:"stats_collect_period" json:"statsCollectPeriod"`
313314
}
314315

315-
// PluginConfig contains toml config related to CRI plugin,
316+
// RuntimeConfig contains toml config related to CRI plugin,
316317
// it is a subset of Config.
317-
type PluginConfig struct {
318+
type RuntimeConfig struct {
318319
// ContainerdConfig contains config related to containerd
319320
ContainerdConfig `toml:"containerd" json:"containerd"`
320321
// CniConfig contains config related to cni
321322
CniConfig `toml:"cni" json:"cni"`
322-
// StreamServerAddress is the ip address streaming server is listening on.
323-
StreamServerAddress string `toml:"stream_server_address" json:"streamServerAddress"`
324-
// StreamServerPort is the port streaming server is listening on.
325-
StreamServerPort string `toml:"stream_server_port" json:"streamServerPort"`
326-
// StreamIdleTimeout is the maximum time a streaming connection
327-
// can be idle before the connection is automatically closed.
328-
// The string is in the golang duration format, see:
329-
// https://golang.org/pkg/time/#ParseDuration
330-
StreamIdleTimeout string `toml:"stream_idle_timeout" json:"streamIdleTimeout"`
331323
// EnableSelinux indicates to enable the selinux support.
332324
EnableSelinux bool `toml:"enable_selinux" json:"enableSelinux"`
333325
// SelinuxCategoryRange allows the upper bound on the category range to be set.
334326
// If not specified or set to 0, defaults to 1024 from the selinux package.
335327
SelinuxCategoryRange int `toml:"selinux_category_range" json:"selinuxCategoryRange"`
336-
// EnableTLSStreaming indicates to enable the TLS streaming support.
337-
EnableTLSStreaming bool `toml:"enable_tls_streaming" json:"enableTLSStreaming"`
338-
// X509KeyPairStreaming is a x509 key pair used for TLS streaming
339-
X509KeyPairStreaming `toml:"x509_key_pair_streaming" json:"x509KeyPairStreaming"`
340328
// MaxContainerLogLineSize is the maximum log line size in bytes for a container.
341329
// Log line longer than the limit will be split into multiple lines. Non-positive
342330
// value means no limit.
@@ -416,10 +404,10 @@ type X509KeyPairStreaming struct {
416404
TLSKeyFile string `toml:"tls_key_file" json:"tlsKeyFile"`
417405
}
418406

419-
// Config contains all configurations for cri server.
407+
// Config contains all configurations for CRI runtime plugin.
420408
type Config struct {
421-
// PluginConfig is the config for CRI plugin.
422-
PluginConfig
409+
// RuntimeConfig is the config for CRI runtime.
410+
RuntimeConfig
423411
// ContainerdRootDir is the root directory path for containerd.
424412
ContainerdRootDir string `json:"containerdRootDir"`
425413
// ContainerdEndpoint is the containerd endpoint path.
@@ -431,10 +419,23 @@ type Config struct {
431419
StateDir string `json:"stateDir"`
432420
}
433421

434-
// ServiceConfig contains all the configuration for the CRI API server.
435-
type ServiceConfig struct {
422+
// ServerConfig contains all the configuration for the CRI API server.
423+
type ServerConfig struct {
436424
// DisableTCPService disables serving CRI on the TCP server.
437425
DisableTCPService bool `toml:"disable_tcp_service" json:"disableTCPService"`
426+
// StreamServerAddress is the ip address streaming server is listening on.
427+
StreamServerAddress string `toml:"stream_server_address" json:"streamServerAddress"`
428+
// StreamServerPort is the port streaming server is listening on.
429+
StreamServerPort string `toml:"stream_server_port" json:"streamServerPort"`
430+
// StreamIdleTimeout is the maximum time a streaming connection
431+
// can be idle before the connection is automatically closed.
432+
// The string is in the golang duration format, see:
433+
// https://golang.org/pkg/time/#ParseDuration
434+
StreamIdleTimeout string `toml:"stream_idle_timeout" json:"streamIdleTimeout"`
435+
// EnableTLSStreaming indicates to enable the TLS streaming support.
436+
EnableTLSStreaming bool `toml:"enable_tls_streaming" json:"enableTLSStreaming"`
437+
// X509KeyPairStreaming is a x509 key pair used for TLS streaming
438+
X509KeyPairStreaming `toml:"x509_key_pair_streaming" json:"x509KeyPairStreaming"`
438439
}
439440

440441
const (
@@ -498,8 +499,8 @@ func ValidateImageConfig(ctx context.Context, c *ImageConfig) ([]deprecation.War
498499
return warnings, nil
499500
}
500501

501-
// ValidatePluginConfig validates the given plugin configuration.
502-
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.Warning, error) {
502+
// ValidateRuntimeConfig validates the given runtime configuration.
503+
func ValidateRuntimeConfig(ctx context.Context, c *RuntimeConfig) ([]deprecation.Warning, error) {
503504
var warnings []deprecation.Warning
504505
if c.ContainerdConfig.Runtimes == nil {
505506
c.ContainerdConfig.Runtimes = make(map[string]Runtime)
@@ -524,13 +525,6 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.W
524525
}
525526
}
526527

527-
// Validation for stream_idle_timeout
528-
if c.StreamIdleTimeout != "" {
529-
if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
530-
return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
531-
}
532-
}
533-
534528
// Validation for drain_exec_sync_io_timeout
535529
if c.DrainExecSyncIOTimeout != "" {
536530
if _, err := time.ParseDuration(c.DrainExecSyncIOTimeout); err != nil {
@@ -543,6 +537,18 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.W
543537
return warnings, nil
544538
}
545539

540+
// ValidateServerConfig validates the given server configuration.
541+
func ValidateServerConfig(ctx context.Context, c *ServerConfig) ([]deprecation.Warning, error) {
542+
var warnings []deprecation.Warning
543+
// Validation for stream_idle_timeout
544+
if c.StreamIdleTimeout != "" {
545+
if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
546+
return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
547+
}
548+
}
549+
return warnings, nil
550+
}
551+
546552
func (config *Config) GetSandboxRuntime(podSandboxConfig *runtime.PodSandboxConfig, runtimeHandler string) (Runtime, error) {
547553
if untrustedWorkload(podSandboxConfig) {
548554
// If the untrusted annotation is provided, runtimeHandler MUST be empty.
@@ -631,3 +637,17 @@ func getRuntimeOptionsType(t string) interface{} {
631637
return &runtimeoptions.Options{}
632638
}
633639
}
640+
641+
func DefaultServerConfig() ServerConfig {
642+
return ServerConfig{
643+
DisableTCPService: true,
644+
StreamServerAddress: "127.0.0.1",
645+
StreamServerPort: "0",
646+
StreamIdleTimeout: streaming.DefaultConfig.StreamIdleTimeout.String(), // 4 hour
647+
EnableTLSStreaming: false,
648+
X509KeyPairStreaming: X509KeyPairStreaming{
649+
TLSKeyFile: "",
650+
TLSCertFile: "",
651+
},
652+
}
653+
}

‎pkg/cri/config/config_kernel_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828

2929
var kernelGreaterEqualThan = kernel.GreaterEqualThan
3030

31-
func ValidateEnableUnprivileged(ctx context.Context, c *PluginConfig) error {
31+
func ValidateEnableUnprivileged(ctx context.Context, c *RuntimeConfig) error {
3232
if c.EnableUnprivilegedICMP || c.EnableUnprivilegedPorts {
3333
fourDotEleven := kernel.KernelVersion{Kernel: 4, Major: 11}
3434
ok, err := kernelGreaterEqualThan(fourDotEleven)

‎pkg/cri/config/config_kernel_linux_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ func TestValidateEnableUnprivileged(t *testing.T) {
3232

3333
tests := []struct {
3434
name string
35-
config *PluginConfig
35+
config *RuntimeConfig
3636
kernelGreater bool
3737
expectedErr string
3838
}{
3939
{
4040
name: "disable unprivileged_icmp and unprivileged_port",
41-
config: &PluginConfig{
41+
config: &RuntimeConfig{
4242
ContainerdConfig: ContainerdConfig{
4343
DefaultRuntimeName: RuntimeDefault,
4444
Runtimes: map[string]Runtime{
@@ -54,7 +54,7 @@ func TestValidateEnableUnprivileged(t *testing.T) {
5454
},
5555
{
5656
name: "enable unprivileged_icmp or unprivileged_port, but kernel version is smaller than 4.11",
57-
config: &PluginConfig{
57+
config: &RuntimeConfig{
5858
ContainerdConfig: ContainerdConfig{
5959
DefaultRuntimeName: RuntimeDefault,
6060
Runtimes: map[string]Runtime{
@@ -71,7 +71,7 @@ func TestValidateEnableUnprivileged(t *testing.T) {
7171
},
7272
{
7373
name: "enable unprivileged_icmp or unprivileged_port, but kernel version is greater than or equal 4.11",
74-
config: &PluginConfig{
74+
config: &RuntimeConfig{
7575
ContainerdConfig: ContainerdConfig{
7676
DefaultRuntimeName: RuntimeDefault,
7777
Runtimes: map[string]Runtime{

‎pkg/cri/config/config_kernel_other.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,6 @@ import (
2222
"context"
2323
)
2424

25-
func ValidateEnableUnprivileged(ctx context.Context, c *PluginConfig) error {
25+
func ValidateEnableUnprivileged(ctx context.Context, c *RuntimeConfig) error {
2626
return nil
2727
}

‎pkg/cri/config/config_test.go‎

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -28,37 +28,40 @@ import (
2828

2929
func TestValidateConfig(t *testing.T) {
3030
for desc, test := range map[string]struct {
31-
config *PluginConfig
32-
expectedErr string
33-
expected *PluginConfig
34-
imageConfig *ImageConfig
35-
imageExpectedErr string
36-
imageExpected *ImageConfig
37-
warnings []deprecation.Warning
31+
runtimeConfig *RuntimeConfig
32+
runtimeExpectedErr string
33+
runtimeExpected *RuntimeConfig
34+
imageConfig *ImageConfig
35+
imageExpectedErr string
36+
imageExpected *ImageConfig
37+
serverConfig *ServerConfig
38+
serverExpectedErr string
39+
serverExpected *ServerConfig
40+
warnings []deprecation.Warning
3841
}{
3942
"no default_runtime_name": {
40-
config: &PluginConfig{},
41-
expectedErr: "`default_runtime_name` is empty",
43+
runtimeConfig: &RuntimeConfig{},
44+
runtimeExpectedErr: "`default_runtime_name` is empty",
4245
},
4346
"no runtime[default_runtime_name]": {
44-
config: &PluginConfig{
47+
runtimeConfig: &RuntimeConfig{
4548
ContainerdConfig: ContainerdConfig{
4649
DefaultRuntimeName: RuntimeDefault,
4750
},
4851
},
49-
expectedErr: "no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"default\"",
52+
runtimeExpectedErr: "no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"default\"",
5053
},
5154

5255
"deprecated auths": {
53-
config: &PluginConfig{
56+
runtimeConfig: &RuntimeConfig{
5457
ContainerdConfig: ContainerdConfig{
5558
DefaultRuntimeName: RuntimeDefault,
5659
Runtimes: map[string]Runtime{
5760
RuntimeDefault: {},
5861
},
5962
},
6063
},
61-
expected: &PluginConfig{
64+
runtimeExpected: &RuntimeConfig{
6265
ContainerdConfig: ContainerdConfig{
6366
DefaultRuntimeName: RuntimeDefault,
6467
Runtimes: map[string]Runtime{
@@ -92,18 +95,10 @@ func TestValidateConfig(t *testing.T) {
9295
warnings: []deprecation.Warning{deprecation.CRIRegistryAuths},
9396
},
9497
"invalid stream_idle_timeout": {
95-
config: &PluginConfig{
98+
serverConfig: &ServerConfig{
9699
StreamIdleTimeout: "invalid",
97-
ContainerdConfig: ContainerdConfig{
98-
DefaultRuntimeName: RuntimeDefault,
99-
Runtimes: map[string]Runtime{
100-
RuntimeDefault: {
101-
Type: "default",
102-
},
103-
},
104-
},
105100
},
106-
expectedErr: "invalid stream idle timeout",
101+
serverExpectedErr: "invalid stream idle timeout",
107102
},
108103
"conflicting mirror registry config": {
109104
imageConfig: &ImageConfig{
@@ -117,7 +112,7 @@ func TestValidateConfig(t *testing.T) {
117112
imageExpectedErr: "`mirrors` cannot be set when `config_path` is provided",
118113
},
119114
"deprecated mirrors": {
120-
config: &PluginConfig{
115+
runtimeConfig: &RuntimeConfig{
121116
ContainerdConfig: ContainerdConfig{
122117
DefaultRuntimeName: RuntimeDefault,
123118
Runtimes: map[string]Runtime{
@@ -132,7 +127,7 @@ func TestValidateConfig(t *testing.T) {
132127
},
133128
},
134129
},
135-
expected: &PluginConfig{
130+
runtimeExpected: &RuntimeConfig{
136131
ContainerdConfig: ContainerdConfig{
137132
DefaultRuntimeName: RuntimeDefault,
138133
Runtimes: map[string]Runtime{
@@ -152,7 +147,7 @@ func TestValidateConfig(t *testing.T) {
152147
warnings: []deprecation.Warning{deprecation.CRIRegistryMirrors},
153148
},
154149
"deprecated configs": {
155-
config: &PluginConfig{
150+
runtimeConfig: &RuntimeConfig{
156151
ContainerdConfig: ContainerdConfig{
157152
DefaultRuntimeName: RuntimeDefault,
158153
Runtimes: map[string]Runtime{
@@ -171,7 +166,7 @@ func TestValidateConfig(t *testing.T) {
171166
},
172167
},
173168
},
174-
expected: &PluginConfig{
169+
runtimeExpected: &RuntimeConfig{
175170
ContainerdConfig: ContainerdConfig{
176171
DefaultRuntimeName: RuntimeDefault,
177172
Runtimes: map[string]Runtime{
@@ -195,7 +190,7 @@ func TestValidateConfig(t *testing.T) {
195190
warnings: []deprecation.Warning{deprecation.CRIRegistryConfigs},
196191
},
197192
"privileged_without_host_devices_all_devices_allowed without privileged_without_host_devices": {
198-
config: &PluginConfig{
193+
runtimeConfig: &RuntimeConfig{
199194
ContainerdConfig: ContainerdConfig{
200195
DefaultRuntimeName: RuntimeDefault,
201196
Runtimes: map[string]Runtime{
@@ -207,10 +202,10 @@ func TestValidateConfig(t *testing.T) {
207202
},
208203
},
209204
},
210-
expectedErr: "`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled",
205+
runtimeExpectedErr: "`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled",
211206
},
212207
"invalid drain_exec_sync_io_timeout input": {
213-
config: &PluginConfig{
208+
runtimeConfig: &RuntimeConfig{
214209
ContainerdConfig: ContainerdConfig{
215210
DefaultRuntimeName: RuntimeDefault,
216211
Runtimes: map[string]Runtime{
@@ -221,18 +216,18 @@ func TestValidateConfig(t *testing.T) {
221216
},
222217
DrainExecSyncIOTimeout: "10",
223218
},
224-
expectedErr: "invalid `drain_exec_sync_io_timeout`",
219+
runtimeExpectedErr: "invalid `drain_exec_sync_io_timeout`",
225220
},
226221
} {
227222
t.Run(desc, func(t *testing.T) {
228223
var warnings []deprecation.Warning
229-
if test.config != nil {
230-
w, err := ValidatePluginConfig(context.Background(), test.config)
231-
if test.expectedErr != "" {
232-
assert.Contains(t, err.Error(), test.expectedErr)
224+
if test.runtimeConfig != nil {
225+
w, err := ValidateRuntimeConfig(context.Background(), test.runtimeConfig)
226+
if test.runtimeExpectedErr != "" {
227+
assert.Contains(t, err.Error(), test.runtimeExpectedErr)
233228
} else {
234229
assert.NoError(t, err)
235-
assert.Equal(t, test.expected, test.config)
230+
assert.Equal(t, test.runtimeExpected, test.runtimeConfig)
236231
}
237232
warnings = append(warnings, w...)
238233
}
@@ -246,6 +241,16 @@ func TestValidateConfig(t *testing.T) {
246241
}
247242
warnings = append(warnings, w...)
248243
}
244+
if test.serverConfig != nil {
245+
w, err := ValidateServerConfig(context.Background(), test.serverConfig)
246+
if test.serverExpectedErr != "" {
247+
assert.Contains(t, err.Error(), test.serverExpectedErr)
248+
} else {
249+
assert.NoError(t, err)
250+
assert.Equal(t, test.serverExpected, test.serverConfig)
251+
}
252+
warnings = append(warnings, w...)
253+
}
249254

250255
if len(test.warnings) > 0 {
251256
assert.ElementsMatch(t, test.warnings, warnings)

0 commit comments

Comments
 (0)