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.

Sign up for GitHub

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

Jump to bottom

chore: increase the range of maxReplicasLimit and set default value o… #7953

Merged
merged 1 commit into from
Aug 13, 2024
Merged

chore: increase the range of maxReplicasLimit and set default value o… #7953

merged 1 commit into from
Aug 13, 2024

Conversation

ian-hui
Copy link
Contributor

@ian-hui ian-hui commented Aug 8, 2024

resolve issue #7941

@CLAassistant
Copy link

CLAassistant commented Aug 8, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines. label Aug 8, 2024
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Aug 8, 2024
@ian-hui ian-hui force-pushed the chore-improve-replicaslimit branch from 8bb8de7 to baf4605 Compare August 8, 2024 12:47
@github-actions github-actions bot removed the size/L Denotes a PR that changes 100-499 lines. label Aug 8, 2024
@apecloud-bot apecloud-bot removed the pre-approve Fork PR Pre Approve Test label Aug 8, 2024
@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines. label Aug 8, 2024
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Aug 8, 2024
@ian-hui ian-hui force-pushed the chore-improve-replicaslimit branch from baf4605 to 6ab4788 Compare August 8, 2024 12:54
@apecloud-bot apecloud-bot added pre-approve Fork PR Pre Approve Test and removed pre-approve Fork PR Pre Approve Test labels Aug 8, 2024
@ian-hui ian-hui force-pushed the chore-improve-replicaslimit branch from 6ab4788 to 68b70ac Compare August 8, 2024 12:56
@apecloud-bot apecloud-bot added pre-approve Fork PR Pre Approve Test and removed pre-approve Fork PR Pre Approve Test labels Aug 8, 2024
nayutah
nayutah reviewed Aug 10, 2024
View reviewed changes
apis/apps/v1alpha1/componentdefinition_types.go Outdated
// +kubebuilder:validation:XValidation:rule="self.minReplicas <= self.maxReplicas",message="the minimum replicas limit should be no greater than the maximum"
type ReplicasLimit struct {
// The minimum limit of replicas.
//
// +kubebuilder:validation:Required
MinReplicas int32 `json:"minReplicas"`
// +kubebuilder:default=1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default 0 is a better choice when you scale a component replicas to 0 and get limit validation errors. Then you will have to update the component spec to admit such case. Anyway, default min 1 has too much implicit assumptions inside.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scale-in operations will delete PVCs.

Setting the minimum replica number to 1 is intended to prevent accidental deletion and data loss.

@ian-hui ian-hui force-pushed the chore-improve-replicaslimit branch from 68b70ac to 1924454 Compare August 12, 2024 06:33
@apecloud-bot apecloud-bot removed the pre-approve Fork PR Pre Approve Test label Aug 12, 2024
@github-actions github-actions bot added size/S Denotes a PR that changes 10-29 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Aug 12, 2024
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Aug 12, 2024
chore: increase the range of maxReplicasLimit and set default value o…
41d371a 
…f replicasLimit

Signed-off-by: ian-hui <ianhuiii@sjtu.edu.cn>
@ian-hui ian-hui force-pushed the chore-improve-replicaslimit branch from 1924454 to 41d371a Compare August 12, 2024 07:34
@apecloud-bot apecloud-bot removed the pre-approve Fork PR Pre Approve Test label Aug 12, 2024
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Aug 12, 2024
@wangyelei wangyelei merged commit 09bc7e4 into apecloud:main Aug 13, 2024
22 checks passed
@wangyelei
Copy link
Contributor

/cherry-pick release-0.9

@github-actions github-actions bot added this to the Release 0.9.2 milestone Aug 13, 2024
@github-actions GitHub Actions
Copy link

🤖 says: Error cherry-picking.

Auto-merging apis/apps/v1alpha1/componentdefinition_types.go
Auto-merging config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml
Auto-merging controllers/apps/transformer_component_validation.go
CONFLICT (content): Merge conflict in controllers/apps/transformer_component_validation.go
Auto-merging deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml
error: could not apply 09bc7e4... chore: increase the range of maxReplicasLimit and set default value o… (#7953)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

@github-actions GitHub Actions
Copy link

🤖 says: ‼️ cherry pick action failed.
See: https://github.com/apecloud/kubeblocks/actions/runs/10362466761

@YTGhost YTGhost mentioned this pull request Aug 13, 2024
Closed
wangyelei pushed a commit that referenced this pull request Aug 13, 2024
@ian-hui @wangyelei
chore: increase the range of maxReplicasLimit and set default value o… (
19fe93e 
#7953)

Signed-off-by: ian-hui <ianhuiii@sjtu.edu.cn>
Co-authored-by: ian-hui <ianhuiii@sjtu.edu.cn>
(cherry picked from commit 09bc7e4)
zhuyi1159 pushed a commit to zhuyi1159/kubeblocks that referenced this pull request Aug 21, 2024
@ian-hui @zhuyi1159
chore: increase the range of maxReplicasLimit and set default value o… (
0e9eace 
apecloud#7953)

Signed-off-by: ian-hui <ianhuiii@sjtu.edu.cn>
Co-authored-by: ian-hui <ianhuiii@sjtu.edu.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nayutah nayutah nayutah left review comments

@free6om free6om free6om approved these changes

@weicao weicao weicao approved these changes

@ldming ldming Awaiting requested review from ldming

@heng4fun heng4fun Awaiting requested review from heng4fun

@wangyelei wangyelei Awaiting requested review from wangyelei

@Y-Rookie Y-Rookie Awaiting requested review from Y-Rookie Y-Rookie is a code owner

Assignees
No one assigned
Labels
pre-approve Fork PR Pre Approve Test size/S Denotes a PR that changes 10-29 lines.
Projects
None yet
Milestone
Release 0.9.2
Development

Successfully merging this pull request may close these issues.
7 participants
@ian-hui @CLAassistant @wangyelei @weicao @free6om @nayutah @apecloud-bot