-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(ws): set status.pauseTime
on workspace
#26
Conversation
@@ -906,7 +902,15 @@ func generateService(workspace *kubefloworgv1beta1.Workspace, imageConfigSpec ku | |||
// generateWorkspaceStatus generates a WorkspaceStatus for a Workspace | |||
func generateWorkspaceStatus(workspace *kubefloworgv1beta1.Workspace, pod *corev1.Pod) kubefloworgv1beta1.WorkspaceStatus { | |||
status := workspace.Status | |||
|
|||
if workspace.Spec.Paused != nil && *workspace.Spec.Paused { |
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.
This can be simplified:
if workspace.Spec.Paused != nil && *workspace.Spec.Paused { | |
if !*workspace.Spec.Paused { |
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.
really? https://go.dev/play/p/36NkL4Ra68g
package main
import "fmt"
type Spec struct {
Paused *bool
}
func main() {
var spec Spec
if !*spec.Paused {
fmt.Println("We're paused and there's some details here")
}
}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x480410]
goroutine 1 [running]:
main.main()
/tmp/sandbox2639457518/prog.go:13 +0x10
Program exited.
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.
@jiridanek yes, this is fine, because the Kubernetes version will always populate the field with false
if not specified, because we set +kubebuilder:default=false
:
notebooks/workspaces/controller/api/v1beta1/workspace_types.go
Lines 34 to 37 in e4e0aff
// if the workspace is paused (no pods running) | |
//+kubebuilder:validation:Optional | |
//+kubebuilder:default=false | |
Paused *bool `json:"paused,omitempty"` |
workspaces/controller/internal/controller/workspace_controller.go
Outdated
Show resolved
Hide resolved
@Adembc can you please add a unit test. |
I have added unit tests to ensure the pause time is correctly set. |
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
152522b
to
def97ba
Compare
status.pauseTime
on workspace
@Adembc thanks for your work on this, I have rebased this PR and made the following changes in def97ba
|
I am going to merge this now, since all the tests are passing. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR populates the
status.pauseTime
on the Workspace:status.pauseTime
to current Unix epoch when paused.status.pauseTime
to 0 when unpaused.