-
Notifications
You must be signed in to change notification settings - Fork 47
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
✨ VirtualMachineReplicaSet CRUD/Scale Up Scale Down support #528
✨ VirtualMachineReplicaSet CRUD/Scale Up Scale Down support #528
Conversation
efbfb3b
to
65c20f8
Compare
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
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.
First pass
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
65c20f8
to
9459c90
Compare
9459c90
to
0e5ebbe
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. Thanks for addressing all the review comments.
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go
Outdated
Show resolved
Hide resolved
1aa8d27
to
7be3273
Compare
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Outdated
Show resolved
Hide resolved
controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller.go
Show resolved
Hide resolved
7be3273
to
df257bf
Compare
This commit adds support for creating, deleting and scaling a VirtualMachineReplicaSet resource. When a VirtualMachineReplicaSet is created, it is reconciled to create the desired number of virtual machine replicas specified in its spec. Similarly, during scale out/scale in, the controller reconciles the replicas to match the desired state by either creating or deleting VirtualMachine objects. Note that a new replica is only created when the controller detect that it requires a new replica (as opposed to a rollout that is triggered via modification of the template spec in a Deployment). When the VirtualMachineReplicaSet resource is deleted, all of its owned virtual machine replicas are also cleaned up. The controller relies on the Kubernetes garbage collection mechanism for this. This change also adds support for deletion policies which can used used to determine how the controller picks a virtual machine to delete from the list of replicas that it controls during a scale in/down. The only supported policy for now is "Random" where all VMs that are not in deleting state get equal weight and the controller can pick any one of them randomly. If a VM is already marked for deletion, that gets the highest deletion priority. In several places, I have added a TODO to revisit since ReplicaSets offer an ability to signal when an application can be considered "available". That is dependent on a "Ready" condition that is exposed by the replicas. Since we don't have a consistent way to do that, I have removed the support for indicating availability of a VM replica set in the Status. Consequently, the `MinReadySeconds` field from the spec is also removed since that is used to signal availability. These can be added in a followup. Testing Done: 1. Create a VirtualMachineReplicaSet 2. Get the resource to ensure all replicas ready ``` k get vmreplicaset -n parunesh-ns test-rs NAME REPLICAS READY AGE test-rs 3 3 3m57s ``` 3. Scale down the replica set ``` k scale vmreplicaset -n parunesh-ns test-rs --replicas=1 virtualmachinereplicaset.vmoperator.vmware.com/test-rs scaled ``` 4. Ensure scale down has cleaned up replicas: ``` k get vmreplicaset -n parunesh-ns test-rs -o wide NAME REPLICAS READY AGE test-rs 1 1 5m15s k get vm -n parunesh-ns NAME POWER-STATE AGE test-rs-zmdmm PoweredOn 3m38s ``` 5. Scale up the replica set ``` k scale vmreplicaset -n parunesh-ns test-rs --replicas=2 virtualmachinereplicaset.vmoperator.vmware.com/test-rs scaled ``` 6. Ensure scaled up has created new replicas ``` k get vmreplicaset -n parunesh-ns test-rs -o wide NAME REPLICAS READY AGE test-rs 2 2 7m1s k get vm -n parunesh-ns NAME POWER-STATE AGE test-rs-r9slq PoweredOn 44s test-rs-zmdmm PoweredOn 4m57s ``` 7. Delete the replica set and ensure all replicas are deleted. ``` k delete vmreplicaset -n parunesh-ns test-rs virtualmachinereplicaset.vmoperator.vmware.com "test-rs" deleted k get vm -n parunesh-ns No resources found in parunesh-ns namespace. ```
df257bf
to
977e6a7
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, thank you!
|
What does this PR do, and why is it needed?
This commit adds support for creating, deleting and scaling a VirtualMachineReplicaSet resource. When a VirtualMachineReplicaSet is created, it is reconciled to create the desired number of virtual machine replicas specified in its spec.
Similarly, during scale out/scale in, the controller reconciles the replicas to match the desired state by either creating or deleting VirtualMachine objects. Note that a new replica is only created when the controller detect that it requires a new replica (as opposed to a rollout that is triggered via modification of the template spec in a Deployment).
When the VirtualMachineReplicaSet resource is deleted, all of its owned virtual machine replicas are also cleaned up. The controller relies on the Kubernetes garbage collection mechanism for this.
This change also adds support for deletion policies which can used used to determine how the controller picks a virtual machine to delete from the list of replicas that it controls during a scale in/down. The only supported policy for now is "Random" where all VMs that are not in deleting state get equal weight and the controller can pick any one of them randomly. If a VM is already marked for deletion, that gets the highest deletion priority.
In several places, I have added a TODO to revisit since ReplicaSets offer an ability to signal when an application can be considered "available". That is dependent on a "Ready" condition that is exposed by the replicas. Since we don't have a consistent way to do that, I have removed the support for indicating availability of a VM replica set in the Status. Consequently, the
MinReadySeconds
field from the spec is also removed since that is used to signal availability. These can be added in a followup.Testing Done:
Create a VirtualMachineReplicaSet
Get the resource to ensure all replicas ready
Are there any special notes for your reviewer:
None.
Please add a release note if necessary: