From df3e15a1d401f525766db1ee5b22203562fe915a Mon Sep 17 00:00:00 2001 From: Wouter Verlaek Date: Thu, 23 Feb 2023 17:36:53 +0100 Subject: [PATCH] [ws-manager-mk2] Remove headless from status (#16530) --- .../pkg/controller/workspace_controller.go | 2 +- .../ws-manager-api/go/crd/v1/workspace_types.go | 9 +++++++++ components/ws-manager-mk2/controllers/status.go | 8 ++------ .../controllers/timeout_controller.go | 2 +- .../controllers/timeout_controller_test.go | 16 ++++++++++------ .../controllers/workspace_controller_test.go | 11 ++--------- components/ws-manager-mk2/service/manager.go | 2 +- 7 files changed, 26 insertions(+), 24 deletions(-) diff --git a/components/ws-daemon/pkg/controller/workspace_controller.go b/components/ws-daemon/pkg/controller/workspace_controller.go index e66a629b71f94f..5c3a9b2aaade95 100644 --- a/components/ws-daemon/pkg/controller/workspace_controller.go +++ b/components/ws-daemon/pkg/controller/workspace_controller.go @@ -147,7 +147,7 @@ func (wsc *WorkspaceController) handleWorkspaceInit(ctx context.Context, ws *wor InstanceId: ws.Name, }, Initializer: &init, - Headless: ws.Status.Headless, + Headless: ws.IsHeadless(), }) if alreadyInit { diff --git a/components/ws-manager-api/go/crd/v1/workspace_types.go b/components/ws-manager-api/go/crd/v1/workspace_types.go index ec7b6feb6927a1..97774f07d9aed7 100644 --- a/components/ws-manager-api/go/crd/v1/workspace_types.go +++ b/components/ws-manager-api/go/crd/v1/workspace_types.go @@ -250,6 +250,15 @@ type WorkspaceList struct { Items []Workspace `json:"items"` } +// IsHeadless returns whether the workspace is a headless type. +// This is added as a function on the workspace, instead of a field +// in the status, to make it easier to consume and not e.g. have to +// wait for the first reconcile of a workspace to set the status +// resource. +func (w *Workspace) IsHeadless() bool { + return w.Spec.Type != WorkspaceTypeRegular +} + func init() { SchemeBuilder.Register(&Workspace{}, &WorkspaceList{}) } diff --git a/components/ws-manager-mk2/controllers/status.go b/components/ws-manager-mk2/controllers/status.go index e6b3ceac17d896..4bbf557d38b8fa 100644 --- a/components/ws-manager-mk2/controllers/status.go +++ b/components/ws-manager-mk2/controllers/status.go @@ -84,10 +84,6 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace workspace.Status.Runtime.PodName = pod.Name } - if workspace.Spec.Type != workspacev1.WorkspaceTypeRegular { - workspace.Status.Headless = true - } - if workspace.Status.URL == "" { url, err := config.RenderWorkspaceURL(cfg.WorkspaceURLTemplate, workspace.Name, workspace.Spec.Ownership.WorkspaceID, cfg.GitpodHostURL) if err != nil { @@ -172,7 +168,7 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace workspace.Status.Phase = workspacev1.WorkspacePhaseInitializing } - case workspace.Status.Headless && (pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed): + case workspace.IsHeadless() && (pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed): workspace.Status.Phase = workspacev1.WorkspacePhaseStopping case pod.Status.Phase == corev1.PodUnknown: @@ -249,7 +245,7 @@ func extractFailure(ws *workspacev1.Workspace, pod *corev1.Pod) (string, *worksp return fmt.Sprintf("container %s ran with an error: exit code %d", cs.Name, terminationState.ExitCode), &phase } } else if terminationState.Reason == "Completed" && !isPodBeingDeleted(pod) { - if ws.Status.Headless { + if ws.IsHeadless() { // headless workspaces are expected to finish return "", nil } diff --git a/components/ws-manager-mk2/controllers/timeout_controller.go b/components/ws-manager-mk2/controllers/timeout_controller.go index a38b1469480b0c..49c0d6942a1d30 100644 --- a/components/ws-manager-mk2/controllers/timeout_controller.go +++ b/components/ws-manager-mk2/controllers/timeout_controller.go @@ -182,7 +182,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas timeout = util.Duration(customTimeout.Duration) } activity := activityNone - if ws.Status.Headless { + if ws.IsHeadless() { timeout = timeouts.HeadlessWorkspace lastActivity = &start activity = activityRunningHeadless diff --git a/components/ws-manager-mk2/controllers/timeout_controller_test.go b/components/ws-manager-mk2/controllers/timeout_controller_test.go index 34113e5edaeb31..f66a543215f737 100644 --- a/components/ws-manager-mk2/controllers/timeout_controller_test.go +++ b/components/ws-manager-mk2/controllers/timeout_controller_test.go @@ -44,6 +44,7 @@ var _ = Describe("TimeoutController", func() { lastActivityAgo *time.Duration age time.Duration customTimeout *time.Duration + update func(ws *workspacev1.Workspace) updateStatus func(ws *workspacev1.Workspace) controllerRestart time.Time expectTimeout bool @@ -59,11 +60,14 @@ var _ = Describe("TimeoutController", func() { r.activity.Store(ws.Name, now.Add(-*tc.lastActivityAgo)) } - if tc.customTimeout != nil { - updateObjWithRetries(fakeClient, ws, false, func(ws *workspacev1.Workspace) { + updateObjWithRetries(fakeClient, ws, false, func(ws *workspacev1.Workspace) { + if tc.customTimeout != nil { ws.Spec.Timeout.Time = &metav1.Duration{Duration: *tc.customTimeout} - }) - } + } + if tc.update != nil { + tc.update(ws) + } + }) updateObjWithRetries(fakeClient, ws, true, func(ws *workspacev1.Workspace) { ws.Status.Phase = tc.phase if tc.updateStatus != nil { @@ -131,8 +135,8 @@ var _ = Describe("TimeoutController", func() { }), Entry("should timeout headless workspace", testCase{ phase: workspacev1.WorkspacePhaseRunning, - updateStatus: func(ws *workspacev1.Workspace) { - ws.Status.Headless = true + update: func(ws *workspacev1.Workspace) { + ws.Spec.Type = workspacev1.WorkspaceTypePrebuild }, age: 2 * time.Hour, lastActivityAgo: nil, diff --git a/components/ws-manager-mk2/controllers/workspace_controller_test.go b/components/ws-manager-mk2/controllers/workspace_controller_test.go index 7151feead44610..9fdcb685eda6ae 100644 --- a/components/ws-manager-mk2/controllers/workspace_controller_test.go +++ b/components/ws-manager-mk2/controllers/workspace_controller_test.go @@ -54,7 +54,6 @@ var _ = Describe("WorkspaceController", func() { g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws)).To(Succeed()) g.Expect(ws.Status.OwnerToken).ToNot(BeEmpty()) g.Expect(ws.Status.URL).ToNot(BeEmpty()) - g.Expect(ws.Status.Headless).To(BeFalse()) }, timeout, interval).Should(Succeed()) // TODO(wv): Once implemented, expect EverReady condition. @@ -196,14 +195,8 @@ var _ = Describe("WorkspaceController", func() { ws.Spec.Type = workspacev1.WorkspaceTypePrebuild pod = createWorkspaceExpectPod(ws) - // Expect headless status to be reported. - Eventually(func() (bool, error) { - err := k8sClient.Get(ctx, types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}, ws) - if err != nil { - return false, err - } - return ws.Status.Headless, nil - }, timeout, interval).Should(BeTrue()) + // Expect headless. + Expect(ws.IsHeadless()).To(BeTrue()) // Expect runtime status also gets reported for headless workspaces. expectRuntimeStatus(ws, pod) diff --git a/components/ws-manager-mk2/service/manager.go b/components/ws-manager-mk2/service/manager.go index aad08f50cfb699..43f802f6cc2c78 100644 --- a/components/ws-manager-mk2/service/manager.go +++ b/components/ws-manager-mk2/service/manager.go @@ -819,7 +819,7 @@ func extractWorkspaceStatus(ws *workspacev1.Workspace) *wsmanapi.WorkspaceStatus SupervisorRef: ws.Spec.Image.IDE.Supervisor, }, IdeImageLayers: ws.Spec.Image.IDE.Refs, - Headless: ws.Status.Headless, + Headless: ws.IsHeadless(), Url: ws.Status.URL, Type: tpe, Timeout: timeout,