Refactor last activity to be consistent acrtoss the controllers

Author: aledbf

components/ws-manager-mk2/controllers/timeout_controller.go

@@ -11,7 +11,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
@@ -21,6 +20,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/gitpod-io/gitpod/common-go/util"
+	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/activity"
 	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/maintenance"
 	config "github.com/gitpod-io/gitpod/ws-manager/api/config"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
@@ -155,7 +155,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 	}
 
 	start := ws.ObjectMeta.CreationTimestamp.Time
-	lastActivity := ws.Status.LastActivity
+	lastActivity := activity.Last(ws)
 	isClosed := ws.IsConditionTrue(workspacev1.WorkspaceConditionClosed)
 
 	switch phase {
@@ -187,8 +187,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 		activity := activityNone
 		if ws.IsHeadless() {
 			timeout = timeouts.HeadlessWorkspace
-			nt := metav1.NewTime(start)
-			lastActivity = &nt
+			lastActivity = &start
 			activity = activityRunningHeadless
 		} else if lastActivity == nil {
 			// The workspace is up and running, but the user has never produced any activity
@@ -202,13 +201,13 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 						return ""
 					}
 				}
-				return decide(lastActivity.Time, afterClosed, activityClosed)
+				return decide(*lastActivity, afterClosed, activityClosed)
 			}()
 			if reason != "" {
 				return reason
 			}
 		}
-		return decide(lastActivity.Time, timeout, activity)
+		return decide(*lastActivity, timeout, activity)
 
 	case workspacev1.WorkspacePhaseStopping:
 		if isWorkspaceBeingDeleted(ws) && !ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) {

components/ws-manager-mk2/pkg/activity/activity.go

@@ -0,0 +1,20 @@
+// Copyright (c) 2023 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+package activity
+
+import (
+	"time"
+
+	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
+)
+
+func Last(ws *workspacev1.Workspace) *time.Time {
+	lastActivity := ws.Status.LastActivity
+	if lastActivity != nil {
+		return &lastActivity.Time
+	}
+
+	return nil
+}

components/ws-manager-mk2/service/manager.go

@@ -40,6 +40,7 @@ import (
 	"github.com/gitpod-io/gitpod/common-go/tracing"
 	"github.com/gitpod-io/gitpod/common-go/util"
 	csapi "github.com/gitpod-io/gitpod/content-service/api"
+	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/activity"
 	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/maintenance"
 	wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api"
 	"github.com/gitpod-io/gitpod/ws-manager/api/config"
@@ -464,7 +465,7 @@ func (wsm *WorkspaceManagerServer) DescribeWorkspace(ctx context.Context, req *w
 		Status: wsm.extractWorkspaceStatus(&ws),
 	}
 
-	lastActivity := getLastActivity(&ws)
+	lastActivity := activity.Last(&ws)
 	if lastActivity != nil {
 		result.LastActivity = lastActivity.UTC().Format(time.RFC3339Nano)
 	}
@@ -1485,7 +1486,7 @@ func (wav *workspaceActivityVec) getWorkspaceActivityCounts() (active, notActive
 			continue
 		}
 
-		hasActivity := getLastActivity(&ws) != nil
+		hasActivity := activity.Last(&ws) != nil
 		if hasActivity {
 			active++
 		} else {
@@ -1495,21 +1496,3 @@ func (wav *workspaceActivityVec) getWorkspaceActivityCounts() (active, notActive
 
 	return
 }
-
-func getLastActivity(ws *workspacev1.Workspace) *time.Time {
-	lastActivity := ws.Status.LastActivity
-	if lastActivity != nil {
-		return &lastActivity.Time
-	}
-
-	// In case we don't have a record of the workspace's last activity, check for the FirstUserActivity condition
-	// to see if the lastActivity got lost on a manager restart.
-	if ws.IsConditionTrue(workspacev1.WorkspaceConditionFirstUserActivity) {
-		now := time.Now().UTC()
-		lastActivityStatus := metav1.NewTime(now)
-		return &lastActivityStatus.Time
-	}
-
-	// If the FirstUserActivity condition isn't present we know that the workspace has never had user activity.
-	return nil
-}