Skip to content

Commit 58cc275

Browse files
samuelkarpsamuelkarp
committed
cri: add ability to emit deprecation warnings
Signed-off-by: Samuel Karp <[email protected]>
1 parent 6cd0e8e commit 58cc275

File tree

3 files changed

+40
-18
lines changed

3 files changed

+40
-18
lines changed

‎pkg/cri/config/config.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"time"
2525

2626
"github.com/containerd/log"
27+
28+
"github.com/containerd/containerd/v2/pkg/deprecation"
2729
)
2830

2931
type SandboxControllerMode string
@@ -365,22 +367,23 @@ const (
365367
)
366368

367369
// ValidatePluginConfig validates the given plugin configuration.
368-
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
370+
func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.Warning, error) {
371+
var warnings []deprecation.Warning
369372
if c.ContainerdConfig.Runtimes == nil {
370373
c.ContainerdConfig.Runtimes = make(map[string]Runtime)
371374
}
372375

373376
// Validation for default_runtime_name
374377
if c.ContainerdConfig.DefaultRuntimeName == "" {
375-
return errors.New("`default_runtime_name` is empty")
378+
return warnings, errors.New("`default_runtime_name` is empty")
376379
}
377380
if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
378-
return fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
381+
return warnings, fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
379382
}
380383

381384
for k, r := range c.ContainerdConfig.Runtimes {
382385
if !r.PrivilegedWithoutHostDevices && r.PrivilegedWithoutHostDevicesAllDevicesAllowed {
383-
return errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
386+
return warnings, errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
384387
}
385388
// If empty, use default podSandbox mode
386389
if len(r.Sandboxer) == 0 {
@@ -392,7 +395,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
392395
useConfigPath := c.Registry.ConfigPath != ""
393396
if len(c.Registry.Mirrors) > 0 {
394397
if useConfigPath {
395-
return errors.New("`mirrors` cannot be set when `config_path` is provided")
398+
return warnings, errors.New("`mirrors` cannot be set when `config_path` is provided")
396399
}
397400
log.G(ctx).Warning("`mirrors` is deprecated, please use `config_path` instead")
398401
}
@@ -406,7 +409,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
406409
auth := auth
407410
u, err := url.Parse(endpoint)
408411
if err != nil {
409-
return fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
412+
return warnings, fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
410413
}
411414
if u.Scheme != "" {
412415
// Do not include the scheme in the new registry config.
@@ -422,22 +425,22 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
422425
// Validation for stream_idle_timeout
423426
if c.StreamIdleTimeout != "" {
424427
if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
425-
return fmt.Errorf("invalid stream idle timeout: %w", err)
428+
return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
426429
}
427430
}
428431

429432
// Validation for image_pull_progress_timeout
430433
if c.ImagePullProgressTimeout != "" {
431434
if _, err := time.ParseDuration(c.ImagePullProgressTimeout); err != nil {
432-
return fmt.Errorf("invalid image pull progress timeout: %w", err)
435+
return warnings, fmt.Errorf("invalid image pull progress timeout: %w", err)
433436
}
434437
}
435438

436439
// Validation for drain_exec_sync_io_timeout
437440
if c.DrainExecSyncIOTimeout != "" {
438441
if _, err := time.ParseDuration(c.DrainExecSyncIOTimeout); err != nil {
439-
return fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
442+
return warnings, fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
440443
}
441444
}
442-
return nil
445+
return warnings, nil
443446
}

‎pkg/cri/config/config_test.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@ import (
2121
"testing"
2222

2323
"github.com/stretchr/testify/assert"
24+
25+
"github.com/containerd/containerd/v2/pkg/deprecation"
2426
)
2527

2628
func TestValidateConfig(t *testing.T) {
2729
for desc, test := range map[string]struct {
2830
config *PluginConfig
2931
expectedErr string
3032
expected *PluginConfig
33+
warnings []deprecation.Warning
3134
}{
3235
"no default_runtime_name": {
3336
config: &PluginConfig{},
@@ -143,13 +146,18 @@ func TestValidateConfig(t *testing.T) {
143146
},
144147
} {
145148
t.Run(desc, func(t *testing.T) {
146-
err := ValidatePluginConfig(context.Background(), test.config)
149+
w, err := ValidatePluginConfig(context.Background(), test.config)
147150
if test.expectedErr != "" {
148151
assert.Contains(t, err.Error(), test.expectedErr)
149152
} else {
150153
assert.NoError(t, err)
151154
assert.Equal(t, test.expected, test.config)
152155
}
156+
if len(test.warnings) > 0 {
157+
assert.ElementsMatch(t, test.warnings, w)
158+
} else {
159+
assert.Len(t, w, 0)
160+
}
153161
})
154162
}
155163
}

‎pkg/cri/cri.go‎

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

24-
containerd "github.com/containerd/containerd/v2/client"
25-
"github.com/containerd/containerd/v2/pkg/cri/nri"
26-
"github.com/containerd/containerd/v2/pkg/cri/server"
27-
nriservice "github.com/containerd/containerd/v2/pkg/nri"
28-
"github.com/containerd/containerd/v2/platforms"
29-
"github.com/containerd/containerd/v2/plugins"
3024
"github.com/containerd/log"
3125
"github.com/containerd/plugin"
3226
"github.com/containerd/plugin/registry"
3327
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
3428
"k8s.io/klog/v2"
3529

30+
containerd "github.com/containerd/containerd/v2/client"
3631
criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
3732
"github.com/containerd/containerd/v2/pkg/cri/constants"
33+
"github.com/containerd/containerd/v2/pkg/cri/nri"
34+
"github.com/containerd/containerd/v2/pkg/cri/server"
35+
nriservice "github.com/containerd/containerd/v2/pkg/nri"
36+
"github.com/containerd/containerd/v2/platforms"
37+
"github.com/containerd/containerd/v2/plugins"
38+
"github.com/containerd/containerd/v2/services/warning"
3839
)
3940

4041
// Register CRI service plugin
@@ -48,6 +49,7 @@ func init() {
4849
plugins.EventPlugin,
4950
plugins.ServicePlugin,
5051
plugins.NRIApiPlugin,
52+
plugins.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}
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.GetSingle(plugins.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)