Add configurable interpreter api

[RainbowMango]

What type of PR is this?
/kind feature
/kind api-change

What this PR does / why we need it:
This PR proposes the new API of the Configurable Interpreter.

Which issue(s) this PR fixes:
Part of #2371

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[codecov-commenter]

Codecov Report

Merging #2681 (564fe79) into master (af4b1c6) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2681      +/-   ##
==========================================
- Coverage   27.50%   27.48%   -0.02%     
==========================================
  Files         190      190              
  Lines       19061    19061              
==========================================
- Hits         5242     5239       -3     
- Misses      13458    13461       +3     
  Partials      361      361              

Flag	Coverage Δ
unittests	27.48% <ø> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/search/proxy/store/util.go	93.56% <0.00%> (-1.76%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[chaunceyjiang]

I'm mainly worried that the lua code is too long, which makes the yaml file of the ResourceInterpreterCustomization CR resource unreadable.

Can you share the yaml file of the ResourceInterpreterCustomization CR resource ?

[ikaven1024]

I'm mainly worried that the lua code is too long, which makes the yaml file of the ResourceInterpreterCustomization CR resource unreadable.

I don't think it s a big matter. People can read and edit it in IDE.
CustomResourceDe is always verify large :)

I either think defining the rules of one resource in different ResourceInterpreterCustomization has little benefit, but introducing too much matters: rule conflicting, unreadable for user... @RainbowMango

[XiShanYongYe-Chang]

I either think defining the rules of one resource in different ResourceInterpreterCustomization has little benefit, but introducing too much matters: rule conflicting, unreadable for user...

+1

[RainbowMango]

@ikaven1024 @chaunceyjiang Please take look.

[XiShanYongYe-Chang]

This line is necessary? It seems redundant:

...
  retention:
    luaScript:
      luaScript: >
       ...

[RainbowMango]

  retention:
    luaScript: >
      desiredObj.spec.fieldFoo = observedObj.spec.fieldFoo
      return desiredObj

This is expected.
LuaScript only holds the function body part

[XiShanYongYe-Chang]

Got it.

[ikaven1024]

Just a question: If I add a script with bad grammar, where and when this mistake will be reported? Do it in apiserver validating, or just throw error in runtime. And how to report these errors, in ResourceInterpreterCustomization.Status, or in events?

[RainbowMango]

Good question.

If I add a script with bad grammar

I don't if there is a kind of tool that can verify the grammar issue. If so we can validate it by admission webhook.
@jameszhangyukun any comments?

I'm thinking to introduce a tool to help user to verify/testing their customizations, like:

karmadactl interpret --type=retention --desired=desired.yaml --observed=observed.yaml --customization=<ResourceInterpreterCustomization> configuration

People can test their scripts before applying.

[XiShanYongYe-Chang]

I'm thinking to introduce a tool to help user to verify/testing their customizations,

If this works, I think it's going to be a new and meaningful job.

[ikaven1024]

LGTM

[chaunceyjiang]

https://argo-cd.readthedocs.io/en/stable/operator-manual/health/#way-2-contribute-a-custom-health-check

Like argo-cd, we can also provide a directory to customize lua scripts for well-known projects.

[jameszhangyukun]

if add a script with bad grammar，the mistake will throw error in runtime. i think we can introduce a tool to verify script before applying

[RainbowMango]

Like argo-cd, we can also provide a directory to customize lua scripts for well-known projects.

+1

[chaunceyjiang]

I either think defining the rules of one resource in different ResourceInterpreterCustomization has little benefit, but introducing too much matters: rule conflicting, unreadable for user...

Do we need to introduce an implicit priority for this?

[RainbowMango]

What priority?

[chaunceyjiang]

As @ikaven1024 said, if the user applies the following two yaml files, which one will work?

apiVersion: config.karmada.io/v1alpah1
kind: ResourceInterpreterCustomization
metadata:
  name: foo
target:
  apiVersion: v1
  kind: Pod
customizations:
  retention: 
    luaScript: >
            desiredObj.spec.fieldFoo = observedObj.spec.fieldFoo
            return desiredObj
  replicaResource:
   ...

apiVersion: config.karmada.io/v1alpah1
kind: ResourceInterpreterCustomization
metadata:
  name: bar
target:
  apiVersion: v1
  kind: Pod
customizations:
  retention: 
    luaScript: >
            desiredObj.spec.fieldFoo = desiredObj.spec.fieldFoo
            return desiredObj
  replicaResource:
   ...

[chaunceyjiang]

Example A

luaScript: >
 function ReflectStatus(observedObj)
     status = {}
     status.readyReplicas = observedObj.status.observedObj
     return status
 end

Example B

luaScript: >
   status = {}
   status.readyReplicas = observedObj.status.observedObj
   return status

Which one is right?

[RainbowMango]

Example B is expected because it has been proved by @jameszhangyukun's demo.

But I'd prefer Example A which is easier to understand and straightforward. @jameszhangyukun Can we load a whole function to Lua runtime instead of a piece of code?

[XiShanYongYe-Chang]

I think is Example B.

[ikaven1024]

But I'd prefer Example A which is easier to understand and straightforward.

+1

[chaunceyjiang]

Maybe we can refer to nginx lua.

https://github.com/openresty/lua-nginx-module#data-sharing-within-an-nginx-worker

[RainbowMango]

Can you make a demo with it?

[jameszhangyukun]

Example B is expected because it has been proved by @jameszhangyukun's demo.

But I'd prefer Example A which is easier to understand and straightforward. @jameszhangyukun Can we load a whole function to Lua runtime instead of a piece of code?

We can. this is the demo call funciton. this is the lua script

function say()
	print("Hello World.")
end

function add( a, b )
	ret1 = a + b
	ret2 = a - b
	return ret1, ret2
end

the go code

package main

import (
	"fmt"
)

import (
	"github.com/yuin/gopher-lua"
)

func main() {

	L := lua.NewState()
	L.DoFile("mylua.lua")
	// Get say function in lua script
	f := L.GetGlobal("say")
	// Push say  function to stack
	L.Push(f)
	// Call Lua Func
	L.Call(0, 0)


	// Get add function in lua script
	f = L.GetGlobal("add")
	L.Push(f)
	// set parameter
	L.Push(lua.LNumber(2))
	L.Push(lua.LNumber(2))
	// Call
	L.Call(2, 2)

	value := L.GetTop()
	fmt.Println(L.Get(value)) // ret2:0
	L.Pop(1)
	value = L.GetTop()
	fmt.Println(L.Get(value)) //re1:4

}

we can use GetGlobalfunction get function in the script

[RainbowMango]

Great!, Let's do it by the function way, I'll update the API description later.

[RainbowMango]

As @ikaven1024 said, if the user applies the following two yaml files, which one will work?

It's obviously an ambiguous situation. I think we can add an admission webhook to deny the second configuration.

[RainbowMango]

Let's do it as per the previous plan and iterate it once we have a better solution.

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment