Skip to content
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

feat: Add TemplatesStatus field to InstanceSetStatus #7781

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

YTGhost
Copy link
Contributor

@YTGhost YTGhost commented Jul 11, 2024

resolve #7400

@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines. label Jul 11, 2024
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Jul 11, 2024
Signed-off-by: Liang Deng <283304489@qq.com>
@apecloud-bot apecloud-bot added pre-approve Fork PR Pre Approve Test and removed pre-approve Fork PR Pre Approve Test labels Jul 15, 2024
@YTGhost YTGhost requested review from weicao and free6om July 15, 2024 03:47
@YTGhost
Copy link
Contributor Author

YTGhost commented Jul 15, 2024

@weicao @free6om Two more points might need to be discussed:

  1. In the design of the KB, the InstanceSet is used as an internal object, so should the TemplatesStatus also be mapped to the upper-level Component?
  2. As the number of InstanceTemplates increases, the data displayed here will also increase. Is it necessary to add a switch to let users choose whether to enable TemplatesStatus?

@weicao
Copy link
Contributor

weicao commented Jul 15, 2024

@weicao @free6om Two more points might need to be discussed:

  1. In the design of the KB, the InstanceSet is used as an internal object, so should the TemplatesStatus also be mapped to the upper-level Component?

The Component is also an internal object. I think it's all right to observe&watch the status of internal objects. So it's fine to put TemplatesStatus in InstanceSet right now.

  1. As the number of InstanceTemplates increases, the data displayed here will also increase. Is it necessary to add a switch to let users choose whether to enable TemplatesStatus?

Will the InstanceSet CR exceed maximum size in kuaishou's environment if TemplatesStatus is added?

@free6om
Copy link
Contributor

free6om commented Jul 15, 2024

@weicao @free6om Two more points might need to be discussed:

  1. In the design of the KB, the InstanceSet is used as an internal object, so should the TemplatesStatus also be mapped to the upper-level Component?

I would treat the main target of this PR as to resolve the Karmada Interpreter related issue, and prefer to keep the top Cluster&Component APIs as simple as possible.
We can discuss the observation of spec.componentSpecs[x].instances of Cluster API level in another issue.

  1. As the number of InstanceTemplates increases, the data displayed here will also increase. Is it necessary to add a switch to let users choose whether to enable TemplatesStatus?

Base on one test we did before, 200 items of TemplatesStatus is a safe bar against object size, you can do an evaluation of your use scenario.

@YTGhost
Copy link
Contributor Author

YTGhost commented Jul 15, 2024

I would treat the main target of this PR as to resolve the Karmada Interpreter related issue, and prefer to keep the top Cluster&Component APIs as simple as possible.
We can discuss the observation of spec.componentSpecs[x].instances of Cluster API level in another issue.

Sure

Base on one test we did before, 200 items of TemplatesStatus is a safe bar against object size, you can do an evaluation of your use scenario.

We have at most a few dozen templates in our scenario, so this is completely sufficient.

@free6om So, it looks like we can merge this PR now?

@free6om free6om merged commit 9daafb7 into apecloud:main Jul 16, 2024
16 checks passed
@github-actions github-actions bot added this to the Release 0.8.4 milestone Jul 16, 2024
@free6om
Copy link
Contributor

free6om commented Jul 16, 2024

/cherry-pick release-0.9

Copy link

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/9951040374

github-actions bot pushed a commit that referenced this pull request Jul 16, 2024
Signed-off-by: Liang Deng <283304489@qq.com>
(cherry picked from commit 9daafb7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-interaction pre-approve Fork PR Pre Approve Test size/L Denotes a PR that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Features] Add a TemplatesStatus field to InstanceSetStatus
4 participants