kubernetes-sigs / kube-scheduler-wasm-extension

All the things to make the scheduler extendable with wasm.
Apache License 2.0
110 stars 22 forks source link

allow the scheduler configuration to have multi wasm plugins #58

Closed sanposhiho closed 1 year ago

sanposhiho commented 1 year ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR changes the entry point to read the scheduler configuration before it initializes the scheduler (app.NewSchedulerCommand) and allows us to have multiple wasm plugins.

apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
  - plugins:
      multipoint:
        enabled:
           - name: wasmplugin1
           - name: wasmplugin2
     pluginConfig:
       - name: wasmplugin1
          args:
            guestPath: "/path/to/wasm-plugin1"
       - name: wasmplugin2
          args:
            guestPath: "/path/to/wasm-plugin2"

Which issue(s) this PR fixes:

Fixes #23

Special notes for your reviewer:

Does this PR introduce a user-facing change?

The scheduler can have multiple wasm plugins.

What are the benchmark results of this change?

N/A (no benchmark related area in this change.)
k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sanposhiho

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kube-scheduler-wasm-extension/blob/main/OWNERS)~~ [sanposhiho] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
sanposhiho commented 1 year ago

After spending good days in NZ, my capacity is coming back here. Will address the comment from Adrian and open it then.

sanposhiho commented 1 year ago

OK, it's ready. @kerthcet Can you please take a look when have time.

Gekko0114 commented 1 year ago

Minor comment

apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
  - plugins:
      multipoint:
        enabled:
           - name: wasmplugin1
           - name: wasmplugin2
     pluginConfig:
       - name: wasmplugin1
          args:
            guestPath: "/path/to/wasm-plugin1"
       - name: wasmplugin2
          args:
            guestPath: "/path/to/wasm-plugin2"

guestPath should have extension .wasm?

Gekko0114 commented 1 year ago

Minor comment: It might be better to have a test case that verifies correct execution by passing multiple wasm files.

Gekko0114 commented 1 year ago

Overall, it looks very good :) I have left several minor comments

sanposhiho commented 1 year ago

@Gekko0114 fixed or replied. PTAL again.

Gekko0114 commented 1 year ago

LGTM!

Gekko0114 commented 1 year ago

/lgtm