Merge pull request #8066 from fuweid/cleanup-blockio-init

*: introduce wrapper pkgs for blockio and rdt

pkg/blockio/blockio_linux.go

@@ -0,0 +1,72 @@
+//go:build linux
+
+/*
+   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 blockio
+
+import (
+	"fmt"
+	"sync"
+
+	"github.com/intel/goresctrl/pkg/blockio"
+	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
+
+	"github.com/containerd/containerd/log"
+)
+
+var (
+	enabled   bool
+	enabledMu sync.RWMutex
+)
+
+// IsEnabled checks whether blockio is enabled.
+func IsEnabled() bool {
+	enabledMu.RLock()
+	defer enabledMu.RUnlock()
+
+	return enabled
+}
+
+// SetConfig updates blockio config with a given config path.
+func SetConfig(configFilePath string) error {
+	enabledMu.Lock()
+	defer enabledMu.Unlock()
+
+	enabled = false
+	if configFilePath == "" {
+		log.L.Debug("No blockio config file specified, blockio not configured")
+		return nil
+	}
+
+	if err := blockio.SetConfigFromFile(configFilePath, true); err != nil {
+		return fmt.Errorf("blockio not enabled: %w", err)
+	}
+	enabled = true
+	return nil
+}
+
+// ClassNameToLinuxOCI converts blockio class name into the LinuxBlockIO
+// structure in the OCI runtime spec.
+func ClassNameToLinuxOCI(className string) (*runtimespec.LinuxBlockIO, error) {
+	return blockio.OciLinuxBlockIO(className)
+}
+
+// ContainerClassFromAnnotations examines container and pod annotations of a
+// container and returns its blockio class.
+func ContainerClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
+	return blockio.ContainerClassFromAnnotations(containerName, containerAnnotations, podAnnotations)
+}

pkg/blockio/blockio_nonlinux.go

@@ -0,0 +1,39 @@
+//go:build !linux
+
+/*
+   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 blockio
+
+import runtimespec "github.com/opencontainers/runtime-spec/specs-go"
+
+// IsEnabled always returns false in non-linux platforms.
+func IsEnabled() bool { return false }
+
+// SetConfig always is no-op in non-linux platforms.
+func SetConfig(configFilePath string) error {
+	return nil
+}
+
+// ClassNameToLinuxOCI always is no-op in non-linux platforms.
+func ClassNameToLinuxOCI(className string) (*runtimespec.LinuxBlockIO, error) {
+	return nil, nil
+}
+
+// ContainerClassFromAnnotations always is no-op in non-linux platforms.
+func ContainerClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
+	return "", nil
+}

pkg/cri/sbserver/blockio_linux.go

@@ -21,10 +21,9 @@ package sbserver
 import (
 	"fmt"
 
-	"github.com/containerd/containerd/services/tasks"
-	"github.com/intel/goresctrl/pkg/blockio"
-	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
+
+	"github.com/containerd/containerd/pkg/blockio"
 )
 
 // blockIOClassFromAnnotations examines container and pod annotations of a
@@ -35,7 +34,7 @@ func (c *criService) blockIOClassFromAnnotations(containerName string, container
 		return "", err
 	}
 
-	if cls != "" && !tasks.BlockIOEnabled() {
+	if cls != "" && !blockio.IsEnabled() {
 		if c.config.ContainerdConfig.IgnoreBlockIONotEnabledErrors {
 			cls = ""
 			logrus.Debugf("continuing create container %s, ignoring blockio not enabled (%v)", containerName, err)
@@ -45,9 +44,3 @@ func (c *criService) blockIOClassFromAnnotations(containerName string, container
 	}
 	return cls, nil
 }
-
-// blockIOToLinuxOci converts blockio class name into the LinuxBlockIO
-// structure in the OCI runtime spec.
-func blockIOToLinuxOci(className string) (*runtimespec.LinuxBlockIO, error) {
-	return blockio.OciLinuxBlockIO(className)
-}

pkg/cri/sbserver/blockio_stub.go

@@ -18,14 +18,6 @@
 
 package sbserver
 
-import (
-	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
-)
-
 func (c *criService) blockIOClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
 	return "", nil
 }
-
-func blockIOToLinuxOci(className string) (*runtimespec.LinuxBlockIO, error) {
-	return nil, nil
-}

pkg/cri/sbserver/container_create.go

@@ -36,6 +36,7 @@ import (
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/pkg/blockio"
 	"github.com/containerd/containerd/pkg/cri/annotations"
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
 	cio "github.com/containerd/containerd/pkg/cri/io"
@@ -639,7 +640,7 @@ func (c *criService) buildLinuxSpec(
 		return nil, fmt.Errorf("failed to set blockio class: %w", err)
 	}
 	if blockIOClass != "" {
-		if linuxBlockIO, err := blockIOToLinuxOci(blockIOClass); err == nil {
+		if linuxBlockIO, err := blockio.ClassNameToLinuxOCI(blockIOClass); err == nil {
 			specOpts = append(specOpts, oci.WithBlockIO(linuxBlockIO))
 		} else {
 			return nil, err

pkg/cri/sbserver/rdt.go

@@ -21,9 +21,9 @@ package sbserver
 import (
 	"fmt"
 
-	"github.com/containerd/containerd/services/tasks"
-	"github.com/intel/goresctrl/pkg/rdt"
 	"github.com/sirupsen/logrus"
+
+	"github.com/containerd/containerd/pkg/rdt"
 )
 
 // rdtClassFromAnnotations examines container and pod annotations of a
@@ -33,13 +33,13 @@ func (c *criService) rdtClassFromAnnotations(containerName string, containerAnno
 
 	if err == nil {
 		// Our internal check that RDT has been enabled
-		if cls != "" && !tasks.RdtEnabled() {
+		if cls != "" && !rdt.IsEnabled() {
 			err = fmt.Errorf("RDT disabled, refusing to set RDT class of container %q to %q", containerName, cls)
 		}
 	}
 
 	if err != nil {
-		if !tasks.RdtEnabled() && c.config.ContainerdConfig.IgnoreRdtNotEnabledErrors {
+		if !rdt.IsEnabled() && c.config.ContainerdConfig.IgnoreRdtNotEnabledErrors {
 			logrus.Debugf("continuing create container %s, ignoring rdt not enabled (%v)", containerName, err)
 			return "", nil
 		}

pkg/cri/server/blockio_linux.go

@@ -21,10 +21,9 @@ package server
 import (
 	"fmt"
 
-	"github.com/containerd/containerd/services/tasks"
-	"github.com/intel/goresctrl/pkg/blockio"
-	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
+
+	"github.com/containerd/containerd/pkg/blockio"
 )
 
 // blockIOClassFromAnnotations examines container and pod annotations of a
@@ -35,7 +34,7 @@ func (c *criService) blockIOClassFromAnnotations(containerName string, container
 		return "", err
 	}
 
-	if cls != "" && !tasks.BlockIOEnabled() {
+	if cls != "" && !blockio.IsEnabled() {
 		if c.config.ContainerdConfig.IgnoreBlockIONotEnabledErrors {
 			cls = ""
 			logrus.Debugf("continuing create container %s, ignoring blockio not enabled (%v)", containerName, err)
@@ -45,9 +44,3 @@ func (c *criService) blockIOClassFromAnnotations(containerName string, container
 	}
 	return cls, nil
 }
-
-// blockIOToLinuxOci converts blockio class name into the LinuxBlockIO
-// structure in the OCI runtime spec.
-func blockIOToLinuxOci(className string) (*runtimespec.LinuxBlockIO, error) {
-	return blockio.OciLinuxBlockIO(className)
-}

pkg/cri/server/blockio_stub_linux.go

@@ -18,14 +18,6 @@
 
 package server
 
-import (
-	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
-)
-
 func (c *criService) blockIOClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
 	return "", nil
 }
-
-func blockIOToLinuxOci(className string) (*runtimespec.LinuxBlockIO, error) {
-	return nil, nil
-}

pkg/cri/server/container_create_linux.go

@@ -36,6 +36,7 @@ import (
 	"github.com/opencontainers/selinux/go-selinux/label"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
+	"github.com/containerd/containerd/pkg/blockio"
 	"github.com/containerd/containerd/pkg/cri/annotations"
 	"github.com/containerd/containerd/pkg/cri/config"
 	customopts "github.com/containerd/containerd/pkg/cri/opts"
@@ -270,7 +271,7 @@ func (c *criService) containerSpec(
 		return nil, fmt.Errorf("failed to set blockio class: %w", err)
 	}
 	if blockIOClass != "" {
-		if linuxBlockIO, err := blockIOToLinuxOci(blockIOClass); err == nil {
+		if linuxBlockIO, err := blockio.ClassNameToLinuxOCI(blockIOClass); err == nil {
 			specOpts = append(specOpts, oci.WithBlockIO(linuxBlockIO))
 		} else {
 			return nil, err

pkg/cri/server/nri-api.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
+	"github.com/containerd/containerd/pkg/blockio"
 	"github.com/containerd/containerd/pkg/cri/annotations"
 	cstore "github.com/containerd/containerd/pkg/cri/store/container"
 	sstore "github.com/containerd/containerd/pkg/cri/store/sandbox"
@@ -247,7 +248,7 @@ func (a *nriAPI) WithContainerAdjustment() containerd.NewContainerOpts {
 			if className == "" {
 				return nil, nil
 			}
-			blockIO, err := blockIOToLinuxOci(className)
+			blockIO, err := blockio.ClassNameToLinuxOCI(className)
 			if err != nil {
 				return nil, err
 			}

pkg/cri/server/rdt_linux.go

@@ -21,9 +21,9 @@ package server
 import (
 	"fmt"
 
-	"github.com/containerd/containerd/services/tasks"
-	"github.com/intel/goresctrl/pkg/rdt"
 	"github.com/sirupsen/logrus"
+
+	"github.com/containerd/containerd/pkg/rdt"
 )
 
 // rdtClassFromAnnotations examines container and pod annotations of a
@@ -33,13 +33,13 @@ func (c *criService) rdtClassFromAnnotations(containerName string, containerAnno
 
 	if err == nil {
 		// Our internal check that RDT has been enabled
-		if cls != "" && !tasks.RdtEnabled() {
+		if cls != "" && !rdt.IsEnabled() {
 			err = fmt.Errorf("RDT disabled, refusing to set RDT class of container %q to %q", containerName, cls)
 		}
 	}
 
 	if err != nil {
-		if !tasks.RdtEnabled() && c.config.ContainerdConfig.IgnoreRdtNotEnabledErrors {
+		if !rdt.IsEnabled() && c.config.ContainerdConfig.IgnoreRdtNotEnabledErrors {
 			logrus.Debugf("continuing create container %s, ignoring rdt not enabled (%v)", containerName, err)
 			return "", nil
 		}

pkg/rdt/rdt_linux.go

@@ -16,10 +16,11 @@
    limitations under the License.
 */
 
-package tasks
+package rdt
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/containerd/containerd/log"
 
@@ -31,28 +32,54 @@ const (
 	ResctrlPrefix = ""
 )
 
-var rdtEnabled bool
+var (
+	enabled   bool
+	enabledMu sync.RWMutex
+)
 
-func RdtEnabled() bool { return rdtEnabled }
+// IsEnabled checks whether rdt is enabled.
+func IsEnabled() bool {
+	enabledMu.RLock()
+	defer enabledMu.RUnlock()
 
-func initRdt(configFilePath string) error {
+	return enabled
-	rdtEnabled = false
+}
 
+var (
+	initOnce sync.Once
+	initErr  error
+)
+
+// SetConfig updates rdt config with a given config path.
+func SetConfig(configFilePath string) error {
+	enabledMu.Lock()
+	defer enabledMu.Unlock()
+
+	enabled = false
 	if configFilePath == "" {
 		log.L.Debug("No RDT config file specified, RDT not configured")
 		return nil
 	}
 
-	if err := rdt.Initialize(ResctrlPrefix); err != nil {
+	initOnce.Do(func() {
-		return fmt.Errorf("RDT not enabled: %w", err)
+		err := rdt.Initialize(ResctrlPrefix)
+		if err != nil {
+			initErr = fmt.Errorf("RDT not enabled: %w", err)
+		}
+	})
+	if initErr != nil {
+		return initErr
 	}
 
 	if err := rdt.SetConfigFromFile(configFilePath, true); err != nil {
 		return err
 	}
-
+	enabled = true
-	rdtEnabled = true
-
 	return nil
-
+}
+
+// ContainerClassFromAnnotations examines container and pod annotations of a
+// container and returns its RDT class.
+func ContainerClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
+	return rdt.ContainerClassFromAnnotations(containerName, containerAnnotations, podAnnotations)
 }

pkg/rdt/rdt_nonlinux.go

@@ -16,8 +16,15 @@
    limitations under the License.
 */
 
-package tasks
+package rdt
 
-func RdtEnabled() bool { return false }
+// IsEnabled always returns false in non-linux platforms.
+func IsEnabled() bool { return false }
 
-func initRdt(configFilePath string) error { return nil }
+// SetConfig always is no-op in non-linux platforms.
+func SetConfig(configFilePath string) error { return nil }
+
+// ContainerClassFromAnnotations always is no-op in non-linux platforms.
+func ContainerClassFromAnnotations(containerName string, containerAnnotations, podAnnotations map[string]string) (string, error) {
+	return "", nil
+}

services/tasks/blockio_default.go

@@ -1,23 +0,0 @@
-//go:build !linux
-
-/*
-   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 tasks
-
-func BlockIOEnabled() bool { return false }
-
-func initBlockIO(configFilePath string) error { return nil }

services/tasks/blockio_linux.go

@@ -1,49 +0,0 @@
-//go:build linux
-
-/*
-   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 tasks
-
-import (
-	"fmt"
-
-	"github.com/containerd/containerd/log"
-
-	"github.com/intel/goresctrl/pkg/blockio"
-)
-
-var blockIOEnabled bool
-
-func BlockIOEnabled() bool { return blockIOEnabled }
-
-func initBlockIO(configFilePath string) error {
-	blockIOEnabled = false
-
-	if configFilePath == "" {
-		log.L.Debug("No blockio config file specified, blockio not configured")
-		return nil
-	}
-
-	if err := blockio.SetConfigFromFile(configFilePath, true); err != nil {
-		return fmt.Errorf("blockio not enabled: %w", err)
-	}
-
-	blockIOEnabled = true
-
-	return nil
-
-}

services/tasks/local.go

@@ -39,6 +39,8 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/metadata"
 	"github.com/containerd/containerd/mount"
+	"github.com/containerd/containerd/pkg/blockio"
+	"github.com/containerd/containerd/pkg/rdt"
 	"github.com/containerd/containerd/pkg/timeout"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/protobuf"
@@ -141,10 +143,10 @@ func initFunc(ic *plugin.InitContext) (interface{}, error) {
 		l.monitor.Monitor(t, nil)
 	}
 
-	if err := initBlockIO(config.BlockIOConfigFile); err != nil {
+	if err := blockio.SetConfig(config.BlockIOConfigFile); err != nil {
 		log.G(ic.Context).WithError(err).Errorf("blockio initialization failed")
 	}
-	if err := initRdt(config.RdtConfigFile); err != nil {
+	if err := rdt.SetConfig(config.RdtConfigFile); err != nil {
 		log.G(ic.Context).WithError(err).Errorf("RDT initialization failed")
 	}