vrg: remove the use of captureStartConditionally #1532
Conversation
raghavendra-talur
changed the title
There was a problem hiding this comment.
This changes the behaviour and make the code more complicated.
I think we want to keep captureAsPrimary and captureAsSecondary fucntions, but call them directly instead of calling them via the protect function.
The issue in this code is trying too hard to avoid code duplication. The most important thing is creating clear and easy to follow code, not avoiding duplication. So the way to clean it up is to inline everything until we have simple and clear code, and then extract helpers for common code used in both primary and secondary cases.
We don't want complex functions that try to do the right thing in all possible actions and replication states. Instead we need simple function that handled exactly one replication state and action.
| } | ||
|
|
||
| func (v *VRGInstance) kubeObjectsProtectSecondary(result *ctrl.Result) { | ||
| v.kubeObjectsProtect(result, kubeObjectsCaptureStartConditionallySecondary) | ||
| v.kubeObjectsProtect(result) |
There was a problem hiding this comment.
If we remove the argument, kubeObjectsProtectPrimary() and kubeObjectsProtectSecondary() are the same, so we should drop them an call kubeObjectsProtect() directly.
There was a problem hiding this comment.
Refactored into one function that performs the capture based on the vrg
spec.
Sounds like the right way.
| @@ -95,14 +92,12 @@ func (v *VRGInstance) kubeObjectsProtect( | |||
| } | |||
|
|
|||
| v.kubeObjectsCaptureStartOrResumeOrDelay(result, | |||
| captureStartConditionally, | |||
| captureToRecoverFrom, | |||
| ) | |||
| log.Info("Kube objects capture for relocate complete") | ||
| // Capture one last time for secondary before relocating | ||
| if v.instance.Spec.ReplicationState == ramen.Secondary && | ||
| v.instance.Spec.Action != ramen.VRGActionRelocate { |
There was a problem hiding this comment.
Where this condition came from? it was not in the previous code. This looks like the same issue as the previous PR - changing the behavior during refactoring.
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
This is going to be useful to inspect the state of the cluster after a test fails. Set the env variable SKIP_CLEANUP to "true" or "1" to skip the cleanup of the environment to make use of the kubeconfig. The kubeconfig is dumped in the testbin directory. After debugging you will have to kill the testEnv. The two binaries that are used by the testEnv are etcd and kube-apiserver. Check for the running process with a command like ps aux | grep -e kube-apiserver -e etcd Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
On Mac, the check for certs is more strict and it fails for submariner service. Turning off the check for certs. More info: golang/go#51991 Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
captureInProgressStatusUpdate was a function pointer that was an empty function when the VRG was being reconciled as Primary and a call to VRGConditionReasonUploading with status false when reconciling as Secondary in the relocate case. Simplify the code by removing the function pointer. * Remove the captureInProgressStatusUpdate parameter from kubeObjectsProtect kubeObjectsCaptureStartOrResumeOrDelay kubeObjectsCaptureStartOrResume kubeObjectsGroupCapture. Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
captureStartConditionally was a function pointer that was * capturing kube objects one last time when the vrg was secondary * capturing kube objects if time since last capture was greater than the capture interval Refactored into one function that performs the capture based on the vrg spec. Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
raghavendra-talur
force-pushed
the
rtalur-remove-function-pointer-2
branch
from
112096b
to
36ad6ee
Compare
captureStartConditionally was a function pointer that was
capture interval
Refactored into one function that performs the capture based on the vrg
spec.