fixup! Add WorkspaceWithConfig struct, use it in controller

Author: AObuchow

controllers/workspace/devworkspace_controller.go

@@ -63,20 +63,20 @@ var clock kubeclock.Clock = &kubeclock.RealClock{}
 // updateWorkspaceStatus updates the current workspace's status field with conditions and phase from the passed in status.
 // Parameters for result and error are returned unmodified, unless error is nil and another error is encountered while
 // updating the status.
-func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspaceWithConfig *common.DevWorkspaceWithConfig, logger logr.Logger, status *currentStatus, reconcileResult reconcile.Result, reconcileError error) (reconcile.Result, error) {
-	syncConditions(&workspaceWithConfig.Status, status)
-	oldPhase := workspaceWithConfig.Status.Phase
-	workspaceWithConfig.Status.Phase = status.phase
+func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspace *common.DevWorkspaceWithConfig, logger logr.Logger, status *currentStatus, reconcileResult reconcile.Result, reconcileError error) (reconcile.Result, error) {
+	syncConditions(&workspace.Status, status)
+	oldPhase := workspace.Status.Phase
+	workspace.Status.Phase = status.phase
 
-	infoMessage := getInfoMessage(&workspaceWithConfig.DevWorkspace, status)
-	if warn := conditions.GetConditionByType(workspaceWithConfig.Status.Conditions, conditions.DevWorkspaceWarning); warn != nil && warn.Status == corev1.ConditionTrue {
+	infoMessage := getInfoMessage(&workspace.DevWorkspace, status)
+	if warn := conditions.GetConditionByType(workspace.Status.Conditions, conditions.DevWorkspaceWarning); warn != nil && warn.Status == corev1.ConditionTrue {
 		infoMessage = fmt.Sprintf("%s %s", warningPresentInfoMessage, infoMessage)
 	}
-	if workspaceWithConfig.Status.Message != infoMessage {
-		workspaceWithConfig.Status.Message = infoMessage
+	if workspace.Status.Message != infoMessage {
+		workspace.Status.Message = infoMessage
 	}
 
-	err := r.Status().Update(context.TODO(), &workspaceWithConfig.DevWorkspace)
+	err := r.Status().Update(context.TODO(), &workspace.DevWorkspace)
 	if err != nil {
 		if k8sErrors.IsConflict(err) {
 			logger.Info("Failed to update workspace status due to conflict; retrying")
@@ -87,7 +87,7 @@ func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspaceWithConfig *comm
 			}
 		}
 	} else {
-		updateMetricsForPhase(workspaceWithConfig, oldPhase, status.phase, logger)
+		updateMetricsForPhase(workspace, oldPhase, status.phase, logger)
 	}
 
 	return reconcileResult, reconcileError
@@ -224,15 +224,15 @@ func getInfoMessage(workspace *dw.DevWorkspace, status *currentStatus) string {
 // updateMetricsForPhase increments DevWorkspace startup metrics based on phase transitions in a DevWorkspace. It avoids
 // incrementing the underlying metrics where possible (e.g. reconciling an already running workspace) by only incrementing
 // counters when the new phase is different from the current on in the DevWorkspace.
-func updateMetricsForPhase(workspaceWithConfig *common.DevWorkspaceWithConfig, oldPhase, newPhase dw.DevWorkspacePhase, logger logr.Logger) {
+func updateMetricsForPhase(workspace *common.DevWorkspaceWithConfig, oldPhase, newPhase dw.DevWorkspacePhase, logger logr.Logger) {
 	if oldPhase == newPhase {
 		return
 	}
 	switch newPhase {
 	case dw.DevWorkspaceStatusRunning:
-		metrics.WorkspaceRunning(workspaceWithConfig, logger)
+		metrics.WorkspaceRunning(workspace, logger)
 	case dw.DevWorkspaceStatusFailed:
-		metrics.WorkspaceFailed(&workspaceWithConfig.DevWorkspace, logger)
+		metrics.WorkspaceFailed(&workspace.DevWorkspace, logger)
 	}
 }
 
@@ -266,17 +266,17 @@ func checkForStartTimeout(workspace *dw.DevWorkspace, config v1alpha1.OperatorCo
 // configured progress timeout. If the workspace is not in the Failing state or does not have a DevWorkspaceFailed
 // condition set, returns false. Otherwise, returns true if the workspace has timed out. Returns an error if
 // timeout is configured with an unparsable duration.
-func checkForFailingTimeout(workspaceWithConfig *common.DevWorkspaceWithConfig) (isTimedOut bool, err error) {
-	if workspaceWithConfig.Status.Phase != devworkspacePhaseFailing {
+func checkForFailingTimeout(workspace *common.DevWorkspaceWithConfig) (isTimedOut bool, err error) {
+	if workspace.Status.Phase != devworkspacePhaseFailing {
 		return false, nil
 	}
-	timeout, err := time.ParseDuration(workspaceWithConfig.Config.Workspace.ProgressTimeout)
+	timeout, err := time.ParseDuration(workspace.Config.Workspace.ProgressTimeout)
 	if err != nil {
 		return false, fmt.Errorf("invalid duration specified for timeout: %w", err)
 	}
 	currTime := clock.Now()
 	failedTime := time.Time{}
-	for _, condition := range workspaceWithConfig.Status.Conditions {
+	for _, condition := range workspace.Status.Conditions {
 		if condition.Type == dw.DevWorkspaceFailedStart {
 			failedTime = condition.LastTransitionTime.Time
 		}

controllers/workspace/status.go

@@ -22,16 +22,16 @@ import (
 // Overwrites the content of the workspace's Template Spec with the workspace config's Template Spec,
 // with the exception of the workspace's projects.
 // If the workspace's Template Spec defined any projects, they are preserved.
-func ApplyDefaultTemplate(workspaceWithConfig *common.DevWorkspaceWithConfig) {
-	if workspaceWithConfig.Config.Workspace.DefaultTemplate == nil {
+func ApplyDefaultTemplate(workspace *common.DevWorkspaceWithConfig) {
+	if workspace.Config.Workspace.DefaultTemplate == nil {
 		return
 	}
-	defaultCopy := workspaceWithConfig.Config.Workspace.DefaultTemplate.DeepCopy()
-	originalProjects := workspaceWithConfig.Spec.Template.Projects
-	workspaceWithConfig.Spec.Template.DevWorkspaceTemplateSpecContent = *defaultCopy
-	workspaceWithConfig.Spec.Template.Projects = append(workspaceWithConfig.Spec.Template.Projects, originalProjects...)
+	defaultCopy := workspace.Config.Workspace.DefaultTemplate.DeepCopy()
+	originalProjects := workspace.Spec.Template.Projects
+	workspace.Spec.Template.DevWorkspaceTemplateSpecContent = *defaultCopy
+	workspace.Spec.Template.Projects = append(workspace.Spec.Template.Projects, originalProjects...)
 }
 
-func NeedsDefaultTemplate(workspaceWithConfig *common.DevWorkspaceWithConfig) bool {
-	return len(workspaceWithConfig.Spec.Template.Components) == 0 && workspaceWithConfig.Config.Workspace.DefaultTemplate != nil
+func NeedsDefaultTemplate(workspace *common.DevWorkspaceWithConfig) bool {
+	return len(workspace.Spec.Template.Components) == 0 && workspace.Config.Workspace.DefaultTemplate != nil
 }

pkg/library/defaults/helper.go

@@ -32,16 +32,15 @@ const (
 	projectClonerContainerName = "project-clone"
 )
 
-func GetProjectCloneInitContainer(workspaceWithConfig *common.DevWorkspaceWithConfig) (*corev1.Container, error) {
+func GetProjectCloneInitContainer(workspace *common.DevWorkspaceWithConfig) (*corev1.Container, error) {
 
-	workspace := workspaceWithConfig.Spec.Template
-	if len(workspace.Projects) == 0 {
+	if len(workspace.Spec.Template.Projects) == 0 {
 		return nil, nil
 	}
-	if workspace.Attributes.GetString(constants.ProjectCloneAttribute, nil) == constants.ProjectCloneDisable {
+	if workspace.Spec.Template.Attributes.GetString(constants.ProjectCloneAttribute, nil) == constants.ProjectCloneDisable {
 		return nil, nil
 	}
-	if !hasContainerComponents(&workspace) {
+	if !hasContainerComponents(&workspace.Spec.Template) {
 		// Avoid adding project-clone init container when DevWorkspace does not define any containers
 		return nil, nil
 	}
@@ -94,7 +93,7 @@ func GetProjectCloneInitContainer(workspaceWithConfig *common.DevWorkspaceWithCo
 				MountPath: constants.DefaultProjectsSourcesRoot,
 			},
 		},
-		ImagePullPolicy: corev1.PullPolicy(workspaceWithConfig.Config.Workspace.ImagePullPolicy),
+		ImagePullPolicy: corev1.PullPolicy(workspace.Config.Workspace.ImagePullPolicy),
 	}, nil
 }
 

pkg/library/projects/clone.go

@@ -46,39 +46,39 @@ func (*AsyncStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTemplateS
 	return needsStorage(workspace)
 }
 
-func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspaceWithConfig *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {
+func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {
 	if err := checkConfigured(); err != nil {
 		return &ProvisioningError{
 			Message: fmt.Sprintf("%s. Contact an administrator to resolve this issue.", err.Error()),
 		}
 	}
 
-	numWorkspaces, _, err := p.getAsyncWorkspaceCount(workspaceWithConfig.Namespace, clusterAPI)
+	numWorkspaces, _, err := p.getAsyncWorkspaceCount(workspace.Namespace, clusterAPI)
 	if err != nil {
 		return err
 	}
 	// If there is more than one started workspace using async storage, then we fail starting additional ones
 	// Note we need to check phase so as to not accidentally fail an already-running workspace when a second one
 	// is created.
-	if numWorkspaces > 1 && workspaceWithConfig.Status.Phase != dw.DevWorkspaceStatusRunning {
+	if numWorkspaces > 1 && workspace.Status.Phase != dw.DevWorkspaceStatusRunning {
 		return &ProvisioningError{
-			Message: fmt.Sprintf("cannot provision storage for workspace %s", workspaceWithConfig.Name),
+			Message: fmt.Sprintf("cannot provision storage for workspace %s", workspace.Name),
 			Err:     fmt.Errorf("at most one workspace using async storage can be running in a namespace"),
 		}
 	}
 
 	// Add ephemeral volumes
-	if err := addEphemeralVolumesFromWorkspace(&workspaceWithConfig.DevWorkspace, podAdditions); err != nil {
+	if err := addEphemeralVolumesFromWorkspace(&workspace.DevWorkspace, podAdditions); err != nil {
 		return err
 	}
 
 	// If persistent storage is not needed, we're done
-	if !p.NeedsStorage(&workspaceWithConfig.Spec.Template) {
+	if !p.NeedsStorage(&workspace.Spec.Template) {
 		return nil
 	}
 
 	// Sync SSH keypair to cluster
-	secret, configmap, err := asyncstorage.GetOrCreateSSHConfig(&workspaceWithConfig.DevWorkspace, clusterAPI)
+	secret, configmap, err := asyncstorage.GetOrCreateSSHConfig(&workspace.DevWorkspace, clusterAPI)
 	if err != nil {
 		if errors.Is(err, asyncstorage.NotReadyError) {
 			return &NotReadyError{
@@ -89,12 +89,12 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd
 		return err
 	}
 
-	pvcName, err := checkForExistingCommonPVC(workspaceWithConfig.Namespace, clusterAPI)
+	pvcName, err := checkForExistingCommonPVC(workspace.Namespace, clusterAPI)
 	if err != nil {
 		return err
 	}
 
-	pvcTerminating, err := checkPVCTerminating(pvcName, workspaceWithConfig.Namespace, clusterAPI, workspaceWithConfig.Config)
+	pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI, workspace.Config)
 	if err != nil {
 		return err
 	} else if pvcTerminating {
@@ -106,15 +106,15 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd
 
 	if pvcName != "" {
 		// Create common PVC if needed
-		clusterPVC, err := syncCommonPVC(workspaceWithConfig.Namespace, clusterAPI, workspaceWithConfig.Config)
+		clusterPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI, workspace.Config)
 		if err != nil {
 			return err
 		}
 		pvcName = clusterPVC.Name
 	}
 
 	// Create async server deployment
-	deploy, err := asyncstorage.SyncWorkspaceSyncDeploymentToCluster(workspaceWithConfig.Namespace, configmap, pvcName, clusterAPI, workspaceWithConfig.Config)
+	deploy, err := asyncstorage.SyncWorkspaceSyncDeploymentToCluster(workspace.Namespace, configmap, pvcName, clusterAPI, workspace.Config)
 	if err != nil {
 		if errors.Is(err, asyncstorage.NotReadyError) {
 			return &NotReadyError{
@@ -157,40 +157,40 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd
 		return err
 	}
 
-	volumes, err := p.addVolumesForAsyncStorage(podAdditions, &workspaceWithConfig.DevWorkspace)
+	volumes, err := p.addVolumesForAsyncStorage(podAdditions, &workspace.DevWorkspace)
 	if err != nil {
 		return err
 	}
 
 	sshSecretVolume := asyncstorage.GetVolumeFromSecret(secret)
-	asyncSidecar := asyncstorage.GetAsyncSidecar(workspaceWithConfig.Status.DevWorkspaceId, sshSecretVolume.Name, volumes)
+	asyncSidecar := asyncstorage.GetAsyncSidecar(workspace.Status.DevWorkspaceId, sshSecretVolume.Name, volumes)
 	podAdditions.Containers = append(podAdditions.Containers, *asyncSidecar)
 	podAdditions.Volumes = append(podAdditions.Volumes, *sshSecretVolume)
 
 	return nil
 }
 
-func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspaceWithConfig *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {
+func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {
 	// TODO: This approach relies on there being a maximum of one workspace running per namespace.
-	asyncDeploy, err := asyncstorage.GetWorkspaceSyncDeploymentCluster(workspaceWithConfig.Namespace, clusterAPI)
+	asyncDeploy, err := asyncstorage.GetWorkspaceSyncDeploymentCluster(workspace.Namespace, clusterAPI)
 	if err != nil {
 		if k8sErrors.IsNotFound(err) {
-			return runCommonPVCCleanupJob(workspaceWithConfig, clusterAPI)
+			return runCommonPVCCleanupJob(workspace, clusterAPI)
 		} else {
 			return err
 		}
 	}
 
 	// Check if another workspace is currently using the async server
-	numWorkspaces, totalWorkspaces, err := p.getAsyncWorkspaceCount(workspaceWithConfig.Namespace, clusterAPI)
+	numWorkspaces, totalWorkspaces, err := p.getAsyncWorkspaceCount(workspace.Namespace, clusterAPI)
 	if err != nil {
 		return err
 	}
 	switch numWorkspaces {
 	case 0:
 		// no problem
 	case 1:
-		if workspaceWithConfig.Spec.Started {
+		if workspace.Spec.Started {
 			// This is the only workspace using the async server, we can safely stop it
 			break
 		}
@@ -219,12 +219,12 @@ func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspaceWithConfig *c
 	}
 
 	// Clean up PVC using usual job
-	err = runCommonPVCCleanupJob(workspaceWithConfig, clusterAPI)
+	err = runCommonPVCCleanupJob(workspace, clusterAPI)
 	if err != nil {
 		return err
 	}
 
-	retry, err := asyncstorage.RemoveAuthorizedKeyFromConfigMap(&workspaceWithConfig.DevWorkspace, clusterAPI)
+	retry, err := asyncstorage.RemoveAuthorizedKeyFromConfigMap(&workspace.DevWorkspace, clusterAPI)
 	if err != nil {
 		return &ProvisioningError{
 			Message: "Failed to remove authorized key from async storage configmap",

pkg/provision/storage/asyncStorage.go

@@ -113,30 +113,30 @@ func runCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, clusterAPI
 	}
 }
 
-func getSpecCommonPVCCleanupJob(workspaceWithConfig *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*batchv1.Job, error) {
-	workspaceId := workspaceWithConfig.Status.DevWorkspaceId
+func getSpecCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*batchv1.Job, error) {
+	workspaceId := workspace.Status.DevWorkspaceId
 
-	pvcName, err := checkForExistingCommonPVC(workspaceWithConfig.Namespace, clusterAPI)
+	pvcName, err := checkForExistingCommonPVC(workspace.Namespace, clusterAPI)
 	if err != nil {
 		return nil, err
 	}
 	if pvcName == "" {
-		pvcName = workspaceWithConfig.Config.Workspace.PVCName
+		pvcName = workspace.Config.Workspace.PVCName
 	}
 
 	jobLabels := map[string]string{
 		constants.DevWorkspaceIDLabel:      workspaceId,
-		constants.DevWorkspaceNameLabel:    workspaceWithConfig.Name,
-		constants.DevWorkspaceCreatorLabel: workspaceWithConfig.Labels[constants.DevWorkspaceCreatorLabel],
+		constants.DevWorkspaceNameLabel:    workspace.Name,
+		constants.DevWorkspaceCreatorLabel: workspace.Labels[constants.DevWorkspaceCreatorLabel],
 	}
-	if restrictedAccess, needsRestrictedAccess := workspaceWithConfig.Annotations[constants.DevWorkspaceRestrictedAccessAnnotation]; needsRestrictedAccess {
+	if restrictedAccess, needsRestrictedAccess := workspace.Annotations[constants.DevWorkspaceRestrictedAccessAnnotation]; needsRestrictedAccess {
 		jobLabels[constants.DevWorkspaceRestrictedAccessAnnotation] = restrictedAccess
 	}
 
 	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      common.PVCCleanupJobName(workspaceId),
-			Namespace: workspaceWithConfig.Namespace,
+			Namespace: workspace.Namespace,
 			Labels:    jobLabels,
 		},
 		Spec: batchv1.JobSpec{
@@ -148,7 +148,7 @@ func getSpecCommonPVCCleanupJob(workspaceWithConfig *common.DevWorkspaceWithConf
 				},
 				Spec: corev1.PodSpec{
 					RestartPolicy:   "Never",
-					SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(workspaceWithConfig.Config),
+					SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(workspace.Config),
 					Volumes: []corev1.Volume{
 						{
 							Name: pvcName,
@@ -191,7 +191,7 @@ func getSpecCommonPVCCleanupJob(workspaceWithConfig *common.DevWorkspaceWithConf
 		},
 	}
 
-	podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(workspaceWithConfig.Namespace, clusterAPI)
+	podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(workspace.Namespace, clusterAPI)
 	if err != nil {
 		return nil, err
 	}
@@ -202,16 +202,16 @@ func getSpecCommonPVCCleanupJob(workspaceWithConfig *common.DevWorkspaceWithConf
 		job.Spec.Template.Spec.NodeSelector = nodeSelector
 	}
 
-	if err := controllerutil.SetControllerReference(workspaceWithConfig, job, clusterAPI.Scheme); err != nil {
+	if err := controllerutil.SetControllerReference(workspace, job, clusterAPI.Scheme); err != nil {
 		return nil, err
 	}
 	return job, nil
 }
 
-func commonPVCExists(workspaceWithConfig *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (bool, error) {
+func commonPVCExists(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (bool, error) {
 	namespacedName := types.NamespacedName{
-		Name:      workspaceWithConfig.Config.Workspace.PVCName,
-		Namespace: workspaceWithConfig.Namespace,
+		Name:      workspace.Config.Workspace.PVCName,
+		Namespace: workspace.Namespace,
 	}
 	err := clusterAPI.Client.Get(clusterAPI.Ctx, namespacedName, &corev1.PersistentVolumeClaim{})
 	if err != nil {