Remove workspace activity

Author: aledbf

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

@@ -23,7 +23,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	"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/api/config"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
 	//+kubebuilder:scaffold:imports
@@ -50,10 +49,10 @@ func TestAPIs(t *testing.T) {
 }
 
 var (
-	ctx        context.Context
-	cancel     context.CancelFunc
-	wsActivity *activity.WorkspaceActivity
-	wsMetrics  *controllerMetrics
+	ctx    context.Context
+	cancel context.CancelFunc
+	//wsActivity *activity.WorkspaceActivity
+	wsMetrics *controllerMetrics
 )
 
 var _ = BeforeSuite(func() {
@@ -111,8 +110,8 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred())
 	Expect(wsReconciler.SetupWithManager(k8sManager)).To(Succeed())
 
-	wsActivity = activity.NewWorkspaceActivity(conf.Namespace, k8sManager.GetClient())
-	timeoutReconciler, err := NewTimeoutReconciler(k8sManager.GetClient(), k8sManager.GetEventRecorderFor("workspace"), conf, wsActivity, maintenance)
+	//wsActivity = activity.NewWorkspaceActivity(conf.Namespace, k8sManager.GetClient())
+	timeoutReconciler, err := NewTimeoutReconciler(k8sManager.GetClient(), k8sManager.GetEventRecorderFor("workspace"), conf, maintenance)
 	Expect(err).ToNot(HaveOccurred())
 	Expect(timeoutReconciler.SetupWithManager(k8sManager)).To(Succeed())
 

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

@@ -11,6 +11,7 @@ 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"
@@ -20,13 +21,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/gitpod-io/gitpod/common-go/util"
-	wsactivity "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"
 )
 
-func NewTimeoutReconciler(c client.Client, recorder record.EventRecorder, cfg config.Configuration, activity *wsactivity.WorkspaceActivity, maintenance maintenance.Maintenance) (*TimeoutReconciler, error) {
+func NewTimeoutReconciler(c client.Client, recorder record.EventRecorder, cfg config.Configuration, maintenance maintenance.Maintenance) (*TimeoutReconciler, error) {
 	if cfg.HeartbeatInterval == 0 {
 		return nil, fmt.Errorf("invalid heartbeat interval, must not be 0")
 	}
@@ -38,7 +38,6 @@ func NewTimeoutReconciler(c client.Client, recorder record.EventRecorder, cfg co
 	return &TimeoutReconciler{
 		Client:            c,
 		Config:            cfg,
-		activity:          activity,
 		reconcileInterval: reconcileInterval,
 		recorder:          recorder,
 		maintenance:       maintenance,
@@ -53,7 +52,6 @@ type TimeoutReconciler struct {
 	client.Client
 
 	Config            config.Configuration
-	activity          *wsactivity.WorkspaceActivity
 	reconcileInterval time.Duration
 	recorder          record.EventRecorder
 	maintenance       maintenance.Maintenance
@@ -157,7 +155,7 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 	}
 
 	start := ws.ObjectMeta.CreationTimestamp.Time
-	lastActivity := r.activity.GetLastActivity(ws)
+	lastActivity := ws.Status.LastActivity
 	isClosed := ws.IsConditionTrue(workspacev1.WorkspaceConditionClosed)
 
 	switch phase {
@@ -189,7 +187,8 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 		activity := activityNone
 		if ws.IsHeadless() {
 			timeout = timeouts.HeadlessWorkspace
-			lastActivity = &start
+			nt := metav1.NewTime(start)
+			lastActivity = &nt
 			activity = activityRunningHeadless
 		} else if lastActivity == nil {
 			// The workspace is up and running, but the user has never produced any activity
@@ -203,13 +202,13 @@ func (r *TimeoutReconciler) isWorkspaceTimedOut(ws *workspacev1.Workspace) (reas
 						return ""
 					}
 				}
-				return decide(*lastActivity, afterClosed, activityClosed)
+				return decide(lastActivity.Time, afterClosed, activityClosed)
 			}()
 			if reason != "" {
 				return reason
 			}
 		}
-		return decide(*lastActivity, timeout, activity)
+		return decide(lastActivity.Time, timeout, activity)
 
 	case workspacev1.WorkspacePhaseStopping:
 		if isWorkspaceBeingDeleted(ws) && !ws.IsConditionTrue(workspacev1.WorkspaceConditionBackupComplete) {

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

@@ -8,7 +8,6 @@ import (
 	"time"
 
 	wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
-	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/activity"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
 	"github.com/google/uuid"
 	. "github.com/onsi/ginkgo/v2"
@@ -36,7 +35,7 @@ var _ = Describe("TimeoutController", func() {
 			// Use a fake client instead of the envtest's k8s client, such that we can add objects
 			// with custom CreationTimestamps and check timeout logic.
 			fakeClient = fake.NewClientBuilder().WithStatusSubresource(&workspacev1.Workspace{}).WithScheme(k8sClient.Scheme()).Build()
-			r, err = NewTimeoutReconciler(fakeClient, record.NewFakeRecorder(100), conf, activity.NewWorkspaceActivity("default", k8sClient), &fakeMaintenance{enabled: false})
+			r, err = NewTimeoutReconciler(fakeClient, record.NewFakeRecorder(100), conf, &fakeMaintenance{enabled: false})
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -48,20 +47,22 @@ var _ = Describe("TimeoutController", func() {
 			customMaxLifetime *time.Duration
 			update            func(ws *workspacev1.Workspace)
 			updateStatus      func(ws *workspacev1.Workspace)
-			controllerRestart time.Time
-			expectTimeout     bool
+			//controllerRestart time.Time
+			expectTimeout bool
 		}
 		DescribeTable("workspace timeouts",
 			func(tc testCase) {
 				By("creating a workspace")
 				ws := newWorkspace(uuid.NewString(), "default")
 				ws.CreationTimestamp = metav1.NewTime(now.Add(-tc.age))
-				Expect(fakeClient.Create(ctx, ws)).To(Succeed())
 
 				if tc.lastActivityAgo != nil {
-					r.activity.Store(ws.Name, now.Add(-*tc.lastActivityAgo))
+					now := metav1.NewTime(now.Add(-*tc.lastActivityAgo))
+					ws.Status.LastActivity = &now
 				}
 
+				Expect(fakeClient.Create(ctx, ws)).To(Succeed())
+
 				updateObjWithRetries(fakeClient, ws, false, func(ws *workspacev1.Workspace) {
 					if tc.customTimeout != nil {
 						ws.Spec.Timeout.Time = &metav1.Duration{Duration: *tc.customTimeout}
@@ -80,14 +81,6 @@ var _ = Describe("TimeoutController", func() {
 					}
 				})
 
-				// Set controller (re)start time.
-				if tc.controllerRestart.IsZero() {
-					// Bit arbitrary, but default to the controller running for ~2 days.
-					r.activity.ManagerStartedAt = now.Add(-48 * time.Hour)
-				} else {
-					r.activity.ManagerStartedAt = tc.controllerRestart
-				}
-
 				// Run the timeout controller for this workspace.
 				By("running the TimeoutController reconcile()")
 				_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: ws.Name, Namespace: ws.Namespace}})
@@ -159,32 +152,11 @@ var _ = Describe("TimeoutController", func() {
 				lastActivityAgo:   pointer.Duration(1 * time.Minute),
 				expectTimeout:     true,
 			}),
-			Entry("shouldn't timeout after controller restart", testCase{
-				phase: workspacev1.WorkspacePhaseRunning,
-				updateStatus: func(ws *workspacev1.Workspace) {
-					// Add FirstUserActivity condition from 5 hours ago.
-					// From this condition the controller should deduce that the workspace
-					// has had user activity, but since lastActivity is nil, it's been cleared on
-					// a restart. The controller therefore should not timeout the workspace and
-					// wait for new user activity. Or timeout once user activity doesn't come
-					// eventually after the controller restart.
-					ws.Status.Conditions = wsk8s.AddUniqueCondition(ws.Status.Conditions, metav1.Condition{
-						Type:               string(workspacev1.WorkspaceConditionFirstUserActivity),
-						Status:             metav1.ConditionTrue,
-						LastTransitionTime: metav1.NewTime(now.Add(-5 * time.Hour)),
-					})
-				},
-				age:               5 * time.Hour,
-				lastActivityAgo:   nil, // No last activity recorded yet after controller restart.
-				controllerRestart: now,
-				expectTimeout:     false,
-			}),
 			Entry("should timeout after controller restart if no FirstUserActivity", testCase{
-				phase:             workspacev1.WorkspacePhaseRunning,
-				age:               5 * time.Hour,
-				lastActivityAgo:   nil, // No last activity recorded yet after controller restart.
-				controllerRestart: now,
-				expectTimeout:     true,
+				phase:           workspacev1.WorkspacePhaseRunning,
+				age:             5 * time.Hour,
+				lastActivityAgo: nil, // No last activity recorded yet after controller restart.
+				expectTimeout:   true,
 			}),
 			Entry("should timeout eventually with no user activity after controller restart", testCase{
 				phase: workspacev1.WorkspacePhaseRunning,
@@ -195,10 +167,9 @@ var _ = Describe("TimeoutController", func() {
 						LastTransitionTime: metav1.NewTime(now.Add(-5 * time.Hour)),
 					})
 				},
-				age:               5 * time.Hour,
-				lastActivityAgo:   nil,
-				controllerRestart: now.Add(-2 * time.Hour),
-				expectTimeout:     true,
+				age:             5 * time.Hour,
+				lastActivityAgo: nil,
+				expectTimeout:   true,
 			}),
 		)
 	})
@@ -207,7 +178,7 @@ var _ = Describe("TimeoutController", func() {
 		var r *TimeoutReconciler
 		BeforeEach(func() {
 			var err error
-			r, err = NewTimeoutReconciler(k8sClient, record.NewFakeRecorder(100), newTestConfig(), activity.NewWorkspaceActivity("default", k8sClient), &fakeMaintenance{enabled: false})
+			r, err = NewTimeoutReconciler(k8sClient, record.NewFakeRecorder(100), newTestConfig(), &fakeMaintenance{enabled: false})
 			Expect(err).ToNot(HaveOccurred())
 		})
 

components/ws-manager-mk2/main.go

@@ -168,13 +168,13 @@ func main() {
 		}
 	}()
 
-	timeoutReconciler, err := controllers.NewTimeoutReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("workspace"), cfg.Manager, activity, maintenanceReconciler)
+	timeoutReconciler, err := controllers.NewTimeoutReconciler(mgr.GetClient(), mgr.GetEventRecorderFor("workspace"), cfg.Manager, maintenanceReconciler)
 	if err != nil {
 		setupLog.Error(err, "unable to create timeout controller", "controller", "Timeout")
 		os.Exit(1)
 	}
 
-	wsmanService, err := setupGRPCService(cfg, mgr.GetClient(), activity, maintenanceReconciler)
+	wsmanService, err := setupGRPCService(cfg, mgr.GetClient(), maintenanceReconciler)
 	if err != nil {
 		setupLog.Error(err, "unable to start manager service")
 		os.Exit(1)
@@ -219,7 +219,7 @@ func main() {
 	}
 }
 
-func setupGRPCService(cfg *config.ServiceConfiguration, k8s client.Client, activity *activity.WorkspaceActivity, maintenance maintenance.Maintenance) (*service.WorkspaceManagerServer, error) {
+func setupGRPCService(cfg *config.ServiceConfiguration, k8s client.Client, maintenance maintenance.Maintenance) (*service.WorkspaceManagerServer, error) {
 	// TODO(cw): remove use of common-go/log
 
 	if len(cfg.RPCServer.RateLimits) > 0 {
@@ -275,7 +275,7 @@ func setupGRPCService(cfg *config.ServiceConfiguration, k8s client.Client, activ
 		imgbldr.RegisterImageBuilderServer(grpcServer, imgproxy.ImageBuilder{D: imgbldr.NewImageBuilderClient(conn)})
 	}
 
-	srv := service.NewWorkspaceManagerServer(k8s, &cfg.Manager, metrics.Registry, activity, maintenance)
+	srv := service.NewWorkspaceManagerServer(k8s, &cfg.Manager, metrics.Registry, maintenance)
 
 	grpc_prometheus.Register(grpcServer)
 	wsmanapi.RegisterWorkspaceManagerServer(grpcServer, srv)