Closed chaunceyjiang closed 1 year ago
/cc @ikaven1024 @RainbowMango @XiShanYongYe-Chang
What do you think?
I do't understand what scene need this feature. Could you show a user case?
I do't understand what scene need this feature. Could you show a user case?
Currently, the built-in interpreter only maintains K8S resources. For some well-known third-party projects, Karmada may need to do some maintenance to reduce the cost of using the ConfigurableInterpreter.
For example, AggregateStatus
may be a commonly used InterpreterOperation
@ikaven1024
I do't understand what scene need this feature. Could you show a user case?
Currently, the built-in interpreter only maintains K8S resources. For some well-known third-party projects, Karmada may need to do some maintenance to reduce the cost of using the ConfigurableInterpreter.
For example,
AggregateStatus
may be a commonly usedInterpreterOperation
This interpreters are supplied by karmada, then why not implement them with go
, just like built-in, rather than inefficient lua
why not implement them with go, just like built-in
Because if you use go
to implement it, you need to introduce third-party project APIs, which will lead to more and more dependencies being introduced into Karmada.
why not implement them with go, just like built-in
Because if you use
go
to implement it, you need to introduce third-party project APIs, which will lead to more and more dependencies being introduced into Karmada.
We can use unstructed, no need import real APIs
We can use unstructed
This requires the maintainer of the third-party project to be familiar with the code of Karmada. On the other hand, this mechanism is convenient for the maintainer of the third-party project, not the maintainer of Kamrada.
We can use unstructed
This requires the maintainer of the third-party project to be familiar with the code of Karmada. On the other hand, this mechanism is convenient for the maintainer of the third-party project, not the maintainer of Kamrada.
That's say, these script files are committed by third-party project.
That's say, these script files are committed by third-party project.
Yes, Karmada maintainer or third-party project maintainer.
Hi @chaunceyjiang, would you like to keep this pr going? We have plans to integrate with some well-known projects. I think this pr is needed.
Hi @chaunceyjiang, would you like to keep this pr going?
Sure
Q: Why is such a directory structure set up, and what are the benefits of doing so? A: First, it is easier to classify ResourceInterpreterCustomization for easy reference. Second, we can verify the content in customizations.yaml, just like karmada-webhook.
/cc @Poor12 Please take a look.
Merging #2768 (37b1e3f) into master (e4f5b62) will increase coverage by
0.17%
. The diff coverage is100.00%
.
:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@ Coverage Diff @@
## master #2768 +/- ##
==========================================
+ Coverage 49.15% 49.32% +0.17%
==========================================
Files 206 205 -1
Lines 18377 18274 -103
==========================================
- Hits 9033 9014 -19
+ Misses 8857 8778 -79
+ Partials 487 482 -5
Flag | Coverage Δ | |
---|---|---|
unittests | 49.32% <100.00%> (+0.17%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pkg/karmadactl/interpret/check.go | 91.17% <ø> (ø) |
|
pkg/resourceinterpreter/configurable/luavm/kube.go | 67.39% <ø> (ø) |
|
pkg/resourceinterpreter/configurable/luavm/lua.go | 57.14% <ø> (ø) |
|
...ourceinterpreter/configurable/luavm/lua_convert.go | 73.91% <ø> (ø) |
|
...webhook/resourceinterpretercustomization/helper.go | 82.05% <ø> (ø) |
|
...r/configurableinterpreter/configmanager/manager.go | 67.21% <100.00%> (ø) |
... and 3 files with indirect coverage changes
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Test
I have not created the Interpretercustomizations resource, but you can see that the cloneset has the aggregatedStatus .
I have 1000 YAML files, totaling 7.8 MB in size. When I embedded these files into the karmada-controller-manager executable file, it only increased its file size by about 1 MB.
I have 1000 YAML files, totaling 7.8 MB in size. When I embedded these files into the karmada-controller-manager executable file, it only increased its file size by about 1 MB.
Does 1000 YAML files mean 1000 resources?
Can we organize the code like this? First PR focus on the code structure and let's back to this PR to abstract the common code and new features.
It makes sense.
Does 1000 YAML files mean 1000 resources?
No, there will be some YAML files for testing one type of resource, and each type of resource has about 10 test files. So there are about 100 types of resources.
No, there will be some YAML files for testing one type of resource, and each type of resource has about 10 test files. So there are about 100 types of resources.
100 resources VS 1MB, is totally acceptable I guess.
100 resources VS 1MB, is totally acceptable I guess.
yes, i think so.
Can we organize the code like this?
Can the luavm directory be separated from configureinterpreter?
Can the luavm directory be separated from configureinterpreter?
I think keeping the luavm in pkg/resourceinterpreter/customized/declarative
would be fine, at least it won't block this PR. Let's revisit during or after this PR.
Good job! Could you please post the latest test result(whether the built-in configurableInterpret takes effect and whether users can override the default third-party crds)?
build-in third-party configurableInterpreter:
users can override the third-party configurableInterpreter:
statusAggregation:
luaScript: >
function AggregateStatus(desiredObj, statusItems)
if statusItems == nil then
return desiredObj
end
if desiredObj.status == nil then
desiredObj.status = {}
end
if desiredObj.generation == nil then
desiredObj.generation = 0
end
generation = desiredObj.generation
replicas = 0
updatedReplicas = 0
readyReplicas = 0
availableReplicas = 0
updatedReadyReplicas = 0
expectedUpdatedReplicas = 0
updateRevision = ''
currentRevision = ''
for i = 1, #statusItems do
if statusItems[i].status ~= nil and statusItems[i].status.replicas ~= nil then
replicas = replicas + statusItems[i].status.replicas
end
if statusItems[i].status ~= nil and statusItems[i].status.availableReplicas ~= nil then
availableReplicas = availableReplicas + statusItems[i].status.availableReplicas
end
end
desiredObj.status.observedGeneration = generation
desiredObj.status.replicas = replicas
desiredObj.status.updatedReplicas = updatedReplicas
desiredObj.status.readyReplicas = readyReplicas
desiredObj.status.availableReplicas = availableReplicas
desiredObj.status.updatedReadyReplicas = updatedReadyReplicas
desiredObj.status.expectedUpdatedReplicas = expectedUpdatedReplicas
desiredObj.status.updateRevision = updateRevision
desiredObj.status.currentRevision = currentRevision
return desiredObj
end
krmada control plane:
➤ k get clone -oyaml
/lgtm
/lgtm Leave approve to @XiShanYongYe-Chang
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: XiShanYongYe-Chang
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Signed-off-by: chaunceyjiang chaunceyjiang@gmail.com
What type of PR is this? /kind feature
What this PR does / why we need it:
Predefine ConfigurableInterpreter for some well-known third-party projects.
Which issue(s) this PR fixes: Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: