-
Notifications
You must be signed in to change notification settings - Fork 919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add kube lib for lua #2871
add kube lib for lua #2871
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2871 +/- ##
==========================================
+ Coverage 37.80% 37.90% +0.09%
==========================================
Files 200 201 +1
Lines 18461 18511 +50
==========================================
+ Hits 6980 7016 +36
- Misses 11073 11084 +11
- Partials 408 411 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
ada4814
to
081a1d6
Compare
I like the idea. Sounds like it makes easier for users to write the scripts. |
0744a22
to
b728378
Compare
/assign |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Do we need to update the getReplicas
example in the examples
at the same time?
OK. I will do it. |
b728378
to
6dd348f
Compare
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
Only some nits and questions.
return 1 | ||
} | ||
|
||
func checkResourceQuantity(ls *lua.LState, n int) resource.Quantity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function isn't about to check
, but parse
a resource to resource.Quantity.
Rename to ParseResourceQuantity
or one more accurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This naming follows gopher-lua
's function CheckXxx
karmada/vendor/github.com/yuin/gopher-lua/auxlib.go
Lines 13 to 29 in d4e5f04
func (ls *LState) CheckAny(n int) LValue { | |
if n > ls.GetTop() { | |
ls.ArgError(n, "value expected") | |
} | |
return ls.Get(n) | |
} | |
func (ls *LState) CheckInt(n int) int { | |
v := ls.Get(n) | |
if intv, ok := v.(LNumber); ok { | |
return int(intv) | |
} | |
ls.TypeError(n, LTNumber) | |
return 0 | |
} | |
func (ls *LState) CheckInt64(n int) int64 { |
func resourceAdd(ls *lua.LState) int { | ||
res := resource.Quantity{} | ||
n := ls.GetTop() | ||
for i := 1; i <= n; i++ { | ||
q := checkResourceQuantity(ls, i) | ||
res.Add(q) | ||
} | ||
|
||
s := res.String() | ||
ls.Push(lua.LString(s)) | ||
return 1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this function would be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users may want to accurate resource quantity with their logic, rather accuratePodRequirements
.
b5bdd35
to
3b1a404
Compare
Signed-off-by: yingjinhui <yingjinhui@didiglobal.com>
3b1a404
to
0f5e377
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango 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:
Approvers can indicate their approval by writing |
Signed-off-by: yingjinhui yingjinhui@didiglobal.com
What type of PR is this?
/kind feature
What this PR does / why we need it:
When users use customization to implement
GetReplicas
, it's difficult to accurate the sum of resource quantity. This PR load thekube
library to help do this work.See the demo:
karmada/pkg/resourceinterpreter/configurableinterpreter/luavm/lua_test.go
Lines 65 to 70 in f6a9956
Which issue(s) this PR fixes:
part of #2371
Special notes for your reviewer:
Does this PR introduce a user-facing change?: