[ws-manager-mk2] Review comments

- Name controller
- Return early in case of error
- Reduce scope of permissions

Author: Furisto

components/ws-daemon/pkg/controller/snapshot_controller.go

@@ -36,6 +36,7 @@ func NewSnapshotController(c client.Client, nodeName string, wso *WorkspaceOpera
 // SetupWithManager sets up the controller with the Manager.
 func (r *SnapshotReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
+		Named("snapshot").
 		For(&workspacev1.Snapshot{}).
 		WithEventFilter(snapshotEventFilter(r.nodeName)).
 		Complete(r)
@@ -64,11 +65,6 @@ func snapshotEventFilter(nodeName string) predicate.Predicate {
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
-// TODO(user): Modify the Reconcile function to compare the state specified by
-// the Snapshot object against the actual cluster state, and then
-// perform operations to make the cluster state reflect the state specified by
-// the user.
-//
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 func (ssc *SnapshotReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {

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

@@ -549,7 +549,11 @@ func (wsm *WorkspaceManagerServer) TakeSnapshot(ctx context.Context, req *wsmana
 			return false, err
 		}
 
-		if sso.Status.URL != "" && sso.Status.Error == "" {
+		if sso.Status.Error != "" {
+			return true, fmt.Errorf(sso.Status.Error)
+		}
+
+		if sso.Status.URL != "" {
 			return true, nil
 		}
 
@@ -641,39 +645,6 @@ func (wsm *WorkspaceManagerServer) modifyWorkspace(ctx context.Context, id strin
 	return nil
 }
 
-// func modifyObject[obj client.Object](ctx context.Context, client client.Client, namespace, id string, updateStatus bool, mod func(o obj) error) error {
-// 	var ws obj
-// 	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-// 		err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: id}, ws)
-// 		if err != nil {
-// 			return err
-// 		}
-
-// 		err = mod(ws)
-// 		if err != nil {
-// 			return err
-// 		}
-
-// 		if updateStatus {
-// 			err = client.Status().Update(ctx, ws)
-// 		} else {
-// 			err = client.Update(ctx, ws)
-
-// 		}
-// 		return err
-// 	})
-// 	if errors.IsNotFound(err) {
-// 		return status.Errorf(codes.NotFound, "%s %s not found", reflect.TypeOf(ws).String(), id)
-// 	}
-// 	if c := status.Code(err); c != codes.Unknown && c != codes.OK {
-// 		return err
-// 	}
-// 	if err != nil {
-// 		return status.Errorf(codes.Internal, "cannot modify %s: %v", reflect.TypeOf(ws).String(), err)
-// 	}
-// 	return nil
-// }
-
 // validateStartWorkspaceRequest ensures that acting on this request will not leave the system in an invalid state
 func validateStartWorkspaceRequest(req *wsmanapi.StartWorkspaceRequest) error {
 	err := validation.ValidateStruct(req.Spec,

install/installer/pkg/components/ws-manager-mk2/role.go

@@ -77,18 +77,13 @@ func role(ctx *common.RenderContext) ([]runtime.Object, error) {
 						"delete",
 						"get",
 						"list",
-						"patch",
-						"update",
-						"watch",
 					},
 				},
 				{
 					APIGroups: []string{"workspace.gitpod.io"},
 					Resources: []string{"snapshots/status"},
 					Verbs: []string{
 						"get",
-						"patch",
-						"update",
 					},
 				},
 				// ConfigMap, Leases, and Events access is required for leader-election.