kubernetes-sigs / kube-scheduler-wasm-extension

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

guest: add plugin.Set for simpler example and fixes URL parsing #69

Closed codefromthecrypt closed 9 months ago

codefromthecrypt commented 9 months ago

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This adds plugin.Set which makes it easier to write plugins. This also makes the example that uses it simpler by avoiding nottinygo. This is simpler because it reduces the amount of flags used, as well a flat package is easier to write. However, this can't be tested with tinygo anymore, and is slower.

Hence, this adds an "advanced" example with the former logic, which is more verbose and tricky, but faster.

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

This also fixes some bugs in URL parsing due to some logic not copied over from dapr. Notably, the original logic handled relative paths and better error messages. This also simplifies the logic further as we were parsing into URL type, just to convert it back to a string! Also, we started using testify by accident where that's not used anywhere else.

cc @evacchi sorry I didn't look closely at your earlier PR or would have caught this!

Does this PR introduce a user-facing change?

NONE

What are the benchmark results of this change?

You can see below that the simple plugin runs >5x slower than the advanced. However, because it is still less than millisecond, some users may be ok with this.

goos: darwin
goarch: arm64
pkg: sigs.k8s.io/kube-scheduler-wasm-extension/internal/e2e/scheduler
                                   │  before.txt  │       now.txt        │
                                   │    sec/op    │    sec/op      vs base   │
Example_NodeNumber/New-12            75.08m ±  2%
Example_NodeNumber/Run-12            47.86µ ± 10%
Example_NodeNumber/Simple/New-12                    62.23m ±   1%
Example_NodeNumber/Simple/Run-12                    188.6µ ± 109%
Example_NodeNumber/Advanced/New-12                  73.61m ±   2%
Example_NodeNumber/Advanced/Run-12                  47.03µ ±   2%
geomean                              1.895m         2.525m         ? ¹ ²
¹ benchmark set differs from baseline; geomeans may not be comparable
² ratios must be >0 to compute geomean
codefromthecrypt commented 9 months ago

added polish, mostly README

codefromthecrypt commented 9 months ago

squashed and ready, thanks for the look @sanposhiho!

k8s-ci-robot commented 9 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: codefromthecrypt, 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)~~ [codefromthecrypt,sanposhiho] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment