From 796c0a85f916f05094024fbbf1601f80d09cb09c Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Wed, 21 Feb 2024 16:29:56 +0100 Subject: [PATCH] Executor: Make runtimeClass configurable Move the definition of the underlying `runtimeClass` to the executor config. This allows an operator to specify a runtimeClass name that makes sense for their cluster, and allows for multiple runtimeClasses for trying out different node pool versions in a live cluster. Also move deployment creation to the CRD to remove cyclotron-specific handling. Signed-off-by: Danielle Lancashire --- api/v1/spinappexecutor_types.go | 15 ++++- api/v1/zz_generated.deepcopy.go | 22 ++++++- .../containerd-shim-spin-executor.yaml | 3 + cmd/main.go | 5 +- ...ore.spinoperator.dev_spinappexecutors.yaml | 21 +++++++ config/samples/shim-executor.yaml | 3 + internal/controller/deployment_test.go | 2 +- internal/controller/spinapp_controller.go | 58 +++++++++++-------- .../controller/spinapp_controller_test.go | 6 +- 9 files changed, 103 insertions(+), 32 deletions(-) diff --git a/api/v1/spinappexecutor_types.go b/api/v1/spinappexecutor_types.go index 301eb238..90b9e7f9 100644 --- a/api/v1/spinappexecutor_types.go +++ b/api/v1/spinappexecutor_types.go @@ -25,8 +25,19 @@ import ( // SpinAppExecutorSpec defines the desired state of SpinAppExecutor type SpinAppExecutorSpec struct { - // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster - // Important: Run "make" to regenerate code after modifying this file + // CreateDeployment specifies whether the Executor wants the SpinKube operator + // to create a deployment for the application or if it will be realized externally. + CreateDeployment bool `json:"createDeployment"` + + // DeploymentConfig specifies how the deployment should be configured when + // createDeployment is true. + DeploymentConfig *ExecutorDeploymentConfig `json:"deploymentConfig,omitempty"` +} + +type ExecutorDeploymentConfig struct { + // RuntimeClassName is the runtime class name that should be used by pods created + // as part of a deployment. + RuntimeClassName string `json:"runtimeClassName"` } // SpinAppExecutorStatus defines the observed state of SpinAppExecutor diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 48bcb3e4..a0115c79 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -26,6 +26,21 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExecutorDeploymentConfig) DeepCopyInto(out *ExecutorDeploymentConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExecutorDeploymentConfig. +func (in *ExecutorDeploymentConfig) DeepCopy() *ExecutorDeploymentConfig { + if in == nil { + return nil + } + out := new(ExecutorDeploymentConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPHealthProbe) DeepCopyInto(out *HTTPHealthProbe) { *out = *in @@ -182,7 +197,7 @@ func (in *SpinAppExecutor) DeepCopyInto(out *SpinAppExecutor) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) out.Status = in.Status } @@ -239,6 +254,11 @@ func (in *SpinAppExecutorList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SpinAppExecutorSpec) DeepCopyInto(out *SpinAppExecutorSpec) { *out = *in + if in.DeploymentConfig != nil { + in, out := &in.DeploymentConfig, &out.DeploymentConfig + *out = new(ExecutorDeploymentConfig) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SpinAppExecutorSpec. diff --git a/charts/spin-operator/templates/containerd-shim-spin-executor.yaml b/charts/spin-operator/templates/containerd-shim-spin-executor.yaml index 19d6068f..531f4dcd 100644 --- a/charts/spin-operator/templates/containerd-shim-spin-executor.yaml +++ b/charts/spin-operator/templates/containerd-shim-spin-executor.yaml @@ -3,3 +3,6 @@ kind: SpinAppExecutor metadata: name: containerd-shim-spin spec: + createDeployment: true + deploymentConfig: + runtimeClassName: wasmtime-spin-v2 diff --git a/cmd/main.go b/cmd/main.go index ee6d8f8d..35cc9f60 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -102,8 +102,9 @@ func main() { } if err = (&controller.SpinAppReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("spinapp-reconciler"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SpinApp") os.Exit(1) diff --git a/config/crd/bases/core.spinoperator.dev_spinappexecutors.yaml b/config/crd/bases/core.spinoperator.dev_spinappexecutors.yaml index ab14af70..a9f536ef 100644 --- a/config/crd/bases/core.spinoperator.dev_spinappexecutors.yaml +++ b/config/crd/bases/core.spinoperator.dev_spinappexecutors.yaml @@ -38,6 +38,27 @@ spec: type: object spec: description: SpinAppExecutorSpec defines the desired state of SpinAppExecutor + properties: + createDeployment: + description: |- + CreateDeployment specifies whether the Executor wants the SpinKube operator + to create a deployment for the application or if it will be realized externally. + type: boolean + deploymentConfig: + description: |- + DeploymentConfig specifies how the deployment should be configured when + createDeployment is true. + properties: + runtimeClassName: + description: |- + RuntimeClassName is the runtime class name that should be used by pods created + as part of a deployment. + type: string + required: + - runtimeClassName + type: object + required: + - createDeployment type: object status: description: SpinAppExecutorStatus defines the observed state of SpinAppExecutor diff --git a/config/samples/shim-executor.yaml b/config/samples/shim-executor.yaml index 19d6068f..531f4dcd 100644 --- a/config/samples/shim-executor.yaml +++ b/config/samples/shim-executor.yaml @@ -3,3 +3,6 @@ kind: SpinAppExecutor metadata: name: containerd-shim-spin spec: + createDeployment: true + deploymentConfig: + runtimeClassName: wasmtime-spin-v2 diff --git a/internal/controller/deployment_test.go b/internal/controller/deployment_test.go index f9f6a040..05c8e31f 100644 --- a/internal/controller/deployment_test.go +++ b/internal/controller/deployment_test.go @@ -227,7 +227,7 @@ func TestSpinHealthCheckToCoreProbe(t *testing.T) { func TestDeploymentLabel(t *testing.T) { scheme := registerAndGetScheme() app := minimalSpinApp() - deployment, err := constructDeployment(context.Background(), app, scheme) + deployment, err := constructDeployment(context.Background(), app, &spinv1.ExecutorDeploymentConfig{}, scheme) require.Nil(t, err) require.NotNil(t, deployment.ObjectMeta.Labels) diff --git a/internal/controller/spinapp_controller.go b/internal/controller/spinapp_controller.go index b9297a88..90c08e28 100644 --- a/internal/controller/spinapp_controller.go +++ b/internal/controller/spinapp_controller.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -49,8 +50,9 @@ const ( // SpinAppReconciler reconciles a SpinApp object type SpinAppReconciler struct { - Client client.Client - Scheme *runtime.Scheme + Client client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder } //+kubebuilder:rbac:groups=core.spinoperator.dev,resources=spinapps,verbs=get;list;watch;create;update;patch;delete @@ -102,30 +104,38 @@ func (r *SpinAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } // Reconcile the child resources - switch spinApp.Spec.Executor { - case constants.CyclotronExecutor: - // Cyclotron does not use a deployment but it may exist if we were previously using a different executor - err := r.deleteDeployment(ctx, &spinApp) - if client.IgnoreNotFound(err) != nil { - return ctrl.Result{}, err - } - err = r.reconcileService(ctx, &spinApp) + var executor spinv1.SpinAppExecutor + if err := r.Client.Get(ctx, types.NamespacedName{ + // Executors must currently be defined in the same namespace as the app. + // When we decide if the operator will be global or namespaced we may want + // executors to be global as they're a platform concern. + Namespace: req.NamespacedName.Namespace, + Name: spinApp.Spec.Executor, + }, &executor); err != nil { + log.Error(err, "unable to fetch executor") + r.Recorder.Event(&spinApp, "Warning", "MissingExecutor", + fmt.Sprintf("Could not find SpinAppExecutor %s/%s", req.NamespacedName.Namespace, spinApp.Spec.Executor)) + return ctrl.Result{}, err + } + + if executor.Spec.CreateDeployment { + err := r.reconcileDeployment(ctx, &spinApp, executor.Spec.DeploymentConfig) if err != nil { return ctrl.Result{}, err } - case constants.ContainerDShimSpinExecutor: - err := r.reconcileDeployment(ctx, &spinApp) - if err != nil { + } else { + // If we shouldn't be managing a deployment for an application ensure any + // previously created deployments have been cleaned up. + err := r.deleteDeployment(ctx, &spinApp) + if client.IgnoreNotFound(err) != nil { return ctrl.Result{}, err } + } - err = r.reconcileService(ctx, &spinApp) - if err != nil { - return ctrl.Result{}, err - } - default: - return ctrl.Result{}, fmt.Errorf("unknown executor %s", spinApp.Spec.Executor) + err = r.reconcileService(ctx, &spinApp) + if err != nil { + return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -209,10 +219,10 @@ func (r *SpinAppReconciler) updateStatus(ctx context.Context, app *spinv1.SpinAp } // reconcileDeployment creates a deployment if one does not exist and reconciles it if it does. -func (r *SpinAppReconciler) reconcileDeployment(ctx context.Context, app *spinv1.SpinApp) error { +func (r *SpinAppReconciler) reconcileDeployment(ctx context.Context, app *spinv1.SpinApp, config *spinv1.ExecutorDeploymentConfig) error { log := logging.FromContext(ctx).WithValues("deployment", app.Name) - desiredDeployment, err := constructDeployment(ctx, app, r.Scheme) + desiredDeployment, err := constructDeployment(ctx, app, config, r.Scheme) if err != nil { log.Error(err, "Unable to construct Deployment") return err @@ -279,9 +289,7 @@ func (r *SpinAppReconciler) deleteDeployment(ctx context.Context, app *spinv1.Sp } // constructDeployment builds an appsv1.Deployment based on the configuration of a SpinApp. -func constructDeployment(ctx context.Context, app *spinv1.SpinApp, scheme *runtime.Scheme) (*appsv1.Deployment, error) { - spinRuntimeClassName := "wasmtime-spin-v2" - +func constructDeployment(ctx context.Context, app *spinv1.SpinApp, config *spinv1.ExecutorDeploymentConfig, scheme *runtime.Scheme) (*appsv1.Deployment, error) { // TODO: Once we land admission webhooks write some validation to make // replicas and enableAutoscaling mutually exclusive. var replicas *int32 @@ -349,7 +357,7 @@ func constructDeployment(ctx context.Context, app *spinv1.SpinApp, scheme *runti Annotations: templateAnnotations, }, Spec: corev1.PodSpec{ - RuntimeClassName: &spinRuntimeClassName, + RuntimeClassName: &config.RuntimeClassName, Containers: []corev1.Container{ { Name: app.Name, diff --git a/internal/controller/spinapp_controller_test.go b/internal/controller/spinapp_controller_test.go index 1c3d30e3..7fc81b03 100644 --- a/internal/controller/spinapp_controller_test.go +++ b/internal/controller/spinapp_controller_test.go @@ -97,11 +97,15 @@ func TestConstructDeployment_MinimalApp(t *testing.T) { app := minimalSpinApp() - deployment, err := constructDeployment(context.Background(), app, nil) + cfg := &spinv1.ExecutorDeploymentConfig{ + RuntimeClassName: "bananarama", + } + deployment, err := constructDeployment(context.Background(), app, cfg, nil) require.NoError(t, err) require.NotNil(t, deployment) require.Equal(t, ptr(int32(1)), deployment.Spec.Replicas) require.Len(t, deployment.Spec.Template.Spec.Containers, 1) require.Equal(t, app.Spec.Image, deployment.Spec.Template.Spec.Containers[0].Image) + require.Equal(t, ptr("bananarama"), deployment.Spec.Template.Spec.RuntimeClassName) }