-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: Devbox Scheduled shutdown. #5559
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
base: main
Are you sure you want to change the base?
feat: Devbox Scheduled shutdown. #5559
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5559 +/- ##
=======================================
Coverage 61.97% 61.97%
=======================================
Files 8 8
Lines 647 647
=======================================
Hits 401 401
Misses 200 200
Partials 46 46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
return ctrl.Result{}, err | ||
} | ||
|
||
if scheduledShutdown.Status.State == devboxv1alpha1.ShutdownStateCompleted { |
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.
考虑删除掉完成的
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.
already fix it.
|
||
const ( | ||
// ShutdownStatePending indicates the shutdown is scheduled but not yet executed | ||
ShutdownStatePending ShutdownState = "Pending" |
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.
ShutdownState 名字不合适,crd的名字也不合适 crd叫 DevBoxSchedue,这里的state叫ScheduleState吧
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.
因为devbox有stop/shutdown两种状态了
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.
already fix it.
|
||
// Format: RFC3339 (e.g., "2006-01-02T15:04:05Z") | ||
// +kubebuilder:validation:Required | ||
ShutdownTime metav1.Time `json:"shutdownTime"` |
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.
这里应该是 scheduledTime
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.
already fix it.
|
||
// +kubebuilder:validation:Required | ||
// +kubebuilder:default=Stopped | ||
ShutdownType ShutdownType `json:"shutdownType"` |
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.
类型预留shutdown,或者考虑支持更多,比如 定时、运行多久后停止的类型这种
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.
already fix it.
// ScheduledShutdownSpec defines the desired state of ScheduledShutdown. | ||
type ScheduledShutdownSpec struct { | ||
// +kubebuilder:validation:Required | ||
DevBoxName string `json:"devBoxName"` |
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.
这里新增devbox uid
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.
already fix it.
type ScheduledShutdownStatus struct { | ||
// State represents the current state of the scheduled shutdown | ||
// +kubebuilder:default=Unknown | ||
State ShutdownState `json:"state,omitempty"` |
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.
加上enum
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.
already fix it.
} | ||
|
||
var devbox devboxv1alpha1.Devbox | ||
if err := r.Get(ctx, types.NamespacedName{Name: scheduledShutdown.Spec.DevBoxName, Namespace: scheduledShutdown.Namespace}, &devbox); err != nil { |
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.
添加devbox uid后判断uid是否一致
if err := r.Get(ctx, types.NamespacedName{Name: scheduledShutdown.Spec.DevBoxName, Namespace: scheduledShutdown.Namespace}, &devbox); err != nil { | ||
if errors.IsNotFound(err) { | ||
logger.Error(err, "DevBox not found", "DevBoxName", scheduledShutdown.Spec.DevBoxName) | ||
return r.updateStatus(ctx, &scheduledShutdown, devboxv1alpha1.ShutdownStateUnknown) |
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.
这里增加状态,uuid不匹配/devbox没找到的状态,既然能找到原因就将原因记录下来
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.
already fix it.
return ctrl.Result{}, nil | ||
} | ||
|
||
// calculateRequeueTime 计算下一次重新队列的时间间隔,使用递减策略 |
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.
使用英文注释
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.
already fix it.
|
||
// calculateRequeueTime 计算下一次重新队列的时间间隔,使用递减策略 | ||
func calculateRequeueTime(now time.Time, shutdownTime time.Time) time.Duration { | ||
totalWaitDuration := shutdownTime.Sub(now) |
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.
如果是负的了怎么办?
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.
关机时间判断抽离出函数,然后多写些测试用例测试
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.
already fix it.
shutdownTime := scheduledShutdown.Spec.ShutdownTime.Time | ||
if now.After(shutdownTime) { | ||
logger.Info("Shutdown time reached, performing shutdown", "DevBoxName", scheduledShutdown.Spec.DevBoxName, "ShutdownType", scheduledShutdown.Spec.ShutdownType) | ||
//shutdown | ||
if err := r.performShutdown(ctx, &devbox, scheduledShutdown.Spec.ShutdownType); err != nil { | ||
logger.Error(err, "Failed to perform shutdown", "DevBoxName", scheduledShutdown.Spec.DevBoxName) | ||
return r.updateStatus(ctx, &scheduledShutdown, devboxv1alpha1.ShutdownStateUnknown) | ||
} | ||
return r.updateStatus(ctx, &scheduledShutdown, devboxv1alpha1.ShutdownStateCompleted) | ||
} |
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.
添加注释在这一段
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.
already fix it.
default: | ||
logger.Error(errors.NewBadRequest("Unknown shutdown type"), "DevBoxName", devbox.Name, "ShutdownType", shutdownType) | ||
} | ||
return r.Update(ctx, devbox) |
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.
这里 得考虑在reconcile过程中devbox变化,以及 default状态下就不需要更新devbox了
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.
part fix.
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.
already fix it.
var devboxSchedule devboxv1alpha1.DevBoxSchedule | ||
if err := r.Get(ctx, req.NamespacedName, &devboxSchedule); err != nil { | ||
if errors.IsNotFound(err) { | ||
logger.Info("DevboxSchedule resource not found. Ignoring since object must be deleted") |
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.
typo error.
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.
already fix it.
if err := r.Get(ctx, types.NamespacedName{Name: devboxSchedule.Spec.DevBoxName, Namespace: devboxSchedule.Namespace}, &devbox); err != nil { | ||
if errors.IsNotFound(err) { | ||
logger.Error(err, "DevBox not found", "DevBoxName", devboxSchedule.Spec.DevBoxName) | ||
return r.updateStatus(ctx, &devboxSchedule, devboxv1alpha1.ScheduleStateNotFound) |
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.
check uuid here.
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.
already fix it.
if devboxSchedule.Status.State == devboxv1alpha1.ScheduleStateCompleted { | ||
logger.Info("Schedule already completed, deleting the CR") | ||
if err := r.Delete(ctx, &devboxSchedule); err != nil { | ||
logger.Error(err, "Failed to delete completed DevboxSchedule") | ||
return ctrl.Result{}, err | ||
} | ||
logger.Info("Successfully deleted completed DevboxSchedule") | ||
return ctrl.Result{}, nil | ||
} |
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.
对只有pending状态的才去get devbox, 这里使用switch判断下schedule状态做具体操作
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.
already fix it.
func calculateRequeueTime(now time.Time, shutdownTime time.Time) time.Duration { | ||
totalWaitDuration := shutdownTime.Sub(now) | ||
var requeueAfter time.Duration | ||
switch { | ||
case totalWaitDuration > 24*time.Hour: | ||
requeueAfter = 24 * time.Hour | ||
case totalWaitDuration > 6*time.Hour: | ||
requeueAfter = 6 * time.Hour | ||
case totalWaitDuration > 1*time.Hour: | ||
requeueAfter = 1 * time.Hour | ||
case totalWaitDuration > 10*time.Minute: | ||
requeueAfter = 10 * time.Minute | ||
default: | ||
requeueAfter = totalWaitDuration | ||
} | ||
if now.Add(requeueAfter).After(shutdownTime) { | ||
requeueAfter = totalWaitDuration | ||
} | ||
return requeueAfter | ||
} |
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.
尽量别硬编码,或者换种方式的硬编码,阶梯式弄个数组出来判断
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.
already fix it.
This issue has been automatically closed because we haven't heard back for more than 60 days, please reopen this issue if necessary. |
No description provided.