Skip to content

Commit

Permalink
ON-15199: Add cplane parameters to Onload CRD
Browse files Browse the repository at this point in the history
  • Loading branch information
ivatet-amd committed Jan 12, 2024
1 parent 4448fa3 commit f6b8691
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 13 deletions.
12 changes: 12 additions & 0 deletions api/v1alpha1/onload_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ type OnloadSpec struct {
// ImagePullPolicy is the policy used when pulling images.
// More info: https://kubernetes.io/docs/concepts/containers/images#updating-images
ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"`

// +optional
// ControlPlane allows fine-tuning of the Onload control plane server.
ControlPlane *ControlPlaneSpec `json:"controlPlane,omitempty"`
}

type ControlPlaneSpec struct {
// +optional
// Parameters is an optional list of parameters passed to the Onload
// control plane server when launched by the Onload kernel module.
// +kubebuilder:default:={"-K"}
Parameters []string `json:"parameters"`
}

// Currently unimplemented
Expand Down
25 changes: 25 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions cmd/worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ func main() {
// A mount point with the Onload control plane server.
onloadCPServerPath := os.Getenv("ONLOAD_CP_SERVER_PATH")

// Params the Onload kmod should pass to the Onload control plane server.
onloadCPServerParams := os.Getenv("ONLOAD_CP_SERVER_PARAMS")

// There is a race condition when the container is starting, and the
// Onload kernel module is loading at the same time, resulting in not
// all files being available in the container.
Expand Down Expand Up @@ -59,8 +62,8 @@ func main() {

// Configure Onload kernel module to launch
// Onload control plane within a container.
err = control_plane.Configure(onloadCPServerPath, containerID,
control_plane.NewKernelParametersWriter())
err = control_plane.Configure(onloadCPServerPath, onloadCPServerParams,
containerID, control_plane.NewKernelParametersWriter())
if err != nil {
glog.Fatal("Failed to configure Onload control plane: ", err)
}
Expand Down
14 changes: 14 additions & 0 deletions config/crd/bases/onload.amd.com_onloads.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@ spec:
description: Onload is the specification of the version of onload
to be used by this CR
properties:
controlPlane:
description: ControlPlane allows fine-tuning of the Onload control
plane server.
properties:
parameters:
default:
- -K
description: Parameters is an optional list of parameters
passed to the Onload control plane server when launched
by the Onload kernel module.
items:
type: string
type: array
type: object
imagePullPolicy:
description: 'ImagePullPolicy is the policy used when pulling
images. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images'
Expand Down
10 changes: 10 additions & 0 deletions controllers/onload_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"slices"
"strings"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -991,6 +992,11 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(

workerContainerName := "onload-worker"

cplaneParams := "-K" // log-to-kmsg
if onload.Spec.Onload.ControlPlane != nil && onload.Spec.Onload.ControlPlane.Parameters != nil {
cplaneParams = strings.Join(onload.Spec.Onload.ControlPlane.Parameters, " ")
}

workerContainerEnv := []corev1.EnvVar{
{
Name: "POD_NAME",
Expand All @@ -1016,6 +1022,10 @@ func (r *OnloadReconciler) createDevicePluginDaemonSet(
Name: "ONLOAD_CP_SERVER_PATH",
Value: "/mnt/onload/sbin/onload_cp_server",
},
{
Name: "ONLOAD_CP_SERVER_PARAMS",
Value: cplaneParams,
},
}

workerContainer := corev1.Container{
Expand Down
52 changes: 52 additions & 0 deletions controllers/onload_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package controllers

import (
"errors"
"slices"
"strconv"
"time"

Expand Down Expand Up @@ -871,5 +872,56 @@ var _ = Describe("onload controller", func() {
"-libMountPath=qux",
),
)

DescribeTable("Testing Onload cplane parameters",
func(cplane *onloadv1alpha1.ControlPlaneSpec, expectedParams string) {
devicePlugin := appsv1.DaemonSet{}
devicePluginName := types.NamespacedName{
Name: onload.Name + "-onload-device-plugin-ds",
Namespace: onload.Namespace,
}

if cplane != nil {
onload.Spec.Onload.ControlPlane = cplane
}

Expect(k8sClient.Create(ctx, onload)).To(BeNil())

Eventually(func() bool {
err := k8sClient.Get(ctx, devicePluginName, &devicePlugin)
return err == nil
}, timeout, pollingInterval).Should(BeTrue())

// Find the worker container.
workerIdx := slices.IndexFunc(devicePlugin.Spec.Template.Spec.Containers,
func(c corev1.Container) bool { return c.Name == "onload-worker" })

Expect(workerIdx).NotTo(Equal(-1))
workerContainer := devicePlugin.Spec.Template.Spec.Containers[workerIdx]

// Find the ONLOAD_CP_SERVER_PARAMS env.
envIdx := slices.IndexFunc(workerContainer.Env,
func(e corev1.EnvVar) bool { return e.Name == "ONLOAD_CP_SERVER_PARAMS" })
Expect(envIdx).NotTo(Equal(-1))
envValue := workerContainer.Env[envIdx].Value

// Compare actual vs. expected.
Expect(envValue).To(Equal(expectedParams))

},
Entry( /*It*/ "should use the default parameters", nil, "-K"),
Entry( /*It*/ "should pass an empty list of parameters",
&onloadv1alpha1.ControlPlaneSpec{Parameters: []string{}}, "",
),
Entry( /*It*/ "should pass a custom list of parameters",
&onloadv1alpha1.ControlPlaneSpec{
Parameters: []string{
"-K",
"--llap-max=100",
},
}, "-K --llap-max=100",
),
)

})
})
11 changes: 9 additions & 2 deletions pkg/control_plane/control_plane.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import (
// Configure the loaded Onload kernel module to launch
// the Onload control plane process within a container.
func Configure(
onloadCPServerPath string, containerID string, kpw KernelParametersWriter,
onloadCPServerPath string, onloadCPServerParams string,
containerID string, kpw KernelParametersWriter,
) error {
// Split the container runtime type from identifier.
containerID, found := strings.CutPrefix(containerID, "cri-o://")
Expand All @@ -31,8 +32,14 @@ func Configure(

glog.Info("Updated Onload control plane server path to ", crictlPath)

// Prepend the whitespace to non-empty cplane parameters.
if onloadCPServerParams != "" {
onloadCPServerParams = " " + onloadCPServerParams
}

// Set the Onload control plane params.
params := fmt.Sprintf("exec %s %s -K", containerID, onloadCPServerPath)
params := fmt.Sprintf("exec %s %s%s", containerID,
onloadCPServerPath, onloadCPServerParams)
err = kpw.SetControlPlaneServerParams(params)
if err != nil {
return err
Expand Down
31 changes: 22 additions & 9 deletions pkg/control_plane/control_plane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,39 +8,52 @@ import (
)

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

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

type mockKernelParametersWriter struct {
expectedParams string
}

func NewMockKernelParametersWriter() KernelParametersWriter {
return &mockKernelParametersWriter{}
func NewMockKernelParametersWriter(expectedParams string) KernelParametersWriter {
return &mockKernelParametersWriter{
expectedParams: expectedParams,
}
}

func (*mockKernelParametersWriter) SetControlPlaneServerPath(path string) error {
Expect(path).To(Equal("/usr/bin/crictl"))
return nil
}

func (*mockKernelParametersWriter) SetControlPlaneServerParams(params string) error {
Expect(params).To(Equal("exec 0123456789abcdef /mock/sbin/onload_cp_server -K"))
func (mkpw *mockKernelParametersWriter) SetControlPlaneServerParams(params string) error {
Expect(params).To(Equal(mkpw.expectedParams))
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() {
err := Configure(mockOnloadCPServerPath, mockCRIOContainerID,
NewMockKernelParametersWriter())
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() {
err := Configure(mockOnloadCPServerPath, mockContainerdContainerID,
NewMockKernelParametersWriter())
notUsed := ""
err := Configure(mockOnloadCPServerPath, mockOnloadCPServerParams,
mockContainerdContainerID, NewMockKernelParametersWriter(notUsed))
Expect(err).Should(HaveOccurred())
})
})

0 comments on commit f6b8691

Please sign in to comment.