Skip to content

Commit 1fbce40

Browse files
samuelkarpsamuelkarp
committed
cri: add ability to emit deprecation warnings
Signed-off-by: Samuel Karp <[email protected]> (cherry picked from commit 58cc275) Signed-off-by: Samuel Karp <[email protected]>
1 parent 311ec86 commit 1fbce40

3 files changed

Lines changed: 45 additions & 24 deletions

File tree

‎pkg/cri/config/config.go‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"time"
2525

2626
"github.com/containerd/containerd/log"
27+
"github.com/containerd/containerd/pkg/deprecation"
2728
"github.com/containerd/containerd/plugin"
2829
)
2930

@@ -396,7 +397,8 @@ const (
396397
)
397398

398399
// ValidatePluginConfig validates the given plugin configuration.
399-
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
400+
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.Warning, error) {
401+
var warnings []deprecation.Warning
400402
if c.ContainerdConfig.Runtimes == nil {
401403
c.ContainerdConfig.Runtimes = make(map[string]Runtime)
402404
}
@@ -405,7 +407,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
405407
if c.ContainerdConfig.UntrustedWorkloadRuntime.Type != "" {
406408
log.G(ctx).Warning("`untrusted_workload_runtime` is deprecated, please use `untrusted` runtime in `runtimes` instead")
407409
if _, ok := c.ContainerdConfig.Runtimes[RuntimeUntrusted]; ok {
408-
return fmt.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted)
410+
return warnings, fmt.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted)
409411
}
410412
c.ContainerdConfig.Runtimes[RuntimeUntrusted] = c.ContainerdConfig.UntrustedWorkloadRuntime
411413
}
@@ -419,41 +421,41 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
419421

420422
// Validation for default_runtime_name
421423
if c.ContainerdConfig.DefaultRuntimeName == "" {
422-
return errors.New("`default_runtime_name` is empty")
424+
return warnings, errors.New("`default_runtime_name` is empty")
423425
}
424426
if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
425-
return fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
427+
return warnings, fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
426428
}
427429

428430
// Validation for deprecated runtime options.
429431
if c.SystemdCgroup {
430432
if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
431-
return fmt.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
433+
return warnings, fmt.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
432434
}
433435
log.G(ctx).Warning("`systemd_cgroup` is deprecated, please use runtime `options` instead")
434436
}
435437
if c.NoPivot {
436438
if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
437-
return fmt.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
439+
return warnings, fmt.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
438440
}
439441
// NoPivot can't be deprecated yet, because there is no alternative config option
440442
// for `io.containerd.runtime.v1.linux`.
441443
}
442444
for k, r := range c.ContainerdConfig.Runtimes {
443445
if r.Engine != "" {
444446
if r.Type != plugin.RuntimeLinuxV1 {
445-
return fmt.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
447+
return warnings, fmt.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
446448
}
447449
log.G(ctx).Warning("`runtime_engine` is deprecated, please use runtime `options` instead")
448450
}
449451
if r.Root != "" {
450452
if r.Type != plugin.RuntimeLinuxV1 {
451-
return fmt.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
453+
return warnings, fmt.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
452454
}
453455
log.G(ctx).Warning("`runtime_root` is deprecated, please use runtime `options` instead")
454456
}
455457
if !r.PrivilegedWithoutHostDevices && r.PrivilegedWithoutHostDevicesAllDevicesAllowed {
456-
return errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
458+
return warnings, errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
457459
}
458460
// If empty, use default podSandbox mode
459461
if len(r.SandboxMode) == 0 {
@@ -465,7 +467,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
465467
useConfigPath := c.Registry.ConfigPath != ""
466468
if len(c.Registry.Mirrors) > 0 {
467469
if useConfigPath {
468-
return errors.New("`mirrors` cannot be set when `config_path` is provided")
470+
return warnings, errors.New("`mirrors` cannot be set when `config_path` is provided")
469471
}
470472
log.G(ctx).Warning("`mirrors` is deprecated, please use `config_path` instead")
471473
}
@@ -478,7 +480,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
478480
}
479481
if hasDeprecatedTLS {
480482
if useConfigPath {
481-
return errors.New("`configs.tls` cannot be set when `config_path` is provided")
483+
return warnings, errors.New("`configs.tls` cannot be set when `config_path` is provided")
482484
}
483485
log.G(ctx).Warning("`configs.tls` is deprecated, please use `config_path` instead")
484486
}
@@ -492,7 +494,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
492494
auth := auth
493495
u, err := url.Parse(endpoint)
494496
if err != nil {
495-
return fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
497+
return warnings, fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
496498
}
497499
if u.Scheme != "" {
498500
// Do not include the scheme in the new registry config.
@@ -508,22 +510,22 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
508510
// Validation for stream_idle_timeout
509511
if c.StreamIdleTimeout != "" {
510512
if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
511-
return fmt.Errorf("invalid stream idle timeout: %w", err)
513+
return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
512514
}
513515
}
514516

515517
// Validation for image_pull_progress_timeout
516518
if c.ImagePullProgressTimeout != "" {
517519
if _, err := time.ParseDuration(c.ImagePullProgressTimeout); err != nil {
518-
return fmt.Errorf("invalid image pull progress timeout: %w", err)
520+
return warnings, fmt.Errorf("invalid image pull progress timeout: %w", err)
519521
}
520522
}
521523

522524
// Validation for drain_exec_sync_io_timeout
523525
if c.DrainExecSyncIOTimeout != "" {
524526
if _, err := time.ParseDuration(c.DrainExecSyncIOTimeout); err != nil {
525-
return fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
527+
return warnings, fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
526528
}
527529
}
528-
return nil
530+
return warnings, nil
529531
}

‎pkg/cri/config/config_test.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,16 @@ import (
2323

2424
"github.com/containerd/containerd/plugin"
2525
"github.com/stretchr/testify/assert"
26+
27+
"github.com/containerd/containerd/pkg/deprecation"
2628
)
2729

2830
func TestValidateConfig(t *testing.T) {
2931
for desc, test := range map[string]struct {
3032
config *PluginConfig
3133
expectedErr string
3234
expected *PluginConfig
35+
warnings []deprecation.Warning
3336
}{
3437
"deprecated untrusted_workload_runtime": {
3538
config: &PluginConfig{
@@ -399,13 +402,18 @@ func TestValidateConfig(t *testing.T) {
399402
},
400403
} {
401404
t.Run(desc, func(t *testing.T) {
402-
err := ValidatePluginConfig(context.Background(), test.config)
405+
w, err := ValidatePluginConfig(context.Background(), test.config)
403406
if test.expectedErr != "" {
404407
assert.Contains(t, err.Error(), test.expectedErr)
405408
} else {
406409
assert.NoError(t, err)
407410
assert.Equal(t, test.expected, test.config)
408411
}
412+
if len(test.warnings) > 0 {
413+
assert.ElementsMatch(t, test.warnings, w)
414+
} else {
415+
assert.Len(t, w, 0)
416+
}
409417
})
410418
}
411419
}

‎pkg/cri/cri.go‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,20 @@ import (
2222
"os"
2323
"path/filepath"
2424

25+
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
26+
"k8s.io/klog/v2"
27+
2528
"github.com/containerd/containerd"
2629
"github.com/containerd/containerd/log"
30+
criconfig "github.com/containerd/containerd/pkg/cri/config"
31+
"github.com/containerd/containerd/pkg/cri/constants"
2732
"github.com/containerd/containerd/pkg/cri/nri"
2833
"github.com/containerd/containerd/pkg/cri/sbserver"
34+
"github.com/containerd/containerd/pkg/cri/server"
2935
nriservice "github.com/containerd/containerd/pkg/nri"
3036
"github.com/containerd/containerd/platforms"
3137
"github.com/containerd/containerd/plugin"
32-
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
33-
"k8s.io/klog/v2"
34-
35-
criconfig "github.com/containerd/containerd/pkg/cri/config"
36-
"github.com/containerd/containerd/pkg/cri/constants"
37-
"github.com/containerd/containerd/pkg/cri/server"
38+
"github.com/containerd/containerd/services/warning"
3839
)
3940

4041
// Register CRI service plugin
@@ -48,6 +49,7 @@ func init() {
4849
plugin.EventPlugin,
4950
plugin.ServicePlugin,
5051
plugin.NRIApiPlugin,
52+
plugin.WarningPlugin,
5153
},
5254
InitFn: initCRIService,
5355
})
@@ -58,8 +60,17 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
5860
ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion, "CRIVersionAlpha": constants.CRIVersionAlpha}
5961
ctx := ic.Context
6062
pluginConfig := ic.Config.(*criconfig.PluginConfig)
61-
if err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
63+
if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
6264
return nil, fmt.Errorf("invalid plugin config: %w", err)
65+
} else if len(warnings) > 0 {
66+
ws, err := ic.Get(plugin.WarningPlugin)
67+
if err != nil {
68+
return nil, err
69+
}
70+
warn := ws.(warning.Service)
71+
for _, w := range warnings {
72+
warn.Emit(ctx, w)
73+
}
6374
}
6475

6576
c := criconfig.Config{

0 commit comments

Comments
 (0)