SCF-830: Added state management for cluster hibernation#17
Conversation
Coverage Report for CI Build 27944349215Warning No base build found for commit Coverage: 45.038%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
serdardalgic
left a comment
serdardalgic
left a comment
There was a problem hiding this comment.
Some minor issues @adshin21. After that, I would like to review it again.
| if c.Statefulset == nil || c.Statefulset.Spec.Replicas == nil { | ||
| return nil | ||
| } | ||
| return c.Statefulset.Spec.Replicas |
There was a problem hiding this comment.
| return c.Statefulset.Spec.Replicas | |
| return c.Statefulset.Status.Replicas |
Shouldn't this be Status as you want to get the current replica count of the StatefulSet?
| // Handle lifecycle transitions (hibernate/wake-up) | ||
| handled, err := c.handleHibernateAndWakeUp(newSpec) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if handled { | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
You can move this section before the SetPostgresCRDStatus call, I think.
Now, you're setting Updating before the lifecycle detection. That's why the manageHibernateState function call inside the handleHibernateAndWakeUp function is receiving status: "Updating", instead of "Stopped". That's why you're using the isWakingUp logic in detectLifecycleTransition function, right?
AFAICS, The "persist" functions persistHibernateTransition and persistWakeUpTransition do not depend on the "Updating" write, as they call UpdatePostgresCR first to get a fresh ResourceVersion to update.
So, my suggestion is:
blocked, err := c.blockLifecycleUpdate(newSpec)
...
handled, err := c.handleHibernateAndWakeUp(newSpec) // moved up
...
newSpec.Status.PostgresClusterStatus = acidv1.ClusterStatusUpdating
...
newSpec, err = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), newSpec) // does the normal update path
After that, you should be able to clean up the isWakingUp logic and have a cleaner detectLifecycleTransition function.
| const ( | ||
| LifecycleActionNone LifecycleAction = iota | ||
| LifecycleActionHibernate // Running -> Stopping (initiate hibernate) | ||
| LifecycleActionStoppingCompleted // Stopping -> Stopped (pods fully terminated) |
There was a problem hiding this comment.
Are you using LifecycleActionStoppingCompleted ? If not, please remove it.
3 participants
No description provided.