[ws-manager-mk2] Timeout controller tests

Author: WVerlaek

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

@@ -40,7 +40,7 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace
 			workspace.Status.Phase = workspacev1.WorkspacePhasePending
 		}
 
-		if workspace.Status.Phase != workspacev1.WorkspacePhasePending {
+		if isDisposalFinished(workspace) {
 			workspace.Status.Phase = workspacev1.WorkspacePhaseStopped
 		}
 		return nil
@@ -124,11 +124,7 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace
 		workspace.Status.Phase = workspacev1.WorkspacePhaseStopping
 
 		if controllerutil.ContainsFinalizer(pod, workspacev1.GitpodFinalizerName) {
-			if wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) ||
-				wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)) ||
-				wsk8s.ConditionPresentAndTrue(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionAborted)) ||
-				wsk8s.ConditionWithStatusAndReason(workspace.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady), false, "InitializationFailure") {
-
+			if isDisposalFinished(workspace) {
 				workspace.Status.Phase = workspacev1.WorkspacePhaseStopped
 			}
 
@@ -191,6 +187,13 @@ func updateWorkspaceStatus(ctx context.Context, workspace *workspacev1.Workspace
 	return nil
 }
 
+func isDisposalFinished(ws *workspacev1.Workspace) bool {
+	return wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupComplete)) ||
+		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionBackupFailure)) ||
+		wsk8s.ConditionPresentAndTrue(ws.Status.Conditions, string(workspacev1.WorkspaceConditionAborted)) ||
+		wsk8s.ConditionWithStatusAndReason(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady), false, "InitializationFailure")
+}
+
 // extractFailure returns a pod failure reason and possibly a phase. If phase is nil then
 // one should extract the phase themselves. If the pod has not failed, this function returns "", nil.
 func extractFailure(ws *workspacev1.Workspace, pod *corev1.Pod) (string, *workspacev1.WorkspacePhase) {

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

@@ -20,6 +20,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"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
@@ -28,6 +30,12 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
+const (
+	timeout  = time.Second * 20
+	duration = time.Second * 2
+	interval = time.Millisecond * 250
+)
+
 // var cfg *rest.Config
 var k8sClient client.Client
 var testEnv *envtest.Environment
@@ -39,8 +47,9 @@ func TestAPIs(t *testing.T) {
 }
 
 var (
-	ctx    context.Context
-	cancel context.CancelFunc
+	ctx        context.Context
+	cancel     context.CancelFunc
+	wsActivity *activity.WorkspaceActivity
 )
 
 var _ = BeforeSuite(func() {
@@ -91,21 +100,15 @@ var _ = BeforeSuite(func() {
 	})
 	Expect(err).ToNot(HaveOccurred())
 
-	wsReconciler, err := NewWorkspaceReconciler(k8sManager.GetClient(), k8sManager.GetScheme(), &config.Configuration{
-		Namespace:      "default",
-		SeccompProfile: "default.json",
-		WorkspaceClasses: map[string]*config.WorkspaceClass{
-			"default": {
-				Name: "default",
-			},
-		},
-		WorkspaceURLTemplate: "{{ .ID }}-{{ .Prefix }}-{{ .Host }}",
-		GitpodHostURL:        "gitpod.io",
-	}, metrics.Registry)
-
+	conf := newTestConfig()
+	wsReconciler, err := NewWorkspaceReconciler(k8sManager.GetClient(), k8sManager.GetScheme(), &conf, metrics.Registry)
 	Expect(err).ToNot(HaveOccurred())
-	err = wsReconciler.SetupWithManager(k8sManager)
+	Expect(wsReconciler.SetupWithManager(k8sManager)).To(Succeed())
+
+	wsActivity = &activity.WorkspaceActivity{}
+	timeoutReconciler, err := NewTimeoutReconciler(k8sManager.GetClient(), conf, wsActivity)
 	Expect(err).ToNot(HaveOccurred())
+	Expect(timeoutReconciler.SetupWithManager(k8sManager)).To(Succeed())
 
 	ctx, cancel = context.WithCancel(context.Background())
 
@@ -117,6 +120,32 @@ var _ = BeforeSuite(func() {
 
 })
 
+func newTestConfig() config.Configuration {
+	return config.Configuration{
+		GitpodHostURL:     "gitpod.io",
+		HeartbeatInterval: util.Duration(30 * time.Second),
+		Namespace:         "default",
+		SeccompProfile:    "default.json",
+		Timeouts: config.WorkspaceTimeoutConfiguration{
+			AfterClose:          util.Duration(1 * time.Minute),
+			Initialization:      util.Duration(30 * time.Minute),
+			TotalStartup:        util.Duration(45 * time.Minute),
+			RegularWorkspace:    util.Duration(60 * time.Minute),
+			MaxLifetime:         util.Duration(36 * time.Hour),
+			HeadlessWorkspace:   util.Duration(90 * time.Minute),
+			Stopping:            util.Duration(60 * time.Minute),
+			ContentFinalization: util.Duration(55 * time.Minute),
+			Interrupted:         util.Duration(5 * time.Minute),
+		},
+		WorkspaceClasses: map[string]*config.WorkspaceClass{
+			"default": {
+				Name: "default",
+			},
+		},
+		WorkspaceURLTemplate: "{{ .ID }}-{{ .Prefix }}-{{ .Host }}",
+	}
+}
+
 var _ = AfterSuite(func() {
 	cancel()
 	By("tearing down the test environment")

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

@@ -11,6 +11,8 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -24,6 +26,9 @@ import (
 )
 
 func NewTimeoutReconciler(c client.Client, cfg config.Configuration, activity *wsactivity.WorkspaceActivity) (*TimeoutReconciler, error) {
+	if cfg.HeartbeatInterval == 0 {
+		return nil, fmt.Errorf("invalid heartbeat interval, must not be 0")
+	}
 	reconcileInterval := time.Duration(cfg.HeartbeatInterval)
 	// Reconcile interval is half the heartbeat interval to catch timed out workspaces in time.
 	// See https://en.wikipedia.org/wiki/Nyquist%E2%80%93Shannon_sampling_theorem why we need this.
@@ -92,18 +97,26 @@ func (r *TimeoutReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
 
 	// Workspace timed out, set Timeout condition.
 	log.Info("Workspace timed out", "reason", timedout)
-	workspace.Status.Conditions = wsk8s.AddUniqueCondition(workspace.Status.Conditions, metav1.Condition{
-		Type:               string(workspacev1.WorkspaceConditionTimeout),
-		Status:             metav1.ConditionTrue,
-		LastTransitionTime: metav1.Now(),
-		Reason:             "TimedOut",
-		Message:            timedout,
-	})
-
-	if err = r.Client.Status().Update(ctx, &workspace); err != nil {
+	if err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		err := r.Get(ctx, types.NamespacedName{Name: workspace.Name, Namespace: workspace.Namespace}, &workspace)
+		if err != nil {
+			return err
+		}
+
+		workspace.Status.Conditions = wsk8s.AddUniqueCondition(workspace.Status.Conditions, metav1.Condition{
+			Type:               string(workspacev1.WorkspaceConditionTimeout),
+			Status:             metav1.ConditionTrue,
+			LastTransitionTime: metav1.Now(),
+			Reason:             "TimedOut",
+			Message:            timedout,
+		})
+
+		return r.Status().Update(ctx, &workspace)
+	}); err != nil {
 		log.Error(err, "Failed to update workspace status with Timeout condition")
 		return ctrl.Result{}, err
 	}
+
 	return ctrl.Result{}, nil
 }