From 0edd38d2db7a2c1a1126c86fa2b0f80ffba2d15a Mon Sep 17 00:00:00 2001 From: Thomas Crawley Date: Thu, 11 Jan 2024 17:21:05 +0000 Subject: [PATCH] ON-15512: Don't fail with non cri-o container runtime 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. --- pkg/control_plane/control_plane.go | 24 ++++++++++--- pkg/control_plane/control_plane_test.go | 47 ++++++++++++------------- 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/pkg/control_plane/control_plane.go b/pkg/control_plane/control_plane.go index 583c127..b9b33e5 100644 --- a/pkg/control_plane/control_plane.go +++ b/pkg/control_plane/control_plane.go @@ -4,11 +4,17 @@ 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( @@ -16,12 +22,22 @@ func Configure( 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" diff --git a/pkg/control_plane/control_plane_test.go b/pkg/control_plane/control_plane_test.go index e5f324a..ac51dc5 100644 --- a/pkg/control_plane/control_plane_test.go +++ b/pkg/control_plane/control_plane_test.go @@ -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 { @@ -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()), +)