Skip to content

Commit

Permalink
ON-15512: Don't fail with non cri-o container runtime
Browse files Browse the repository at this point in the history
Any container runtime that is compatible with the cri should work fine
since we are using `crictl` which is designed to run against the
standard.
  • Loading branch information
tcrawley-xilinx committed Jan 12, 2024
1 parent a6d199b commit 0edd38d
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 28 deletions.
24 changes: 20 additions & 4 deletions pkg/control_plane/control_plane.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,40 @@ package control_plane

import (
"fmt"
"slices"
"strings"

"github.com/golang/glog"
)

// I couldn't find a definitive list anywhere, so this is a list of the
// "graduated" container runtimes from cncf.io
// https://landscape.cncf.io/card-mode?category=container-runtime
var knownGoodContainerRuntimes = []string{"cri-o", "containerd"}

// Configure the loaded Onload kernel module to launch
// the Onload control plane process within a container.
func Configure(
onloadCPServerPath string, onloadCPServerParams string,
containerID string, kpw KernelParametersWriter,
) error {
// Split the container runtime type from identifier.
containerID, found := strings.CutPrefix(containerID, "cri-o://")
if !found {
return fmt.Errorf("unsupported container runtime in %s", containerID)
runtimeAndIdentifier := strings.Split(containerID, "://")
if len(runtimeAndIdentifier) != 2 {
return fmt.Errorf("unexpected format of containerID %s", containerID)
}

if !slices.Contains(knownGoodContainerRuntimes, runtimeAndIdentifier[0]) {
// Warn, but continue
glog.Warningf("Unexpected container runtime: %s."+
" Running in a compatible container runtime (CRI) is required."+
" Execution will continue, but starting the cplane may fail.",
runtimeAndIdentifier[0])
}
containerID = runtimeAndIdentifier[1]

glog.Info("CRI-O container ID is ", containerID)
glog.Info("Container ID found", "runtime",
runtimeAndIdentifier[0], "id", containerID)

// Set the Onload control plane path.
crictlPath := "/usr/bin/crictl"
Expand Down
47 changes: 23 additions & 24 deletions pkg/control_plane/control_plane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@ package control_plane
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/types"
)

const (
mockOnloadCPServerPath = "/mock/sbin/onload_cp_server"
mockOnloadCPServerParams = "-mock -foo -bar"

mockCRIOContainerID = "cri-o://0123456789abcdef"
mockContainerdContainerID = "containerd://fedcba9876543210"
goodContainerID = "0123456789abcdef"

mockCRIOContainerID = "cri-o://" + goodContainerID
mockContainerdContainerID = "containerd://" + goodContainerID
mockTooShortContainerID = "cri-o" + goodContainerID
mockTooLongContainerId = mockCRIOContainerID + "://suffix"
)

type mockKernelParametersWriter struct {
Expand All @@ -35,25 +40,19 @@ func (mkpw *mockKernelParametersWriter) SetControlPlaneServerParams(params strin
return nil
}

var _ = Describe("Configure Onload to launch the control plane process within a container", func() {
It("Should work with the CRI-O runtime", func() {
expectedParams := "exec 0123456789abcdef /mock/sbin/onload_cp_server -mock -foo -bar"
err := Configure(mockOnloadCPServerPath, mockOnloadCPServerParams,
mockCRIOContainerID, NewMockKernelParametersWriter(expectedParams))
Expect(err).Should(Succeed())
})

It("Should work with the CRI-O runtime and empty parameter list", func() {
expectedParams := "exec 0123456789abcdef /mock/sbin/onload_cp_server"
err := Configure(mockOnloadCPServerPath, "", mockCRIOContainerID,
NewMockKernelParametersWriter(expectedParams))
Expect(err).Should(Succeed())
})

It("Should fail with non CRI-O runtime, e.g. containerd", func() {
notUsed := ""
err := Configure(mockOnloadCPServerPath, mockOnloadCPServerParams,
mockContainerdContainerID, NewMockKernelParametersWriter(notUsed))
Expect(err).Should(HaveOccurred())
})
})
var _ = DescribeTable(
"Configure Onload to launch the control plane process within a container",
func(
serverPath, serverParams, containerID, expectedParams string,
matcher types.GomegaMatcher,
) {
err := Configure(serverPath, serverParams,
containerID, NewMockKernelParametersWriter(expectedParams))
Expect(err).Should(matcher)
},
Entry("should work with the CRI-O runtime", mockOnloadCPServerPath, mockOnloadCPServerParams, mockCRIOContainerID, "exec 0123456789abcdef /mock/sbin/onload_cp_server -mock -foo -bar", Succeed()),
Entry("should work with the CRI-O runtime and empty parameter list", mockOnloadCPServerPath, "", mockCRIOContainerID, "exec 0123456789abcdef /mock/sbin/onload_cp_server", Succeed()),
Entry("should pass with non CRI-O (but still good) runtime, e.g. containerd", mockOnloadCPServerPath, "", mockContainerdContainerID, "exec 0123456789abcdef /mock/sbin/onload_cp_server", Succeed()),
Entry("should reject a too short ContainerId", mockOnloadCPServerPath, "", mockTooShortContainerID, "", HaveOccurred()),
Entry("should reject a too long ContainerId", mockOnloadCPServerPath, "", mockTooLongContainerId, "", HaveOccurred()),
)

0 comments on commit 0edd38d

Please # to comment.