Clean up error messages and formatting

Signed-off-by: Angel Misevski <[email protected]>

Author: amisevsk

controllers/workspace/devworkspace_controller.go

@@ -185,7 +185,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
 	if err != nil {
 		reqLogger.Info("DevWorkspace start failed")
 		reconcileStatus.Phase = devworkspace.WorkspaceStatusFailed
-		reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile persistent storage: %s", err)
+		reconcileStatus.Conditions[devworkspace.WorkspaceFailedStart] = fmt.Sprintf("Error processing devfile volumes: %s", err)
 		return reconcile.Result{}, nil
 	}
 	componentDescriptions, err := shimlib.GetComponentDescriptionsFromPodAdditions(devfilePodAdditions, workspace.Spec.Template)
@@ -212,7 +212,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct
 	}
 
 	if storagelib.NeedsStorage(workspace.Spec.Template) {
-		pvcStatus := provision.SyncPVC(workspace, componentDescriptions, r.Client, reqLogger)
+		pvcStatus := provision.SyncPVC(workspace, r.Client, reqLogger)
 		if pvcStatus.Err != nil || !pvcStatus.Continue {
 			return reconcile.Result{Requeue: true}, pvcStatus.Err
 		}

controllers/workspace/provision/pvc.go

@@ -14,7 +14,6 @@ package provision
 
 import (
 	devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
-	"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
 	"github.com/devfile/devworkspace-operator/pkg/config"
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -23,7 +22,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func SyncPVC(workspace *devworkspace.DevWorkspace, components []v1alpha1.ComponentDescription, client client.Client, reqLogger logr.Logger) ProvisioningStatus {
+func SyncPVC(workspace *devworkspace.DevWorkspace, client client.Client, reqLogger logr.Logger) ProvisioningStatus {
 	pvc, err := generatePVC(workspace)
 	if err != nil {
 		return ProvisioningStatus{Err: err}

pkg/library/storage/commonStorage.go

@@ -61,24 +61,25 @@ func RewriteContainerVolumeMounts(workspaceId string, podAdditions *v1alpha1.Pod
 	// TODO: Support more than the common PVC strategy here (storage provisioner interface?)
 	// TODO: What should we do when a volume isn't explicitly defined?
 	commonPVCName := config.ControllerCfg.GetWorkspacePVCName()
-	for cIdx, container := range podAdditions.Containers {
-		for vmIdx, vm := range container.VolumeMounts {
-			if _, ok := devfileVolumes[vm.Name]; !ok {
-				return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name)
+	rewriteVolumeMounts := func(containers []corev1.Container) error {
+		for cIdx, container := range containers {
+			for vmIdx, vm := range container.VolumeMounts {
+				if _, ok := devfileVolumes[vm.Name]; !ok {
+					return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name)
+				}
+				containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name)
+				containers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName
 			}
-			podAdditions.Containers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name)
-			podAdditions.Containers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName
 		}
+		return nil
 	}
-	for cIdx, container := range podAdditions.InitContainers {
-		for vmIdx, vm := range container.VolumeMounts {
-			if _, ok := devfileVolumes[vm.Name]; !ok {
-				return fmt.Errorf("container '%s' references undefined volume '%s'", container.Name, vm.Name)
-			}
-			podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].SubPath = fmt.Sprintf("%s/%s", workspaceId, vm.Name)
-			podAdditions.InitContainers[cIdx].VolumeMounts[vmIdx].Name = commonPVCName
-		}
+	if err := rewriteVolumeMounts(podAdditions.Containers); err != nil {
+		return err
 	}
+	if err := rewriteVolumeMounts(podAdditions.InitContainers); err != nil {
+		return err
+	}
+
 	podAdditions.Volumes = append(podAdditions.Volumes, corev1.Volume{
 		Name: commonPVCName,
 		VolumeSource: corev1.VolumeSource{

pkg/library/storage/testdata/error-duplicate-volumes.yaml

@@ -1,4 +1,4 @@
-name: "Does not modify PodAdditions when storage is not required"
+name: "Returns error when workspace defines duplicate volumes"
 
 input:
   workspaceId: "test-workspaceid"

pkg/library/storage/testdata/error-undefined-volume-init-container.yaml

@@ -1,4 +1,4 @@
-name: "Does not modify PodAdditions when storage is not required"
+name: "Returns error on undefined volume in init container"
 
 input:
   workspaceId: "test-workspaceid"

pkg/library/storage/testdata/error-undefined-volume.yaml

@@ -1,4 +1,4 @@
-name: "Does not modify PodAdditions when storage is not required"
+name: "Returns error on undefined volume in container"
 
 input:
   workspaceId: "test-workspaceid"

pkg/library/storage/testdata/rewrites-volumes-for-common-pvc-strategy.yaml

@@ -1,4 +1,4 @@
-name: "Does not modify PodAdditions when storage is not required"
+name: "Rewrites volumeMounts according to common PVC strategy"
 
 input:
   workspaceId: "test-workspaceid"