Merge pull request #9336 from ruiwen-zhao/cri-warning

[release/1.7] Expose usage of cri-api v1alpha2

contrib/fuzz/cri_sbserver_fuzzer.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containerd/containerd"
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
 	"github.com/containerd/containerd/pkg/cri/sbserver"
+	"github.com/containerd/containerd/pkg/cri/server/testing"
 )
 
 func FuzzCRISandboxServer(data []byte) int {
@@ -37,7 +38,7 @@ func FuzzCRISandboxServer(data []byte) int {
 	}
 	defer client.Close()
 
-	c, err := sbserver.NewCRIService(criconfig.Config{}, client, nil)
+	c, err := sbserver.NewCRIService(criconfig.Config{}, client, nil, testing.NewFakeWarningService())
 	if err != nil {
 		panic(err)
 	}

contrib/fuzz/cri_server_fuzzer.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containerd/containerd"
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
 	"github.com/containerd/containerd/pkg/cri/server"
+	"github.com/containerd/containerd/pkg/cri/server/testing"
 )
 
 func FuzzCRIServer(data []byte) int {
@@ -37,7 +38,7 @@ func FuzzCRIServer(data []byte) int {
 	}
 	defer client.Close()
 
-	c, err := server.NewCRIService(criconfig.Config{}, client, nil)
+	c, err := server.NewCRIService(criconfig.Config{}, client, nil, testing.NewFakeWarningService())
 	if err != nil {
 		panic(err)
 	}

integration/image_pull_timeout_test.go

@@ -41,6 +41,7 @@ import (
 	"github.com/containerd/containerd/namespaces"
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
 	criserver "github.com/containerd/containerd/pkg/cri/server"
+	servertesting "github.com/containerd/containerd/pkg/cri/server/testing"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
@@ -471,5 +472,5 @@ func initLocalCRIPlugin(client *containerd.Client, tmpDir string, registryCfg cr
 		RootDir:           filepath.Join(criWorkDir, "root"),
 		StateDir:          filepath.Join(criWorkDir, "state"),
 	}
-	return criserver.NewCRIService(cfg, client, nil)
+	return criserver.NewCRIService(cfg, client, nil, servertesting.NewFakeWarningService())
 }

pkg/cri/cri.go

@@ -60,14 +60,15 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion, "CRIVersionAlpha": constants.CRIVersionAlpha}
 	ctx := ic.Context
 	pluginConfig := ic.Config.(*criconfig.PluginConfig)
+	ws, err := ic.Get(plugin.WarningPlugin)
+	if err != nil {
+		return nil, err
+	}
+	warn := ws.(warning.Service)
+
 	if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
 		return nil, fmt.Errorf("invalid plugin config: %w", err)
 	} else if len(warnings) > 0 {
-		ws, err := ic.Get(plugin.WarningPlugin)
-		if err != nil {
-			return nil, err
-		}
-		warn := ws.(warning.Service)
 		for _, w := range warnings {
 			warn.Emit(ctx, w)
 		}
@@ -100,10 +101,10 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	var s server.CRIService
 	if os.Getenv("ENABLE_CRI_SANDBOXES") != "" {
 		log.G(ctx).Info("using experimental CRI Sandbox server - unset ENABLE_CRI_SANDBOXES to disable")
-		s, err = sbserver.NewCRIService(c, client, getNRIAPI(ic))
+		s, err = sbserver.NewCRIService(c, client, getNRIAPI(ic), warn)
 	} else {
 		log.G(ctx).Info("using legacy CRI server")
-		s, err = server.NewCRIService(c, client, getNRIAPI(ic))
+		s, err = server.NewCRIService(c, client, getNRIAPI(ic), warn)
 	}
 	if err != nil {
 		return nil, fmt.Errorf("failed to create CRI service: %w", err)

pkg/cri/instrument/instrumented_service.go

@@ -19,14 +19,17 @@ package instrument
 import (
 	"context"
 	"errors"
+	"sync"
 
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
+	"github.com/containerd/containerd/services/warning"
 	runtime_alpha "github.com/containerd/containerd/third_party/k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	"github.com/containerd/containerd/tracing"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
 	ctrdutil "github.com/containerd/containerd/pkg/cri/util"
+	"github.com/containerd/containerd/pkg/deprecation"
 )
 
 const (
@@ -69,10 +72,12 @@ type instrumentedAlphaService struct {
 	c criService
 	runtime_alpha.UnimplementedRuntimeServiceServer
 	runtime_alpha.UnimplementedImageServiceServer
+	warn        warning.Service
+	emitWarning sync.Once
 }
 
-func NewAlphaService(c criService) GRPCAlphaServices {
-	return &instrumentedAlphaService{c: c}
+func NewAlphaService(c criService, warn warning.Service) GRPCAlphaServices {
+	return &instrumentedAlphaService{c: c, warn: warn}
 }
 
 // checkInitialized returns error if the server is not fully initialized.
@@ -1486,6 +1491,13 @@ func (in *instrumentedService) Status(ctx context.Context, r *runtime.StatusRequ
 }
 
 func (in *instrumentedAlphaService) Status(ctx context.Context, r *runtime_alpha.StatusRequest) (res *runtime_alpha.StatusResponse, err error) {
+	// Only emit the warning the first time an v1alpha2 api is called
+	in.emitWarning.Do(func() {
+		log.G(ctx).Warning("CRI API v1alpha2 is deprecated since containerd v1.7 and removed in containerd v2.0. Use CRI API v1 instead.")
+		if in.warn != nil {
+			in.warn.Emit(ctx, deprecation.CRIAPIV1Alpha2)
+		}
+	})
 	if err := in.checkInitialized(); err != nil {
 		return nil, err
 	}
@@ -1539,6 +1551,13 @@ func (in *instrumentedService) Version(ctx context.Context, r *runtime.VersionRe
 }
 
 func (in *instrumentedAlphaService) Version(ctx context.Context, r *runtime_alpha.VersionRequest) (res *runtime_alpha.VersionResponse, err error) {
+	// Only emit the warning the first time the v1alpha2 api is called
+	in.emitWarning.Do(func() {
+		log.G(ctx).Warning("CRI API v1alpha2 is deprecated since containerd v1.7 and removed in containerd v2.0. Use CRI API v1 instead.")
+		if in.warn != nil {
+			in.warn.Emit(ctx, deprecation.CRIAPIV1Alpha2)
+		}
+	})
 	if err := in.checkInitialized(); err != nil {
 		return nil, err
 	}

pkg/cri/sbserver/service.go

@@ -35,6 +35,7 @@ import (
 	"github.com/containerd/containerd/pkg/kmutex"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/sandbox"
+	"github.com/containerd/containerd/services/warning"
 	runtime_alpha "github.com/containerd/containerd/third_party/k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	"github.com/containerd/go-cni"
 	"github.com/sirupsen/logrus"
@@ -122,10 +123,12 @@ type criService struct {
 	containerEventsChan chan runtime.ContainerEventResponse
 	// nri is used to hook NRI into CRI request processing.
 	nri *nri.API
+	// warn is used to emit warnings for cri-api v1alpha2 usage.
+	warn warning.Service
 }
 
 // NewCRIService returns a new instance of CRIService
-func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API) (CRIService, error) {
+func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API, warn warning.Service) (CRIService, error) {
 	var err error
 	labels := label.NewStore()
 	c := &criService{
@@ -142,6 +145,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.
 		netPlugin:                   make(map[string]cni.CNI),
 		unpackDuplicationSuppressor: kmutex.New(),
 		sandboxControllers:          make(map[criconfig.SandboxControllerMode]sandbox.Controller),
+		warn:                        warn,
 	}
 
 	// TODO: figure out a proper channel size.
@@ -343,7 +347,7 @@ func (c *criService) register(s *grpc.Server) error {
 	runtime.RegisterRuntimeServiceServer(s, instrumented)
 	runtime.RegisterImageServiceServer(s, instrumented)
 
-	instrumentedAlpha := instrument.NewAlphaService(c)
+	instrumentedAlpha := instrument.NewAlphaService(c, c.warn)
 	runtime_alpha.RegisterRuntimeServiceServer(s, instrumentedAlpha)
 	runtime_alpha.RegisterImageServiceServer(s, instrumentedAlpha)
 

pkg/cri/server/service.go

@@ -33,6 +33,7 @@ import (
 	"github.com/containerd/containerd/pkg/cri/streaming"
 	"github.com/containerd/containerd/pkg/kmutex"
 	"github.com/containerd/containerd/plugin"
+	"github.com/containerd/containerd/services/warning"
 	runtime_alpha "github.com/containerd/containerd/third_party/k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	cni "github.com/containerd/go-cni"
 	"github.com/sirupsen/logrus"
@@ -117,10 +118,12 @@ type criService struct {
 	// containerEventsChan is used to capture container events and send them
 	// to the caller of GetContainerEvents.
 	containerEventsChan chan runtime.ContainerEventResponse
+	// warn is used to emit warnings for cri-api v1alpha2 usage.
+	warn warning.Service
 }
 
 // NewCRIService returns a new instance of CRIService
-func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API) (CRIService, error) {
+func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.API, warn warning.Service) (CRIService, error) {
 	var err error
 	labels := label.NewStore()
 	c := &criService{
@@ -136,6 +139,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client, nri *nri.
 		initialized:                 atomic.NewBool(false),
 		netPlugin:                   make(map[string]cni.CNI),
 		unpackDuplicationSuppressor: kmutex.New(),
+		warn:                        warn,
 	}
 
 	// TODO: figure out a proper channel size.
@@ -328,7 +332,7 @@ func (c *criService) register(s *grpc.Server) error {
 	runtime.RegisterRuntimeServiceServer(s, instrumented)
 	runtime.RegisterImageServiceServer(s, instrumented)
 
-	instrumentedAlpha := instrument.NewAlphaService(c)
+	instrumentedAlpha := instrument.NewAlphaService(c, c.warn)
 	runtime_alpha.RegisterRuntimeServiceServer(s, instrumentedAlpha)
 	runtime_alpha.RegisterImageServiceServer(s, instrumentedAlpha)
 

pkg/cri/server/service_test.go

@@ -17,22 +17,27 @@
 package server
 
 import (
+	"context"
 	"encoding/json"
 	"os"
 	"testing"
 
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/third_party/k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	"github.com/containerd/go-cni"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/containerd/containerd/pkg/atomic"
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
+	"github.com/containerd/containerd/pkg/cri/instrument"
 	servertesting "github.com/containerd/containerd/pkg/cri/server/testing"
 	containerstore "github.com/containerd/containerd/pkg/cri/store/container"
 	imagestore "github.com/containerd/containerd/pkg/cri/store/image"
 	"github.com/containerd/containerd/pkg/cri/store/label"
 	sandboxstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
 	snapshotstore "github.com/containerd/containerd/pkg/cri/store/snapshot"
+	"github.com/containerd/containerd/pkg/deprecation"
 	ostesting "github.com/containerd/containerd/pkg/os/testing"
 	"github.com/containerd/containerd/pkg/registrar"
 )
@@ -53,6 +58,7 @@ func newTestCRIService() *criService {
 		netPlugin: map[string]cni.CNI{
 			defaultNetworkPlugin: servertesting.NewFakeCNIPlugin(),
 		},
+		initialized: atomic.NewBool(false),
 	}
 }
 
@@ -86,3 +92,18 @@ func TestLoadBaseOCISpec(t *testing.T) {
 	assert.Equal(t, "1.0.2", out.Version)
 	assert.Equal(t, "default", out.Hostname)
 }
+
+func TestAlphaCRIWarning(t *testing.T) {
+	ctx := context.Background()
+	ws := servertesting.NewFakeWarningService()
+	c := instrument.NewAlphaService(newTestCRIService(), ws)
+
+	c.Version(ctx, &v1alpha2.VersionRequest{})
+	c.Status(ctx, &v1alpha2.StatusRequest{})
+
+	// Only emit the warning the first time an v1alpha2 api is called.
+	expectedWarnings := []deprecation.Warning{
+		deprecation.CRIAPIV1Alpha2,
+	}
+	assert.Equal(t, expectedWarnings, ws.GetWarnings())
+}

pkg/cri/server/testing/fake_warning_service.go

@@ -0,0 +1,46 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package testing
+
+import (
+	"context"
+
+	"github.com/containerd/containerd/pkg/deprecation"
+	"github.com/containerd/containerd/services/warning"
+)
+
+// FakeWarningService is a fake service used for test.
+type FakeWarningService struct {
+	warnings []deprecation.Warning
+}
+
+// NewFakeWarningService create a FakeWarningService.
+func NewFakeWarningService() *FakeWarningService {
+	return &FakeWarningService{warnings: []deprecation.Warning{}}
+}
+
+func (ws *FakeWarningService) Emit(ctx context.Context, w deprecation.Warning) {
+	ws.warnings = append(ws.warnings, w)
+}
+
+func (ws *FakeWarningService) Warnings() []warning.Warning {
+	return nil
+}
+
+func (ws *FakeWarningService) GetWarnings() []deprecation.Warning {
+	return ws.warnings
+}

pkg/deprecation/deprecation.go

@@ -31,6 +31,8 @@ const (
 	CRIRegistryAuths Warning = Prefix + "cri-registry-auths"
 	// CRIRegistryConfigs is a warning for the use of the `configs` property
 	CRIRegistryConfigs Warning = Prefix + "cri-registry-configs"
+	// CRIAPIV1Alpha2 is a warning for the use of CRI-API v1alpha2
+	CRIAPIV1Alpha2 Warning = Prefix + "cri-api-v1alpha2"
 )
 
 var messages = map[Warning]string{
@@ -43,6 +45,7 @@ var messages = map[Warning]string{
 		"Use `ImagePullSecrets` instead.",
 	CRIRegistryConfigs: "The `configs` property of `[plugins.\"io.containerd.grpc.v1.cri\".registry]` is deprecated since containerd v1.5 and will be removed in containerd v2.0." +
 		"Use `config_path` instead.",
+	CRIAPIV1Alpha2: "CRI API v1alpha2 is deprecated since containerd v1.7 and removed in containerd v2.0. Use CRI API v1 instead.",
 }
 
 // Valid checks whether a given Warning is valid