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

provide ability to adjust deployment strategy #156

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

leelavg
Copy link
Contributor

@leelavg leelavg commented Oct 15, 2024

default value is choosen in such a way to make kubernetes replace one replica when there are 2 replicas running on 2 distinct nodes.

since these replicas are already using leader election a loss of 1 replica during rollout doesn't pose any issues.

minor: fixed a typo s/defautUpdateStrategy/defaultUpdateStrategy/

fixes: #140

@leelavg
Copy link
Contributor Author

leelavg commented Oct 15, 2024

Functionality was already tested as part of linked issue, the defaults and change is checked as below

// new defaults
> k get deploy rbd.csi.ceph.com-ctrlplugin -oyaml | yq -r .spec.strategy
rollingUpdate:
  maxSurge: 0
  maxUnavailable: 1
type: RollingUpdate

// no regression in typo change
> k get ds rbd.csi.ceph.com-nodeplugin -oyaml | yq -r .spec.updateStrategy
rollingUpdate:
  maxSurge: 0
  maxUnavailable: 1
type: RollingUpdate

// when driver spec is updated same is realized in the deployment
> k get deploy rbd.csi.ceph.com-ctrlplugin -oyaml | yq -r .spec.strategy
rollingUpdate:
  maxSurge: 25%
  maxUnavailable: 25%
type: RollingUpdate

api/v1alpha1/driver_types.go Outdated Show resolved Hide resolved
@nb-ohad
Copy link
Collaborator

nb-ohad commented Oct 15, 2024

@leelavg Can you please reorganize the commit so the first commit holds the API changes (plus generated changes) then a 2nd commit for the changes that are needed to support the new API field in the controller?

api/v1alpha1/driver_types.go Outdated Show resolved Hide resolved
internal/controller/defaults.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Show resolved Hide resolved
@Madhu-1 Madhu-1 requested a review from nb-ohad October 24, 2024 09:58
@nb-ohad nb-ohad merged commit 5eadea2 into ceph:main Nov 6, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current csi deployments mandate usage of three nodes or else rollout get stuck
3 participants