Skip to content

Commit 878823f

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 5a014ed commit 878823f

3 files changed

Lines changed: 43 additions & 22 deletions

File tree

‎pkg/cri/config/config.go‎

Lines changed: 15 additions & 13 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

@@ -338,7 +339,8 @@ const (
338339
)
339340

340341
// ValidatePluginConfig validates the given plugin configuration.
341-
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
342+
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.Warning, error) {
343+
var warnings []deprecation.Warning
342344
if c.ContainerdConfig.Runtimes == nil {
343345
c.ContainerdConfig.Runtimes = make(map[string]Runtime)
344346
}
@@ -347,7 +349,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
347349
if c.ContainerdConfig.UntrustedWorkloadRuntime.Type != "" {
348350
log.G(ctx).Warning("`untrusted_workload_runtime` is deprecated, please use `untrusted` runtime in `runtimes` instead")
349351
if _, ok := c.ContainerdConfig.Runtimes[RuntimeUntrusted]; ok {
350-
return fmt.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted)
352+
return warnings, fmt.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted)
351353
}
352354
c.ContainerdConfig.Runtimes[RuntimeUntrusted] = c.ContainerdConfig.UntrustedWorkloadRuntime
353355
}
@@ -361,36 +363,36 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
361363

362364
// Validation for default_runtime_name
363365
if c.ContainerdConfig.DefaultRuntimeName == "" {
364-
return errors.New("`default_runtime_name` is empty")
366+
return warnings, errors.New("`default_runtime_name` is empty")
365367
}
366368
if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
367-
return fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
369+
return warnings, fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
368370
}
369371

370372
// Validation for deprecated runtime options.
371373
if c.SystemdCgroup {
372374
if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
373-
return fmt.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
375+
return warnings, fmt.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
374376
}
375377
log.G(ctx).Warning("`systemd_cgroup` is deprecated, please use runtime `options` instead")
376378
}
377379
if c.NoPivot {
378380
if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
379-
return fmt.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
381+
return warnings, fmt.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
380382
}
381383
// NoPivot can't be deprecated yet, because there is no alternative config option
382384
// for `io.containerd.runtime.v1.linux`.
383385
}
384386
for _, r := range c.ContainerdConfig.Runtimes {
385387
if r.Engine != "" {
386388
if r.Type != plugin.RuntimeLinuxV1 {
387-
return fmt.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
389+
return warnings, fmt.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
388390
}
389391
log.G(ctx).Warning("`runtime_engine` is deprecated, please use runtime `options` instead")
390392
}
391393
if r.Root != "" {
392394
if r.Type != plugin.RuntimeLinuxV1 {
393-
return fmt.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
395+
return warnings, fmt.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
394396
}
395397
log.G(ctx).Warning("`runtime_root` is deprecated, please use runtime `options` instead")
396398
}
@@ -399,7 +401,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
399401
useConfigPath := c.Registry.ConfigPath != ""
400402
if len(c.Registry.Mirrors) > 0 {
401403
if useConfigPath {
402-
return errors.New("`mirrors` cannot be set when `config_path` is provided")
404+
return warnings, errors.New("`mirrors` cannot be set when `config_path` is provided")
403405
}
404406
log.G(ctx).Warning("`mirrors` is deprecated, please use `config_path` instead")
405407
}
@@ -412,7 +414,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
412414
}
413415
if hasDeprecatedTLS {
414416
if useConfigPath {
415-
return errors.New("`configs.tls` cannot be set when `config_path` is provided")
417+
return warnings, errors.New("`configs.tls` cannot be set when `config_path` is provided")
416418
}
417419
log.G(ctx).Warning("`configs.tls` is deprecated, please use `config_path` instead")
418420
}
@@ -426,7 +428,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
426428
auth := auth
427429
u, err := url.Parse(endpoint)
428430
if err != nil {
429-
return fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
431+
return warnings, fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
430432
}
431433
if u.Scheme != "" {
432434
// Do not include the scheme in the new registry config.
@@ -442,8 +444,8 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
442444
// Validation for stream_idle_timeout
443445
if c.StreamIdleTimeout != "" {
444446
if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
445-
return fmt.Errorf("invalid stream idle timeout: %w", err)
447+
return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
446448
}
447449
}
448-
return nil
450+
return warnings, nil
449451
}

‎pkg/cri/config/config_test.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,18 @@ import (
2121
"fmt"
2222
"testing"
2323

24-
"github.com/containerd/containerd/plugin"
2524
"github.com/stretchr/testify/assert"
25+
26+
"github.com/containerd/containerd/pkg/deprecation"
27+
"github.com/containerd/containerd/plugin"
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{
@@ -362,13 +365,18 @@ func TestValidateConfig(t *testing.T) {
362365
},
363366
} {
364367
t.Run(desc, func(t *testing.T) {
365-
err := ValidatePluginConfig(context.Background(), test.config)
368+
w, err := ValidatePluginConfig(context.Background(), test.config)
366369
if test.expectedErr != "" {
367370
assert.Contains(t, err.Error(), test.expectedErr)
368371
} else {
369372
assert.NoError(t, err)
370373
assert.Equal(t, test.expected, test.config)
371374
}
375+
if len(test.warnings) > 0 {
376+
assert.ElementsMatch(t, test.warnings, w)
377+
} else {
378+
assert.Len(t, w, 0)
379+
}
372380
})
373381
}
374382
}

‎pkg/cri/cri.go‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import (
2121
"fmt"
2222
"path/filepath"
2323

24+
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
25+
"k8s.io/klog/v2"
26+
2427
"github.com/containerd/containerd"
2528
"github.com/containerd/containerd/api/services/containers/v1"
2629
"github.com/containerd/containerd/api/services/diff/v1"
@@ -31,16 +34,14 @@ import (
3134
"github.com/containerd/containerd/content"
3235
"github.com/containerd/containerd/leases"
3336
"github.com/containerd/containerd/log"
37+
criconfig "github.com/containerd/containerd/pkg/cri/config"
38+
"github.com/containerd/containerd/pkg/cri/constants"
39+
"github.com/containerd/containerd/pkg/cri/server"
3440
"github.com/containerd/containerd/platforms"
3541
"github.com/containerd/containerd/plugin"
3642
"github.com/containerd/containerd/services"
43+
"github.com/containerd/containerd/services/warning"
3744
"github.com/containerd/containerd/snapshots"
38-
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
39-
"k8s.io/klog/v2"
40-
41-
criconfig "github.com/containerd/containerd/pkg/cri/config"
42-
"github.com/containerd/containerd/pkg/cri/constants"
43-
"github.com/containerd/containerd/pkg/cri/server"
4445
)
4546

4647
// Register CRI service plugin
@@ -53,6 +54,7 @@ func init() {
5354
Requires: []plugin.Type{
5455
plugin.EventPlugin,
5556
plugin.ServicePlugin,
57+
plugin.WarningPlugin,
5658
},
5759
InitFn: initCRIService,
5860
})
@@ -63,8 +65,17 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
6365
ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion, "CRIVersionAlpha": constants.CRIVersionAlpha}
6466
ctx := ic.Context
6567
pluginConfig := ic.Config.(*criconfig.PluginConfig)
66-
if err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
68+
if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
6769
return nil, fmt.Errorf("invalid plugin config: %w", err)
70+
} else if len(warnings) > 0 {
71+
ws, err := ic.Get(plugin.WarningPlugin)
72+
if err != nil {
73+
return nil, err
74+
}
75+
warn := ws.(warning.Service)
76+
for _, w := range warnings {
77+
warn.Emit(ctx, w)
78+
}
6879
}
6980

7081
c := criconfig.Config{

0 commit comments

Comments
 (0)