#200: Apply review feedback

- AppDefinition resource version fix after rebase
- Catch all exceptions when adding session, workspace resources and add error state
- Do not handle resources in HANDLED state again.
- Smaller fixes

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/client/ResourceClient.java

@@ -63,7 +63,7 @@ default T edit(String correlationId, String name, Consumer<T> consumer) {
     }
 
     default T editStatus(String correlationId, String name, Consumer<T> consumer) {
-	trace(correlationId, "Edit status of" + name);
+	trace(correlationId, "Edit status of " + name);
 	Resource<T> resource = resource(name);
 	if (resource.get() == null) {
 	    return null;

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/AppDefinition.java

@@ -25,14 +25,14 @@
 import io.fabric8.kubernetes.model.annotation.Singular;
 import io.fabric8.kubernetes.model.annotation.Version;
 
-@Version("v8beta")
+@Version("v7beta")
 @Group("theia.cloud")
 @Singular("appdefinition")
 @Plural("appdefinitions")
 public class AppDefinition extends CustomResource<AppDefinitionSpec, AppDefinitionStatus> implements Namespaced {
 
     private static final long serialVersionUID = 8749670583218521755L;
-    public static final String API = "theia.cloud/v8beta";
+    public static final String API = "theia.cloud/v7beta";
     public static final String KIND = "AppDefinition";
     public static final String CRD_NAME = "appdefinitions.theia.cloud";
 

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/AppDefinitionStatus.java

@@ -19,5 +19,8 @@
 
 @JsonDeserialize
 public class AppDefinitionStatus extends ResourceStatus {
-
+    // This class is empty as only the common properties of the super class are
+    // used. Already define a specific class to allow easier extension, properly
+    // type the resources and resource clients.
+    // It is planned to extend this later with AppDefinition specific status steps.
 }

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/OperatorStatus.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (C) 2022 EclipseSource and others.
+ * Copyright (C) 2023 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -20,11 +20,18 @@
  */
 public interface OperatorStatus {
 
+    /**
+     * The default status describing that the resource is new and was not handled
+     * before.
+     */
     String NEW = "NEW";
 
+    /** The operator tried to handle this resource but an error occurred. */
     String ERROR = "ERROR";
 
+    /** The operator started handling this resource. */
     String HANDLING = "HANDLING";
 
+    /** The operator successfully finished handling this resource. */
     String HANDLED = "HANDLED";
 }

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/SessionStatus.java

@@ -19,5 +19,8 @@
 
 @JsonDeserialize
 public class SessionStatus extends ResourceStatus {
-
+    // This class is empty as only the common properties of the super class are
+    // used. Already define a specific class to allow easier extension, properly
+    // type the resources and resource clients.
+    // It is planned to extend this later with Session specific status steps.
 }

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/StatusStep.java

@@ -27,6 +27,10 @@ public class StatusStep {
     public StatusStep() {
     }
 
+    public StatusStep(String status) {
+	this(status, "");
+    }
+
     public StatusStep(String status, String message) {
 	this.status = status;
 	this.message = message;

java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/LazySessionHandler.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (C) 2022 EclipseSource, Lockular, Ericsson, STMicroelectronics and 
+ * Copyright (C) 2022-2023 EclipseSource, Lockular, Ericsson, STMicroelectronics and 
  * others.
  *
  * This program and the accompanying materials are made available under the
@@ -90,15 +90,36 @@ public class LazySessionHandler implements SessionHandler {
 
     @Override
     public boolean sessionAdded(Session session, String correlationId) {
+	try {
+	    return doSessionAdded(session, correlationId);
+	} catch (Throwable ex) {
+	    LOGGER.error(formatLogMessage(correlationId,
+		    "An unexpected exception occurred while adding Session: " + session), ex);
+	    client.sessions().updateStatus(correlationId, session, status -> {
+		status.setOperatorStatus(OperatorStatus.ERROR);
+		status.setOperatorMessage(
+			"Unexpected error. Please check the logs for correlationId: " + correlationId);
+	    });
+	    return false;
+	}
+    }
 
+    protected boolean doSessionAdded(Session session, String correlationId) {
 	/* session information */
 	String sessionResourceName = session.getMetadata().getName();
 	String sessionResourceUID = session.getMetadata().getUid();
 
-	// Check current session status and ignore if handling failed before
+	// Check current session status and ignore if handling failed or finished before
 	Optional<SessionStatus> status = Optional.ofNullable(session.getStatus());
 	String operatorStatus = status.map(ResourceStatus::getOperatorStatus).orElse(OperatorStatus.NEW);
+	if (OperatorStatus.HANDLED.equals(operatorStatus)) {
+	    LOGGER.trace(formatLogMessage(correlationId,
+		    "Session was successfully handled before and is skipped now. Session: " + session));
+	    return true;
+	}
 	if (OperatorStatus.ERROR.equals(operatorStatus) || OperatorStatus.HANDLING.equals(operatorStatus)) {
+	    // TODO In the HANDLING case we should not return but continue where we left
+	    // off.
 	    LOGGER.warn(formatLogMessage(correlationId,
 		    "Session could not be handled before and is skipped now. Current status: " + operatorStatus
 			    + ". Session: " + session));
@@ -162,6 +183,8 @@ public boolean sessionAdded(Session session, String correlationId) {
 		s.setOperatorStatus(OperatorStatus.HANDLED);
 		s.setOperatorMessage("Service already exists.");
 	    });
+	    // TODO do not return true if the sessions was in handling state at the start of
+	    // this handler
 	    return true;
 	}
 
@@ -187,6 +210,8 @@ public boolean sessionAdded(Session session, String correlationId) {
 		    s.setOperatorStatus(OperatorStatus.HANDLED);
 		    s.setOperatorMessage("Configmaps already exist.");
 		});
+		// TODO do not return true if the sessions was in handling state at the start of
+		// this handler
 		return true;
 	    }
 	    createAndApplyEmailConfigMap(correlationId, sessionResourceName, sessionResourceUID, session);
@@ -352,8 +377,7 @@ protected void createAndApplyEmailConfigMap(String correlationId, String session
 	    return;
 	}
 	K8sUtil.loadAndCreateConfigMapWithOwnerReference(client.kubernetes(), client.namespace(), correlationId,
-		configMapYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0,
-		configmap -> {
+		configMapYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0, configmap -> {
 		    configmap.setData(Collections.singletonMap(AddedHandlerUtil.FILENAME_AUTHENTICATED_EMAILS_LIST,
 			    session.getSpec().getUser()));
 		});
@@ -372,8 +396,7 @@ protected void createAndApplyProxyConfigMap(String correlationId, String session
 	    return;
 	}
 	K8sUtil.loadAndCreateConfigMapWithOwnerReference(client.kubernetes(), client.namespace(), correlationId,
-		configMapYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0,
-		configMap -> {
+		configMapYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0, configMap -> {
 		    String host = arguments.getInstancesHost() + ingressPathProvider.getPath(appDefinition, session);
 		    int port = appDefinition.getSpec().getPort();
 		    AddedHandlerUtil.updateProxyConfigMap(client.kubernetes(), client.namespace(), configMap, host,
@@ -396,8 +419,7 @@ protected void createAndApplyDeployment(String correlationId, String sessionReso
 	    return;
 	}
 	K8sUtil.loadAndCreateDeploymentWithOwnerReference(client.kubernetes(), client.namespace(), correlationId,
-		deploymentYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0,
-		deployment -> {
+		deploymentYaml, Session.API, Session.KIND, sessionResourceName, sessionResourceUID, 0, deployment -> {
 		    pvName.ifPresent(name -> addVolumeClaim(deployment, name, appDefinition.getSpec()));
 		    bandwidthLimiter.limit(deployment, appDefinition.getSpec().getDownlinkLimit(),
 			    appDefinition.getSpec().getUplinkLimit(), correlationId);

java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/LazyStartAppDefinitionHandler.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (C) 2022 EclipseSource, Lockular, Ericsson, STMicroelectronics and 
+ * Copyright (C) 2022-2023 EclipseSource, Lockular, Ericsson, STMicroelectronics and 
  * others.
  *
  * This program and the accompanying materials are made available under the
@@ -41,6 +41,22 @@ public class LazyStartAppDefinitionHandler implements AppDefinitionHandler {
 
     @Override
     public boolean appDefinitionAdded(AppDefinition appDefinition, String correlationId) {
+	try {
+	    return appDefinitionAdded(appDefinition, correlationId);
+	} catch (Throwable ex) {
+	    LOGGER.error(formatLogMessage(correlationId,
+		    "An unexpected exception occurred while adding AppDefinition: " + appDefinition), ex);
+	    // TODO update status
+//	    client.appDefinitions().updateStatus(correlationId, appDefinition, status ->
+//	    {
+//		status.setOperatorStatus(OperatorStatus.ERROR);
+//		status.setOperatorMessage("Unexpected error. Please check the logs for correlationId " + correlationId);
+//	    });
+	    return false;
+	}
+    }
+
+    protected boolean doAppDefinitionAdded(AppDefinition appDefinition, String correlationId) {
 	LOGGER.info(formatLogMessage(correlationId, "Handling " + appDefinition));
 
 	// TODO Check current session status and ignore if handling failed before

java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/LazyWorkspaceHandler.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (C) 2022 EclipseSource and others.
+ * Copyright (C) 2022-2023 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -44,12 +44,34 @@ public class LazyWorkspaceHandler implements WorkspaceHandler {
 
     @Override
     public boolean workspaceAdded(Workspace workspace, String correlationId) {
+	try {
+	    return doWorkspaceAdded(workspace, correlationId);
+	} catch (Throwable ex) {
+	    LOGGER.error(formatLogMessage(correlationId,
+		    "An unexpected exception occurred while adding Workspace: " + workspace), ex);
+	    client.workspaces().updateStatus(correlationId, workspace, status -> {
+		status.setOperatorStatus(OperatorStatus.ERROR);
+		status.setOperatorMessage(
+			"Unexpected error. Please check the logs for correlationId: " + correlationId);
+	    });
+	    return false;
+	}
+    }
+
+    protected boolean doWorkspaceAdded(Workspace workspace, String correlationId) {
 	LOGGER.info(formatLogMessage(correlationId, "Handling " + workspace));
 
 	// Check current session status and ignore if handling failed before
 	Optional<WorkspaceStatus> status = Optional.ofNullable(workspace.getStatus());
 	String operatorStatus = status.map(ResourceStatus::getOperatorStatus).orElse(OperatorStatus.NEW);
+	if (OperatorStatus.HANDLED.equals(operatorStatus)) {
+	    LOGGER.trace(formatLogMessage(correlationId,
+		    "Workspace was successfully handled before and is skipped now. Workspace: " + workspace));
+	    return true;
+	}
 	if (OperatorStatus.ERROR.equals(operatorStatus) || OperatorStatus.HANDLING.equals(operatorStatus)) {
+	    // TODO In the HANDLING case we should not return but continue where we left
+	    // off.
 	    LOGGER.warn(formatLogMessage(correlationId,
 		    "Workspace could not be handled before and is skipped now. Current status: " + operatorStatus
 			    + ". Workspace: " + workspace));
@@ -62,17 +84,16 @@ public boolean workspaceAdded(Workspace workspace, String correlationId) {
 	});
 
 	String storageName = WorkspaceUtil.getStorageName(workspace);
-	client.workspaces().updateStatus(correlationId, workspace,
-		s -> s.setVolumeClaim(new StatusStep("started", null)));
+	client.workspaces().updateStatus(correlationId, workspace, s -> s.setVolumeClaim(new StatusStep("started")));
 
 	if (!client.persistentVolumes().has(storageName)) {
 	    LOGGER.trace(formatLogMessage(correlationId, "Creating new persistent volume named " + storageName));
 	    persistentVolumeHandler.createAndApplyPersistentVolume(correlationId, workspace);
 	}
 
 	client.workspaces().updateStatus(correlationId, workspace, s -> {
-	    s.setVolumeClaim(new StatusStep("finished", null));
-	    s.setVolumeAttach(new StatusStep("started", null));
+	    s.setVolumeClaim(new StatusStep("finished"));
+	    s.setVolumeAttach(new StatusStep("started"));
 	});
 
 	if (!client.persistentVolumeClaims().has(storageName)) {
@@ -81,15 +102,15 @@ public boolean workspaceAdded(Workspace workspace, String correlationId) {
 	}
 
 	client.workspaces().updateStatus(correlationId, workspace, s -> {
-	    s.setVolumeAttach(new StatusStep("claimed", null));
+	    s.setVolumeAttach(new StatusStep("claimed"));
 	});
 
 	LOGGER.trace(formatLogMessage(correlationId, "Set workspace storage " + storageName));
 	client.workspaces().edit(correlationId, workspace.getSpec().getName(),
 		toEdit -> toEdit.getSpec().setStorage(storageName));
 
 	client.workspaces().updateStatus(correlationId, workspace, s -> {
-	    s.setVolumeAttach(new StatusStep("finished", null));
+	    s.setVolumeAttach(new StatusStep("finished"));
 	});
 
 	client.workspaces().updateStatus(correlationId, workspace, s -> {