diff --git a/controllers/controller/devworkspacerouting/solvers/basic_solver.go b/controllers/controller/devworkspacerouting/solvers/basic_solver.go index 23beeccc3..f5e4007f9 100644 --- a/controllers/controller/devworkspacerouting/solvers/basic_solver.go +++ b/controllers/controller/devworkspacerouting/solvers/basic_solver.go @@ -57,7 +57,8 @@ func (s *BasicSolver) Finalize(*controllerv1alpha1.DevWorkspaceRouting) error { func (s *BasicSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) (RoutingObjects, error) { routingObjects := RoutingObjects{} - routingSuffix := config.Routing.ClusterHostSuffix + // TODO: Use workspace-scoped ClusterHostSuffix to allow overriding + routingSuffix := config.GetGlobalConfig().Routing.ClusterHostSuffix if routingSuffix == "" { return routingObjects, &RoutingInvalid{"basic routing requires .config.routing.clusterHostSuffix to be set in operator config"} } diff --git a/controllers/workspace/cleanup.go b/controllers/workspace/cleanup.go index 0e6c53a8e..a89be8274 100644 --- a/controllers/workspace/cleanup.go +++ b/controllers/workspace/cleanup.go @@ -17,8 +17,8 @@ import ( "context" "fmt" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -27,7 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func (r *DevWorkspaceReconciler) deleteWorkspaceOwnedObjects(ctx context.Context, workspace *dw.DevWorkspace) (requeue bool, err error) { +func (r *DevWorkspaceReconciler) deleteWorkspaceOwnedObjects(ctx context.Context, workspace *common.DevWorkspaceWithConfig) (requeue bool, err error) { idLabelSelector, err := labels.Parse(fmt.Sprintf("%s=%s", constants.DevWorkspaceIDLabel, workspace.Status.DevWorkspaceId)) if err != nil { return false, err diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index eab63771d..84a257f82 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -28,6 +28,7 @@ import ( "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/config" + wkspConfig "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/library/annotate" containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" @@ -109,8 +110,8 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } // Fetch the Workspace instance - workspace := &dw.DevWorkspace{} - err = r.Get(ctx, req.NamespacedName, workspace) + rawWorkspace := &dw.DevWorkspace{} + err = r.Get(ctx, req.NamespacedName, rawWorkspace) if err != nil { if k8sErrors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -121,8 +122,19 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Error reading the object - requeue the request. return reconcile.Result{}, err } + + config, err := wkspConfig.ResolveConfigForWorkspace(rawWorkspace, clusterAPI.Client) + if err != nil { + reqLogger.Error(err, "Error applying external DevWorkspace-Operator configuration") + config = wkspConfig.GetGlobalConfig() + } + configString := wkspConfig.GetCurrentConfigString(config) + workspace := &common.DevWorkspaceWithConfig{} + workspace.DevWorkspace = rawWorkspace + workspace.Config = config + reqLogger = reqLogger.WithValues(constants.DevWorkspaceIDLoggerKey, workspace.Status.DevWorkspaceId) - reqLogger.Info("Reconciling Workspace") + reqLogger.Info("Reconciling Workspace", "resolvedConfig", configString) // Check if the DevWorkspaceRouting instance is marked to be deleted, which is // indicated by the deletion timestamp being set. @@ -137,10 +149,10 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request if err != nil { workspace.Status.Phase = dw.DevWorkspaceStatusFailed workspace.Status.Message = fmt.Sprintf("Failed to set DevWorkspace ID: %s", err.Error()) - return reconcile.Result{}, r.Status().Update(ctx, workspace) + return reconcile.Result{}, r.Status().Update(ctx, workspace.DevWorkspace) } workspace.Status.DevWorkspaceId = workspaceId - err = r.Status().Update(ctx, workspace) + err = r.Status().Update(ctx, workspace.DevWorkspace) return reconcile.Result{Requeue: true}, err } @@ -157,7 +169,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } patch := []byte(`{"spec":{"started": false}}`) - err := r.Client.Patch(context.Background(), workspace, client.RawPatch(types.MergePatchType, patch)) + err := r.Client.Patch(context.Background(), workspace.DevWorkspace, client.RawPatch(types.MergePatchType, patch)) if err != nil { return reconcile.Result{}, err } @@ -188,7 +200,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request Message: "DevWorkspace is starting", }, } - err = r.Status().Update(ctx, workspace) + err = r.Status().Update(ctx, workspace.DevWorkspace) if err == nil { metrics.WorkspaceStarted(workspace, reqLogger) } @@ -198,7 +210,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Prepare handling workspace status and condition reconcileStatus := currentStatus{phase: dw.DevWorkspaceStatusStarting} reconcileStatus.setConditionTrue(conditions.Started, "DevWorkspace is starting") - clusterWorkspace := workspace.DeepCopy() + clusterWorkspace := &common.DevWorkspaceWithConfig{} + clusterWorkspace.DevWorkspace = workspace.DevWorkspace.DeepCopy() + clusterWorkspace.Config = workspace.Config timingInfo := map[string]string{} timing.SetTime(timingInfo, timing.DevWorkspaceStarted) @@ -229,7 +243,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request if _, ok := clusterWorkspace.Annotations[constants.DevWorkspaceStopReasonAnnotation]; ok { delete(clusterWorkspace.Annotations, constants.DevWorkspaceStopReasonAnnotation) - err = r.Update(context.TODO(), clusterWorkspace) + err = r.Update(context.TODO(), clusterWorkspace.DevWorkspace) return reconcile.Result{Requeue: true}, err } @@ -275,12 +289,12 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Note: we need to check the flattened workspace to see if a finalizer is needed, as plugins could require storage if storageProvisioner.NeedsStorage(&workspace.Spec.Template) { coputil.AddFinalizer(clusterWorkspace, constants.StorageCleanupFinalizer) - if err := r.Update(ctx, clusterWorkspace); err != nil { + if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil { return reconcile.Result{}, err } } - devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(&workspace.Spec.Template) + devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(&workspace.Spec.Template, workspace.Config.Workspace.ImagePullPolicy) if err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Error processing devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus) } @@ -291,7 +305,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } // Add init container to clone projects - if projectClone, err := projects.GetProjectCloneInitContainer(&workspace.Spec.Template); err != nil { + if projectClone, err := projects.GetProjectCloneInitContainer(&workspace.Spec.Template, workspace.Config.Workspace.ImagePullPolicy); err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Failed to set up project-clone init container: %s", err), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus) } else if projectClone != nil { devfilePodAdditions.InitContainers = append(devfilePodAdditions.InitContainers, *projectClone) @@ -405,7 +419,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } if wsprovision.NeedsServiceAccountFinalizer(&workspace.Spec.Template) { coputil.AddFinalizer(clusterWorkspace, constants.ServiceAccountCleanupFinalizer) - if err := r.Update(ctx, clusterWorkspace); err != nil { + if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil { return reconcile.Result{}, err } } @@ -457,7 +471,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request return reconcile.Result{}, nil } -func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *dw.DevWorkspace, logger logr.Logger) (reconcile.Result, error) { +func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *common.DevWorkspaceWithConfig, logger logr.Logger) (reconcile.Result, error) { status := currentStatus{phase: dw.DevWorkspaceStatusStopping} if workspace.Status.Phase == devworkspacePhaseFailing || workspace.Status.Phase == dw.DevWorkspaceStatusFailed { status.phase = workspace.Status.Phase @@ -488,7 +502,7 @@ func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *d return r.updateWorkspaceStatus(workspace, logger, &status, reconcile.Result{}, nil) } -func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWorkspace, logger logr.Logger) (stopped bool, err error) { +func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *common.DevWorkspaceWithConfig, logger logr.Logger) (stopped bool, err error) { workspaceDeployment := &appsv1.Deployment{} namespaceName := types.NamespacedName{ Name: common.DeploymentName(workspace.Status.DevWorkspaceId), @@ -525,7 +539,7 @@ func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWo } // CleanupOnStop should never be nil as a default is always set - if config.Workspace.CleanupOnStop == nil || !*config.Workspace.CleanupOnStop { + if workspace.Config.Workspace.CleanupOnStop == nil || !*workspace.Config.Workspace.CleanupOnStop { replicas := workspaceDeployment.Spec.Replicas if replicas == nil || *replicas > 0 { logger.Info("Stopping workspace") @@ -547,7 +561,7 @@ func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWo // failWorkspace marks a workspace as failed by setting relevant fields in the status struct. // These changes are not synced to cluster immediately, and are intended to be synced to the cluster via a deferred function // in the main reconcile loop. If needed, changes can be flushed to the cluster immediately via `updateWorkspaceStatus()` -func (r *DevWorkspaceReconciler) failWorkspace(workspace *dw.DevWorkspace, msg string, reason metrics.FailureReason, logger logr.Logger, status *currentStatus) (reconcile.Result, error) { +func (r *DevWorkspaceReconciler) failWorkspace(workspace *common.DevWorkspaceWithConfig, msg string, reason metrics.FailureReason, logger logr.Logger, status *currentStatus) (reconcile.Result, error) { logger.Info("DevWorkspace failed to start: " + msg) status.phase = devworkspacePhaseFailing status.setConditionTrueWithReason(dw.DevWorkspaceFailedStart, msg, string(reason)) @@ -558,7 +572,7 @@ func (r *DevWorkspaceReconciler) failWorkspace(workspace *dw.DevWorkspace, msg s } func (r *DevWorkspaceReconciler) syncTimingToCluster( - ctx context.Context, workspace *dw.DevWorkspace, timingInfo map[string]string, reqLogger logr.Logger) { + ctx context.Context, workspace *common.DevWorkspaceWithConfig, timingInfo map[string]string, reqLogger logr.Logger) { if timing.IsEnabled() { if workspace.Annotations == nil { workspace.Annotations = map[string]string{} @@ -568,7 +582,7 @@ func (r *DevWorkspaceReconciler) syncTimingToCluster( workspace.Annotations[timingEvent] = timestamp } } - if err := r.Update(ctx, workspace); err != nil { + if err := r.Update(ctx, workspace.DevWorkspace); err != nil { if k8sErrors.IsConflict(err) { reqLogger.Info("Got conflict when trying to apply timing annotations to workspace") } else { @@ -579,7 +593,7 @@ func (r *DevWorkspaceReconciler) syncTimingToCluster( } func (r *DevWorkspaceReconciler) syncStartedAtToCluster( - ctx context.Context, workspace *dw.DevWorkspace, reqLogger logr.Logger) { + ctx context.Context, workspace *common.DevWorkspaceWithConfig, reqLogger logr.Logger) { if workspace.Annotations == nil { workspace.Annotations = map[string]string{} @@ -590,7 +604,7 @@ func (r *DevWorkspaceReconciler) syncStartedAtToCluster( } workspace.Annotations[constants.DevWorkspaceStartedAtAnnotation] = timing.CurrentTime() - if err := r.Update(ctx, workspace); err != nil { + if err := r.Update(ctx, workspace.DevWorkspace); err != nil { if k8sErrors.IsConflict(err) { reqLogger.Info("Got conflict when trying to apply started-at annotations to workspace") } else { @@ -600,12 +614,12 @@ func (r *DevWorkspaceReconciler) syncStartedAtToCluster( } func (r *DevWorkspaceReconciler) removeStartedAtFromCluster( - ctx context.Context, workspace *dw.DevWorkspace, reqLogger logr.Logger) { + ctx context.Context, workspace *common.DevWorkspaceWithConfig, reqLogger logr.Logger) { if workspace.Annotations == nil { workspace.Annotations = map[string]string{} } delete(workspace.Annotations, constants.DevWorkspaceStartedAtAnnotation) - if err := r.Update(ctx, workspace); err != nil { + if err := r.Update(ctx, workspace.DevWorkspace); err != nil { if k8sErrors.IsConflict(err) { reqLogger.Info("Got conflict when trying to apply timing annotations to workspace") } else { @@ -614,7 +628,7 @@ func (r *DevWorkspaceReconciler) removeStartedAtFromCluster( } } -func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace *dw.DevWorkspace) (string, error) { +func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace *common.DevWorkspaceWithConfig) (string, error) { if idOverride := workspace.Annotations[constants.WorkspaceIdOverrideAnnotation]; idOverride != "" { if len(idOverride) > 25 { return "", fmt.Errorf("maximum length for DevWorkspace ID override is 25 characters") @@ -676,8 +690,9 @@ func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Req } } + // TODO: Label PVCs used for workspace storage so that they can be cleaned up if non-default name is used. // Otherwise, check if common PVC is deleted to make sure all DevWorkspaces see it happen - if obj.GetName() != config.Workspace.PVCName || obj.GetDeletionTimestamp() == nil { + if obj.GetName() != config.GetGlobalConfig().Workspace.PVCName || obj.GetDeletionTimestamp() == nil { // We're looking for a deleted common PVC return []reconcile.Request{} } diff --git a/controllers/workspace/finalize.go b/controllers/workspace/finalize.go index 201c176f1..2a3e3549a 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/constants" @@ -34,7 +35,7 @@ import ( wsprovision "github.com/devfile/devworkspace-operator/pkg/provision/workspace" ) -func (r *DevWorkspaceReconciler) workspaceNeedsFinalize(workspace *dw.DevWorkspace) bool { +func (r *DevWorkspaceReconciler) workspaceNeedsFinalize(workspace *common.DevWorkspaceWithConfig) bool { for _, finalizer := range workspace.Finalizers { if finalizer == constants.StorageCleanupFinalizer { return true @@ -46,7 +47,7 @@ func (r *DevWorkspaceReconciler) workspaceNeedsFinalize(workspace *dw.DevWorkspa return false } -func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (finalizeResult reconcile.Result, finalizeErr error) { +func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig) (finalizeResult reconcile.Result, finalizeErr error) { // Tracked state for the finalize process; we update the workspace status in a deferred function (and pass the // named return value for finalize()) to update the workspace's status with whatever is in finalizeStatus // when this function returns. @@ -74,7 +75,7 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, return reconcile.Result{}, nil } -func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace, finalizeStatus *currentStatus) (reconcile.Result, error) { +func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) { // Need to make sure Deployment is cleaned up before starting job to avoid mounting issues for RWO PVCs wait, err := wsprovision.DeleteWorkspaceDeployment(ctx, workspace, r.Client) if err != nil { @@ -91,7 +92,7 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L // Namespace is terminating, it's redundant to clean PVC files since it's going to be removed log.Info("Namespace is terminating; clearing storage finalizer") coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer) - return reconcile.Result{}, r.Update(ctx, workspace) + return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) } storageProvisioner, err := storage.GetProvisioner(workspace) @@ -126,10 +127,10 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L } log.Info("PVC clean up successful; clearing finalizer") coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer) - return reconcile.Result{}, r.Update(ctx, workspace) + return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) } -func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace, finalizeStatus *currentStatus) (reconcile.Result, error) { +func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) { retry, err := wsprovision.FinalizeServiceAccount(workspace, ctx, r.NonCachingClient) if err != nil { log.Error(err, "Failed to finalize workspace ServiceAccount") @@ -142,7 +143,7 @@ func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log } log.Info("ServiceAccount clean up successful; clearing finalizer") coputil.RemoveFinalizer(workspace, constants.ServiceAccountCleanupFinalizer) - return reconcile.Result{}, r.Update(ctx, workspace) + return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace) } func (r *DevWorkspaceReconciler) namespaceIsTerminating(ctx context.Context, namespace string) (bool, error) { diff --git a/controllers/workspace/http.go b/controllers/workspace/http.go index 5e032d440..1f073b3ac 100644 --- a/controllers/workspace/http.go +++ b/controllers/workspace/http.go @@ -34,11 +34,13 @@ func setupHttpClients() { InsecureSkipVerify: true, } - if config.Routing != nil && config.Routing.ProxyConfig != nil { + globalConfig := config.GetGlobalConfig() + + if globalConfig.Routing != nil && globalConfig.Routing.ProxyConfig != nil { proxyConf := httpproxy.Config{ - HTTPProxy: config.Routing.ProxyConfig.HttpProxy, - HTTPSProxy: config.Routing.ProxyConfig.HttpsProxy, - NoProxy: config.Routing.ProxyConfig.NoProxy, + HTTPProxy: globalConfig.Routing.ProxyConfig.HttpProxy, + HTTPSProxy: globalConfig.Routing.ProxyConfig.HttpsProxy, + NoProxy: globalConfig.Routing.ProxyConfig.NoProxy, } proxyFunc := func(req *http.Request) (*url.URL, error) { return proxyConf.ProxyFunc()(req.URL) diff --git a/controllers/workspace/metrics/failure_reason.go b/controllers/workspace/metrics/failure_reason.go index ee0f6c8e7..f809018d3 100644 --- a/controllers/workspace/metrics/failure_reason.go +++ b/controllers/workspace/metrics/failure_reason.go @@ -17,6 +17,7 @@ package metrics import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" ) @@ -37,7 +38,7 @@ var devworkspaceFailureReasons = []FailureReason{ } // GetFailureReason returns the FailureReason of the provided DevWorkspace -func GetFailureReason(wksp *dw.DevWorkspace) FailureReason { +func GetFailureReason(wksp *common.DevWorkspaceWithConfig) FailureReason { failedCondition := conditions.GetConditionByType(wksp.Status.Conditions, dw.DevWorkspaceFailedStart) if failedCondition != nil { for _, reason := range devworkspaceFailureReasons { diff --git a/controllers/workspace/metrics/update.go b/controllers/workspace/metrics/update.go index 40c3f6af0..2c6a7f67c 100644 --- a/controllers/workspace/metrics/update.go +++ b/controllers/workspace/metrics/update.go @@ -20,14 +20,14 @@ import ( "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" ) // WorkspaceStarted updates metrics for workspaces entering the 'Starting' phase, given a workspace. If an error is // encountered, the provided logger is used to log the error. -func WorkspaceStarted(wksp *dw.DevWorkspace, log logr.Logger) { +func WorkspaceStarted(wksp *common.DevWorkspaceWithConfig, log logr.Logger) { _, ok := wksp.GetAnnotations()[constants.DevWorkspaceStartedAtAnnotation] if !ok { incrementMetricForWorkspace(workspaceTotal, wksp, log) @@ -37,7 +37,7 @@ func WorkspaceStarted(wksp *dw.DevWorkspace, log logr.Logger) { // WorkspaceRunning updates metrics for workspaces entering the 'Running' phase, given a workspace. If an error is // encountered, the provided logger is used to log the error. This function assumes the provided workspace has // fully-synced conditions (i.e. the WorkspaceReady condition is present). -func WorkspaceRunning(wksp *dw.DevWorkspace, log logr.Logger) { +func WorkspaceRunning(wksp *common.DevWorkspaceWithConfig, log logr.Logger) { _, ok := wksp.GetAnnotations()[constants.DevWorkspaceStartedAtAnnotation] if !ok { incrementMetricForWorkspace(workspaceStarts, wksp, log) @@ -47,18 +47,18 @@ func WorkspaceRunning(wksp *dw.DevWorkspace, log logr.Logger) { // WorkspaceFailed updates metrics for workspace entering the 'Failed' phase. If an error is encountered, the provided // logger is used to log the error. -func WorkspaceFailed(wksp *dw.DevWorkspace, log logr.Logger) { +func WorkspaceFailed(wksp *common.DevWorkspaceWithConfig, log logr.Logger) { incrementMetricForWorkspaceFailure(workspaceFailures, wksp, log) } -func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *dw.DevWorkspace, log logr.Logger) { - sourceLabel := wksp.Labels[workspaceSourceLabel] +func incrementMetricForWorkspace(metric *prometheus.CounterVec, workspace *common.DevWorkspaceWithConfig, log logr.Logger) { + sourceLabel := workspace.Labels[workspaceSourceLabel] if sourceLabel == "" { sourceLabel = "unknown" } - routingClass := wksp.Spec.RoutingClass + routingClass := workspace.Spec.RoutingClass if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + routingClass = workspace.Config.Routing.DefaultRoutingClass } ctr, err := metric.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass}) if err != nil { @@ -67,12 +67,12 @@ func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *dw.DevWork ctr.Inc() } -func incrementMetricForWorkspaceFailure(metric *prometheus.CounterVec, wksp *dw.DevWorkspace, log logr.Logger) { - sourceLabel := wksp.Labels[workspaceSourceLabel] +func incrementMetricForWorkspaceFailure(metric *prometheus.CounterVec, workspace *common.DevWorkspaceWithConfig, log logr.Logger) { + sourceLabel := workspace.Labels[workspaceSourceLabel] if sourceLabel == "" { sourceLabel = "unknown" } - reason := GetFailureReason(wksp) + reason := GetFailureReason(workspace) ctr, err := metric.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsReasonLabel: string(reason)}) if err != nil { log.Error(err, "Failed to increment metric") @@ -80,24 +80,24 @@ func incrementMetricForWorkspaceFailure(metric *prometheus.CounterVec, wksp *dw. ctr.Inc() } -func incrementStartTimeBucketForWorkspace(wksp *dw.DevWorkspace, log logr.Logger) { - sourceLabel := wksp.Labels[workspaceSourceLabel] +func incrementStartTimeBucketForWorkspace(workspace *common.DevWorkspaceWithConfig, log logr.Logger) { + sourceLabel := workspace.Labels[workspaceSourceLabel] if sourceLabel == "" { sourceLabel = "unknown" } - routingClass := wksp.Spec.RoutingClass + routingClass := workspace.Spec.RoutingClass if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + routingClass = workspace.Config.Routing.DefaultRoutingClass } hist, err := workspaceStartupTimesHist.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass}) if err != nil { log.Error(err, "Failed to update metric") } - readyCondition := conditions.GetConditionByType(wksp.Status.Conditions, dw.DevWorkspaceReady) + readyCondition := conditions.GetConditionByType(workspace.Status.Conditions, dw.DevWorkspaceReady) if readyCondition == nil { return } - startedCondition := conditions.GetConditionByType(wksp.Status.Conditions, conditions.Started) + startedCondition := conditions.GetConditionByType(workspace.Status.Conditions, conditions.Started) if startedCondition == nil { return } diff --git a/controllers/workspace/status.go b/controllers/workspace/status.go index 2eb36ec89..91777fd15 100644 --- a/controllers/workspace/status.go +++ b/controllers/workspace/status.go @@ -23,6 +23,7 @@ import ( "time" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/go-logr/logr" @@ -35,7 +36,6 @@ import ( "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/controllers/workspace/metrics" "github.com/devfile/devworkspace-operator/pkg/conditions" - "github.com/devfile/devworkspace-operator/pkg/config" ) const ( @@ -64,7 +64,7 @@ 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(workspace *dw.DevWorkspace, logger logr.Logger, status *currentStatus, reconcileResult reconcile.Result, reconcileError error) (reconcile.Result, error) { +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 @@ -77,7 +77,7 @@ func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspace *dw.DevWorkspac workspace.Status.Message = infoMessage } - err := r.Status().Update(context.TODO(), workspace) + 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") @@ -144,18 +144,18 @@ func syncConditions(workspaceStatus *dw.DevWorkspaceStatus, currentStatus *curre }) } -func syncWorkspaceMainURL(workspace *dw.DevWorkspace, exposedEndpoints map[string]v1alpha1.ExposedEndpointList, clusterAPI sync.ClusterAPI) (ok bool, err error) { +func syncWorkspaceMainURL(workspace *common.DevWorkspaceWithConfig, exposedEndpoints map[string]v1alpha1.ExposedEndpointList, clusterAPI sync.ClusterAPI) (ok bool, err error) { mainUrl := getMainUrl(exposedEndpoints) if workspace.Status.MainUrl == mainUrl { return true, nil } workspace.Status.MainUrl = mainUrl - err = clusterAPI.Client.Status().Update(context.TODO(), workspace) + err = clusterAPI.Client.Status().Update(context.TODO(), workspace.DevWorkspace) return false, err } -func checkServerStatus(workspace *dw.DevWorkspace) (ok bool, err error) { +func checkServerStatus(workspace *common.DevWorkspaceWithConfig) (ok bool, err error) { mainUrl := workspace.Status.MainUrl if mainUrl == "" { // Support DevWorkspaces that do not specify an mainUrl @@ -190,7 +190,7 @@ func getMainUrl(exposedEndpoints map[string]v1alpha1.ExposedEndpointList) string return "" } -func getInfoMessage(workspace *dw.DevWorkspace, status *currentStatus) string { +func getInfoMessage(workspace *common.DevWorkspaceWithConfig, status *currentStatus) string { // Check for errors and failure if cond, ok := status.conditions[dw.DevWorkspaceError]; ok { return cond.Message @@ -225,7 +225,7 @@ 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(workspace *dw.DevWorkspace, oldPhase, newPhase dw.DevWorkspacePhase, logger logr.Logger) { +func updateMetricsForPhase(workspace *common.DevWorkspaceWithConfig, oldPhase, newPhase dw.DevWorkspacePhase, logger logr.Logger) { if oldPhase == newPhase { return } @@ -241,11 +241,11 @@ func updateMetricsForPhase(workspace *dw.DevWorkspace, oldPhase, newPhase dw.Dev // startup timeout. This is determined by checking to see if the last condition transition time is more // than [timeout] duration ago. Workspaces that are not in the "Starting" phase cannot timeout. Returns // an error with message when timeout is reached. -func checkForStartTimeout(workspace *dw.DevWorkspace) error { +func checkForStartTimeout(workspace *common.DevWorkspaceWithConfig) error { if workspace.Status.Phase != dw.DevWorkspaceStatusStarting { return nil } - timeout, err := time.ParseDuration(config.Workspace.ProgressTimeout) + timeout, err := time.ParseDuration(workspace.Config.Workspace.ProgressTimeout) if err != nil { return fmt.Errorf("invalid duration specified for timeout: %w", err) } @@ -258,7 +258,7 @@ func checkForStartTimeout(workspace *dw.DevWorkspace) error { } if !lastUpdateTime.IsZero() && lastUpdateTime.Add(timeout).Before(currTime) { return fmt.Errorf("devworkspace failed to progress past phase '%s' for longer than timeout (%s)", - workspace.Status.Phase, config.Workspace.ProgressTimeout) + workspace.Status.Phase, workspace.Config.Workspace.ProgressTimeout) } return nil } @@ -267,11 +267,11 @@ func checkForStartTimeout(workspace *dw.DevWorkspace) error { // 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(workspace *dw.DevWorkspace) (isTimedOut bool, err error) { +func checkForFailingTimeout(workspace *common.DevWorkspaceWithConfig) (isTimedOut bool, err error) { if workspace.Status.Phase != devworkspacePhaseFailing { return false, nil } - timeout, err := time.ParseDuration(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) } diff --git a/controllers/workspace/validation.go b/controllers/workspace/validation.go index 623e6f2c1..46e5a7cec 100644 --- a/controllers/workspace/validation.go +++ b/controllers/workspace/validation.go @@ -18,8 +18,7 @@ package controllers import ( "fmt" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/webhook" ) @@ -29,7 +28,7 @@ import ( // default. // // If error is not nil, a user-readable message is returned that can be propagated to the user to explain the issue. -func (r *DevWorkspaceReconciler) validateCreatorLabel(workspace *dw.DevWorkspace) (msg string, err error) { +func (r *DevWorkspaceReconciler) validateCreatorLabel(workspace *common.DevWorkspaceWithConfig) (msg string, err error) { if _, present := workspace.Labels[constants.DevWorkspaceCreatorLabel]; !present { return "DevWorkspace was created without creator ID label. It must be recreated to resolve the issue", fmt.Errorf("devworkspace does not have creator label applied") diff --git a/docs/additional-configuration.adoc b/docs/additional-configuration.adoc index baf2ba943..8d0eb32a8 100644 --- a/docs/additional-configuration.adoc +++ b/docs/additional-configuration.adoc @@ -156,6 +156,33 @@ Note: As for automatically mounting secrets, it is necessary to apply the `contr This will mount a file `/tmp/.git-credentials/credentials` in all workspace containers, and construct a git config to use this file as a credentials store. The mount path specified by the `controller.devfile.io/mount-path` annotation can by omitted, in which case `/` is used (mounting a file `/credentials`). +## Setting an alternate configuration for a workspace +It is possible to configure a workspace to use an alternate DevWorkspaceOperatorConfig. +In order to do so, the alternate DevWorkspaceOperatorConfig must exist on the cluster, and the `controller.devfile.io/devworkspace-config` workspace attribute must be set. +The `controller.devfile.io/devworkspace-config` attribute takes two string fields: `name` and `namespace`. + +* `name`: the `metadata.name` of the alternate DevWorkspaceOperatorConfig. +* `namespace`: the `metadata.namespace` of the alternate DevWorkspaceOperatorConfig. + +[source,yaml] +---- +kind: DevWorkspace +apiVersion: workspace.devfile.io/v1alpha2 +metadata: + name: my-workspace +spec: + template: + attributes: + controller.devfile.io/devworkspace-config: + name: + namespace: +---- + +*Note:* the alternate DevWorkspaceOperatorConfig will be +merged with the default DevWorkspaceOperatorConfig, overriding +fields in the default configuration. Fields unset in the overridden +configuration will use the global values. + ## Debugging a failing workspace Normally, when a workspace fails to start, the deployment will be scaled down and the workspace will be stopped in a `Failed` state. This can make it difficult to debug misconfiguration errors, so the annotation `controller.devfile.io/debug-start: "true"` can be applied to DevWorkspaces to leave resources for failed workspaces on the cluster. This allows viewing logs from workspace containers. diff --git a/pkg/common/types.go b/pkg/common/types.go new file mode 100644 index 000000000..70dbda034 --- /dev/null +++ b/pkg/common/types.go @@ -0,0 +1,24 @@ +// Copyright (c) 2019-2022 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package common + +import ( + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" +) + +type DevWorkspaceWithConfig struct { + *dw.DevWorkspace `json:",inline"` + Config *controllerv1alpha1.OperatorConfiguration `json:"resolvedConfig"` +} diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index f05ead396..77e3cf067 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -31,7 +31,11 @@ import ( "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) -const testNamespace = "test-namespace" +const ( + testNamespace = "test-namespace" + externalConfigName = "external-config-name" + externalConfigNamespace = "external-config-namespace" +) var ( scheme = runtime.NewScheme() @@ -68,3 +72,13 @@ func buildConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceO Config: config, } } + +func buildExternalConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceOperatorConfig { + return &v1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: externalConfigName, + Namespace: externalConfigNamespace, + }, + Config: config, + } +} diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 4ee83a752..e682679f7 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -21,6 +21,7 @@ import ( "strings" "sync" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/config/proxy" routeV1 "github.com/openshift/api/route/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -30,6 +31,7 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) @@ -39,20 +41,54 @@ const ( ) var ( - Routing *controller.RoutingConfig - Workspace *controller.WorkspaceConfig internalConfig *controller.OperatorConfiguration configMutex sync.Mutex configNamespace string log = ctrl.Log.WithName("operator-configuration") ) -func SetConfigForTesting(config *controller.OperatorConfiguration) { +func GetGlobalConfig() *controller.OperatorConfiguration { + return internalConfig.DeepCopy() +} + +// ResolveConfigForWorkspace returns the resulting config from merging the global DevWorkspaceOperatorConfig with the +// DevWorkspaceOperatorConfig specified by the optional workspace attribute `controller.devfile.io/devworkspace-config`. +// If the `controller.devfile.io/devworkspace-config` is not set, the global DevWorkspaceOperatorConfig is returned. +// If the `controller.devfile.io/devworkspace-config` attribute is incorrectly set, or the specified DevWorkspaceOperatorConfig +// does not exist on the cluster, an error is returned. +func ResolveConfigForWorkspace(workspace *dw.DevWorkspace, client crclient.Client) (*controller.OperatorConfiguration, error) { + if !workspace.Spec.Template.Attributes.Exists(constants.ExternalDevWorkspaceConfiguration) { + return GetGlobalConfig(), nil + } + + namespacedName := types.NamespacedName{} + err := workspace.Spec.Template.Attributes.GetInto(constants.ExternalDevWorkspaceConfiguration, &namespacedName) + if err != nil { + return nil, fmt.Errorf("failed to read attribute %s in DevWorkspace attributes: %w", constants.ExternalDevWorkspaceConfiguration, err) + } + + if namespacedName.Name == "" { + return nil, fmt.Errorf("'name' must be set for attribute %s in DevWorkspace attributes", constants.ExternalDevWorkspaceConfiguration) + } + + if namespacedName.Namespace == "" { + return nil, fmt.Errorf("'namespace' must be set for attribute %s in DevWorkspace attributes", constants.ExternalDevWorkspaceConfiguration) + } + + externalDWOC := &controller.DevWorkspaceOperatorConfig{} + err = client.Get(context.TODO(), namespacedName, externalDWOC) + if err != nil { + return nil, fmt.Errorf("could not fetch external DWOC with name %s in namespace %s: %w", namespacedName.Name, namespacedName.Namespace, err) + } + return getMergedConfig(externalDWOC.Config, internalConfig), nil +} + +func GetConfigForTesting(customConfig *controller.OperatorConfiguration) *controller.OperatorConfiguration { configMutex.Lock() defer configMutex.Unlock() - internalConfig = defaultConfig.DeepCopy() - mergeConfig(config, internalConfig) - updatePublicConfig() + testConfig := defaultConfig.DeepCopy() + mergeConfig(customConfig, testConfig) + return testConfig } func SetupControllerConfig(client crclient.Client) error { @@ -93,7 +129,7 @@ func SetupControllerConfig(client crclient.Client) error { defaultConfig.Routing.ProxyConfig = clusterProxy internalConfig.Routing.ProxyConfig = proxy.MergeProxyConfigs(clusterProxy, internalConfig.Routing.ProxyConfig) - updatePublicConfig() + logCurrentConfig() return nil } @@ -119,6 +155,13 @@ func getClusterConfig(namespace string, client crclient.Client) (*controller.Dev return clusterConfig, nil } +func getMergedConfig(from, to *controller.OperatorConfiguration) *controller.OperatorConfiguration { + mergedConfig := to.DeepCopy() + fromCopy := from.DeepCopy() + mergeConfig(fromCopy, mergedConfig) + return mergedConfig +} + func syncConfigFrom(newConfig *controller.DevWorkspaceOperatorConfig) { if newConfig == nil || newConfig.Name != OperatorConfigName || newConfig.Namespace != configNamespace { return @@ -127,19 +170,13 @@ func syncConfigFrom(newConfig *controller.DevWorkspaceOperatorConfig) { defer configMutex.Unlock() internalConfig = defaultConfig.DeepCopy() mergeConfig(newConfig.Config, internalConfig) - updatePublicConfig() + logCurrentConfig() } func restoreDefaultConfig() { configMutex.Lock() defer configMutex.Unlock() internalConfig = defaultConfig.DeepCopy() - updatePublicConfig() -} - -func updatePublicConfig() { - Routing = internalConfig.Routing.DeepCopy() - Workspace = internalConfig.Workspace.DeepCopy() logCurrentConfig() } @@ -260,57 +297,68 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { } } -// logCurrentConfig formats the current operator configuration as a plain string -func logCurrentConfig() { - if internalConfig == nil { - return +func GetCurrentConfigString(currConfig *controller.OperatorConfiguration) string { + if currConfig == nil { + return "" } + + routing := currConfig.Routing var config []string - if Routing != nil { - if Routing.ClusterHostSuffix != "" && Routing.ClusterHostSuffix != defaultConfig.Routing.ClusterHostSuffix { - config = append(config, fmt.Sprintf("routing.clusterHostSuffix=%s", Routing.ClusterHostSuffix)) + if routing != nil { + if routing.ClusterHostSuffix != "" && routing.ClusterHostSuffix != defaultConfig.Routing.ClusterHostSuffix { + config = append(config, fmt.Sprintf("routing.clusterHostSuffix=%s", routing.ClusterHostSuffix)) } - if Routing.DefaultRoutingClass != defaultConfig.Routing.DefaultRoutingClass { - config = append(config, fmt.Sprintf("routing.defaultRoutingClass=%s", Routing.DefaultRoutingClass)) + if routing.DefaultRoutingClass != defaultConfig.Routing.DefaultRoutingClass { + config = append(config, fmt.Sprintf("routing.defaultRoutingClass=%s", routing.DefaultRoutingClass)) } } - if Workspace != nil { - if Workspace.ImagePullPolicy != defaultConfig.Workspace.ImagePullPolicy { - config = append(config, fmt.Sprintf("workspace.imagePullPolicy=%s", Workspace.ImagePullPolicy)) + workspace := currConfig.Workspace + if workspace != nil { + if workspace.ImagePullPolicy != defaultConfig.Workspace.ImagePullPolicy { + config = append(config, fmt.Sprintf("workspace.imagePullPolicy=%s", workspace.ImagePullPolicy)) } - if Workspace.PVCName != defaultConfig.Workspace.PVCName { - config = append(config, fmt.Sprintf("workspace.pvcName=%s", Workspace.PVCName)) + if workspace.PVCName != defaultConfig.Workspace.PVCName { + config = append(config, fmt.Sprintf("workspace.pvcName=%s", workspace.PVCName)) } - if Workspace.StorageClassName != nil && Workspace.StorageClassName != defaultConfig.Workspace.StorageClassName { - config = append(config, fmt.Sprintf("workspace.storageClassName=%s", *Workspace.StorageClassName)) + if workspace.StorageClassName != nil && workspace.StorageClassName != defaultConfig.Workspace.StorageClassName { + config = append(config, fmt.Sprintf("workspace.storageClassName=%s", *workspace.StorageClassName)) } - if Workspace.IdleTimeout != defaultConfig.Workspace.IdleTimeout { - config = append(config, fmt.Sprintf("workspace.idleTimeout=%s", Workspace.IdleTimeout)) + if workspace.IdleTimeout != defaultConfig.Workspace.IdleTimeout { + config = append(config, fmt.Sprintf("workspace.idleTimeout=%s", workspace.IdleTimeout)) } - if Workspace.IgnoredUnrecoverableEvents != nil { + if workspace.IgnoredUnrecoverableEvents != nil { config = append(config, fmt.Sprintf("workspace.ignoredUnrecoverableEvents=%s", - strings.Join(Workspace.IgnoredUnrecoverableEvents, ";"))) + strings.Join(workspace.IgnoredUnrecoverableEvents, ";"))) } - if Workspace.DefaultStorageSize != nil { - if Workspace.DefaultStorageSize.Common != nil { - config = append(config, fmt.Sprintf("workspace.defaultStorageSize.common=%s", Workspace.DefaultStorageSize.Common.String())) + if workspace.DefaultStorageSize != nil { + if workspace.DefaultStorageSize.Common != nil && workspace.DefaultStorageSize.Common.String() != defaultConfig.Workspace.DefaultStorageSize.Common.String() { + config = append(config, fmt.Sprintf("workspace.defaultStorageSize.common=%s", workspace.DefaultStorageSize.Common.String())) } - if Workspace.DefaultStorageSize.PerWorkspace != nil { - config = append(config, fmt.Sprintf("workspace.defaultStorageSize.perWorkspace=%s", Workspace.DefaultStorageSize.PerWorkspace.String())) + if workspace.DefaultStorageSize.PerWorkspace != nil && workspace.DefaultStorageSize.PerWorkspace.String() != defaultConfig.Workspace.DefaultStorageSize.PerWorkspace.String() { + config = append(config, fmt.Sprintf("workspace.defaultStorageSize.perWorkspace=%s", workspace.DefaultStorageSize.PerWorkspace.String())) } } - if Workspace.DefaultTemplate != nil { - config = append(config, fmt.Sprintf("workspace.defaultTemplate is set")) + if workspace.DefaultTemplate != nil { + config = append(config, "workspace.defaultTemplate is set") } } - if internalConfig.EnableExperimentalFeatures != nil && *internalConfig.EnableExperimentalFeatures { + if currConfig.EnableExperimentalFeatures != nil && *currConfig.EnableExperimentalFeatures { config = append(config, "enableExperimentalFeatures=true") } - if len(config) == 0 { + return "" + } else { + return strings.Join(config, ",") + } +} + +// logCurrentConfig formats the current operator configuration as a plain string +func logCurrentConfig() { + currConfig := GetCurrentConfigString(internalConfig) + if len(currConfig) == 0 { log.Info("Updated config to [(default config)]") } else { - log.Info(fmt.Sprintf("Updated config to [%s]", strings.Join(config, ","))) + log.Info(fmt.Sprintf("Updated config to [%s]", currConfig)) } if internalConfig.Routing.ProxyConfig != nil { diff --git a/pkg/config/sync_test.go b/pkg/config/sync_test.go index 12af19df3..c9848381d 100644 --- a/pkg/config/sync_test.go +++ b/pkg/config/sync_test.go @@ -16,18 +16,24 @@ package config import ( + "context" "fmt" "testing" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + attributes "github.com/devfile/api/v2/pkg/attributes" "github.com/google/go-cmp/cmp" fuzz "github.com/google/gofuzz" routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) @@ -41,17 +47,6 @@ func TestSetupControllerConfigUsesDefault(t *testing.T) { assert.Equal(t, defaultConfig, internalConfig, "Config used should be the default") } -func TestSetupControllerDefaultsAreExported(t *testing.T) { - setupForTest(t) - client := fake.NewClientBuilder().WithScheme(scheme).Build() - err := SetupControllerConfig(client) - if !assert.NoError(t, err, "Should not return error") { - return - } - assert.Equal(t, defaultConfig.Routing, Routing, "Configuration should be exported") - assert.Equal(t, defaultConfig.Workspace, Workspace, "Configuration should be exported") -} - func TestSetupControllerConfigFailsWhenAlreadySetup(t *testing.T) { setupForTest(t) client := fake.NewClientBuilder().WithScheme(scheme).Build() @@ -92,8 +87,96 @@ func TestSetupControllerMergesClusterConfig(t *testing.T) { return } assert.Equal(t, expectedConfig, internalConfig, fmt.Sprintf("Processed config should merge settings from cluster: %s", cmp.Diff(internalConfig, expectedConfig))) - assert.Equal(t, internalConfig.Routing, Routing, fmt.Sprintf("Changes to config should be propagated to exported fields")) - assert.Equal(t, internalConfig.Workspace, Workspace, fmt.Sprintf("Changes to config should be propagated to exported fields")) +} + +func TestCatchesNonExistentExternalDWOC(t *testing.T) { + setupForTest(t) + + workspace := &dw.DevWorkspace{} + attributes := attributes.Attributes{} + namespacedName := types.NamespacedName{ + Name: "external-config-name", + Namespace: "external-config-namespace", + } + attributes.Put(constants.ExternalDevWorkspaceConfiguration, namespacedName, nil) + workspace.Spec.Template.DevWorkspaceTemplateSpecContent = dw.DevWorkspaceTemplateSpecContent{ + Attributes: attributes, + } + client := fake.NewClientBuilder().WithScheme(scheme).Build() + + resolvedConfig, err := ResolveConfigForWorkspace(workspace, client) + if !assert.Error(t, err, "Error should be given if external DWOC specified in workspace spec does not exist") { + return + } + assert.Equal(t, resolvedConfig, internalConfig, "Internal/Global DWOC should be used as fallback if external DWOC could not be resolved") +} + +func TestMergeExternalConfig(t *testing.T) { + setupForTest(t) + + workspace := &dw.DevWorkspace{} + attributes := attributes.Attributes{} + namespacedName := types.NamespacedName{ + Name: externalConfigName, + Namespace: externalConfigNamespace, + } + attributes.Put(constants.ExternalDevWorkspaceConfiguration, namespacedName, nil) + workspace.Spec.Template.DevWorkspaceTemplateSpecContent = dw.DevWorkspaceTemplateSpecContent{ + Attributes: attributes, + } + + // External config is based off of the default/internal config, with just a few changes made + // So when the internal config is merged with the external one, they will become identical + externalConfig := buildExternalConfig(defaultConfig.DeepCopy()) + externalConfig.Config.Workspace.ImagePullPolicy = "Always" + externalConfig.Config.Workspace.PVCName = "test-PVC-name" + storageSize := resource.MustParse("15Gi") + externalConfig.Config.Workspace.DefaultStorageSize = &v1alpha1.StorageSizes{ + Common: &storageSize, + PerWorkspace: &storageSize, + } + + clusterConfig := buildConfig(defaultConfig.DeepCopy()) + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(clusterConfig, externalConfig).Build() + err := SetupControllerConfig(client) + if !assert.NoError(t, err, "Should not return error") { + return + } + + // Sanity check + if !cmp.Equal(clusterConfig.Config, internalConfig) { + t.Error("Internal config should be same as cluster config before starting:", cmp.Diff(clusterConfig.Config, internalConfig)) + } + + resolvedConfig, err := ResolveConfigForWorkspace(workspace, client) + if !assert.NoError(t, err, "Should not return error") { + return + } + + // Compare the resolved config and external config + if !cmp.Equal(resolvedConfig, externalConfig.Config) { + t.Error("Resolved config and external config should match after merge:", cmp.Diff(resolvedConfig, externalConfig.Config)) + } + + // Ensure the internal config was not affected by merge + if !cmp.Equal(clusterConfig.Config, internalConfig) { + t.Error("Internal config should remain the same after merge:", cmp.Diff(clusterConfig.Config, internalConfig)) + } + + // Get the global config off cluster and ensure it hasn't changed + retrievedClusterConfig := &v1alpha1.DevWorkspaceOperatorConfig{} + namespacedName = types.NamespacedName{ + Name: OperatorConfigName, + Namespace: testNamespace, + } + err = client.Get(context.TODO(), namespacedName, retrievedClusterConfig) + if !assert.NoError(t, err, "Should not return error when fetching config from cluster") { + return + } + + if !cmp.Equal(retrievedClusterConfig.Config, clusterConfig.Config) { + t.Error("Config on cluster and global config should match after merge; global config should not have been modified from merge:", cmp.Diff(retrievedClusterConfig, clusterConfig.Config)) + } } func TestSetupControllerAlwaysSetsDefaultClusterRoutingSuffix(t *testing.T) { @@ -119,7 +202,7 @@ func TestSetupControllerAlwaysSetsDefaultClusterRoutingSuffix(t *testing.T) { return } assert.Equal(t, "test-host", defaultConfig.Routing.ClusterHostSuffix, "Should set default clusterRoutingSuffix even if config overrides it initially") - assert.Equal(t, "192.168.0.1.nip.io", Routing.ClusterHostSuffix, "Should use value from config for clusterRoutingSuffix") + assert.Equal(t, "192.168.0.1.nip.io", internalConfig.Routing.ClusterHostSuffix, "Should use value from config for clusterRoutingSuffix") } func TestDetectsOpenShiftRouteSuffix(t *testing.T) { @@ -196,10 +279,10 @@ func TestSyncConfigRestoresClusterRoutingSuffix(t *testing.T) { }, }) syncConfigFrom(config) - assert.Equal(t, "192.168.0.1.nip.io", Routing.ClusterHostSuffix, "Should update clusterRoutingSuffix from config") + assert.Equal(t, "192.168.0.1.nip.io", internalConfig.Routing.ClusterHostSuffix, "Should update clusterRoutingSuffix from config") config.Config.Routing.ClusterHostSuffix = "" syncConfigFrom(config) - assert.Equal(t, "default.routing.suffix", Routing.ClusterHostSuffix, "Should restore default clusterRoutingSuffix if it is available") + assert.Equal(t, "default.routing.suffix", internalConfig.Routing.ClusterHostSuffix, "Should restore default clusterRoutingSuffix if it is available") } func TestSyncConfigDoesNotEraseClusterRoutingSuffix(t *testing.T) { @@ -219,7 +302,7 @@ func TestSyncConfigDoesNotEraseClusterRoutingSuffix(t *testing.T) { if !assert.NoError(t, err, "Should not return error") { return } - assert.Equal(t, "test-host", Routing.ClusterHostSuffix, "Should get clusterHostSuffix from route on OpenShift") + assert.Equal(t, "test-host", internalConfig.Routing.ClusterHostSuffix, "Should get clusterHostSuffix from route on OpenShift") syncConfigFrom(buildConfig(&v1alpha1.OperatorConfiguration{ Routing: &v1alpha1.RoutingConfig{ DefaultRoutingClass: "test-routingClass", @@ -231,7 +314,7 @@ func TestSyncConfigDoesNotEraseClusterRoutingSuffix(t *testing.T) { if !assert.NoError(t, err, "Should not return error") { return } - assert.Equal(t, "test-host", Routing.ClusterHostSuffix, "clusterHostSuffix should persist after an update") + assert.Equal(t, "test-host", internalConfig.Routing.ClusterHostSuffix, "clusterHostSuffix should persist after an update") } func TestMergeConfigLooksAtAllFields(t *testing.T) { diff --git a/pkg/constants/attributes.go b/pkg/constants/attributes.go index 71994662f..59f161d15 100644 --- a/pkg/constants/attributes.go +++ b/pkg/constants/attributes.go @@ -28,6 +28,21 @@ const ( // stopped. DevWorkspaceStorageTypeAttribute = "controller.devfile.io/storage-type" + // ExternalDevWorkspaceConfiguration is an attribute that allows for specifying an (optional) external DevWorkspaceOperatorConfig + // which will merged with the internal/global DevWorkspaceOperatorConfig. The DevWorkspaceOperatorConfig resulting from the merge will be used for the workspace. + // The fields which are set in the external DevWorkspaceOperatorConfig will overwrite those existing in the + // internal/global DevWorkspaceOperatorConfig during the merge. + // The structure of the attribute value should contain two strings: name and namespace. + // 'name' specifies the metadata.name of the external operator configuration. + // 'namespace' specifies the metadata.namespace of the external operator configuration . + // For example: + // + // attributes: + // controller.devfile.io/devworkspace-config: + // name: external-dwoc-name + // namespace: some-namespace + ExternalDevWorkspaceConfiguration = "controller.devfile.io/devworkspace-config" + // RuntimeClassNameAttribute is an attribute added to a DevWorkspace to specify a runtimeClassName for container // components in the DevWorkspace (pod.spec.runtimeClassName). If empty, no runtimeClassName is added. RuntimeClassNameAttribute = "controller.devfile.io/runtime-class" diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index c55eb8aaa..34440783e 100644 --- a/pkg/library/container/container.go +++ b/pkg/library/container/container.go @@ -43,7 +43,7 @@ import ( // rewritten as Volumes are added to PodAdditions, in order to support e.g. using one PVC to hold all volumes // // Note: Requires DevWorkspace to be flattened (i.e. the DevWorkspace contains no Parent or Components of type Plugin) -func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec) (*v1alpha1.PodAdditions, error) { +func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec, pullPolicy string) (*v1alpha1.PodAdditions, error) { if !flatten.DevWorkspaceIsFlattened(workspace) { return nil, fmt.Errorf("devfile is not flattened") } @@ -58,7 +58,7 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec) (*v1al if component.Container == nil { continue } - k8sContainer, err := convertContainerToK8s(component) + k8sContainer, err := convertContainerToK8s(component, pullPolicy) if err != nil { return nil, err } @@ -71,7 +71,7 @@ func GetKubeContainersFromDevfile(workspace *dw.DevWorkspaceTemplateSpec) (*v1al } for _, container := range initContainers { - k8sContainer, err := convertContainerToK8s(container) + k8sContainer, err := convertContainerToK8s(container, pullPolicy) if err != nil { return nil, err } diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go index a30e96fda..3c3a6114d 100644 --- a/pkg/library/container/container_test.go +++ b/pkg/library/container/container_test.go @@ -16,7 +16,6 @@ package container import ( - "fmt" "os" "path/filepath" "testing" @@ -27,7 +26,6 @@ import ( "sigs.k8s.io/yaml" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/config" ) type testCase struct { @@ -41,15 +39,7 @@ type testOutput struct { ErrRegexp *string `json:"errRegexp,omitempty"` } -var testControllerCfg = &v1alpha1.OperatorConfiguration{ - Workspace: &v1alpha1.WorkspaceConfig{ - ImagePullPolicy: "Always", - }, -} - -func setupControllerCfg() { - config.SetConfigForTesting(testControllerCfg) -} +const testImagePullPolicy = "Always" func loadAllTestCasesOrPanic(t *testing.T, fromDir string) []testCase { files, err := os.ReadDir(fromDir) @@ -76,19 +66,18 @@ func loadTestCaseOrPanic(t *testing.T, testPath string) testCase { if err := yaml.Unmarshal(bytes, &test); err != nil { t.Fatal(err) } - t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) + t.Logf("Read file:\n%+v\n\n", test) return test } func TestGetKubeContainersFromDevfile(t *testing.T) { tests := loadAllTestCasesOrPanic(t, "./testdata") - setupControllerCfg() for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.True(t, len(tt.Input.Components) > 0, "Input defines no components") - gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input) + gotPodAdditions, err := GetKubeContainersFromDevfile(tt.Input, testImagePullPolicy) if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { diff --git a/pkg/library/container/conversion.go b/pkg/library/container/conversion.go index d549ac423..de1094366 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -21,14 +21,13 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" ) -func convertContainerToK8s(devfileComponent dw.Component) (*v1.Container, error) { +func convertContainerToK8s(devfileComponent dw.Component, pullPolicy string) (*v1.Container, error) { if devfileComponent.Container == nil { return nil, fmt.Errorf("cannot get k8s container from non-container component") } @@ -48,7 +47,7 @@ func convertContainerToK8s(devfileComponent dw.Component) (*v1.Container, error) Ports: devfileEndpointsToContainerPorts(devfileContainer.Endpoints), Env: devfileEnvToContainerEnv(devfileComponent.Name, devfileContainer.Env), VolumeMounts: devfileVolumeMountsToContainerVolumeMounts(devfileContainer.VolumeMounts), - ImagePullPolicy: v1.PullPolicy(config.Workspace.ImagePullPolicy), + ImagePullPolicy: v1.PullPolicy(pullPolicy), } return container, nil diff --git a/pkg/library/defaults/helper.go b/pkg/library/defaults/helper.go index 4c5fdfbb9..52bf9e51a 100644 --- a/pkg/library/defaults/helper.go +++ b/pkg/library/defaults/helper.go @@ -16,23 +16,22 @@ package defaults import ( - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/common" ) // 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(workspace *dw.DevWorkspace) { - if config.Workspace.DefaultTemplate == nil { +func ApplyDefaultTemplate(workspace *common.DevWorkspaceWithConfig) { + if workspace.Config.Workspace.DefaultTemplate == nil { return } - defaultCopy := config.Workspace.DefaultTemplate.DeepCopy() + 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(workspace *dw.DevWorkspace) bool { - return len(workspace.Spec.Template.Components) == 0 && config.Workspace.DefaultTemplate != nil +func NeedsDefaultTemplate(workspace *common.DevWorkspaceWithConfig) bool { + return len(workspace.Spec.Template.Components) == 0 && workspace.Config.Workspace.DefaultTemplate != nil } diff --git a/pkg/library/env/workspaceenv.go b/pkg/library/env/workspaceenv.go index 37db72bef..7d14682f0 100644 --- a/pkg/library/env/workspaceenv.go +++ b/pkg/library/env/workspaceenv.go @@ -24,14 +24,14 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" - "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" ) // AddCommonEnvironmentVariables adds environment variables to each container in podAdditions. Environment variables added include common // info environment variables and environment variables defined by a workspaceEnv attribute in the devfile itself -func AddCommonEnvironmentVariables(podAdditions *v1alpha1.PodAdditions, clusterDW *dw.DevWorkspace, flattenedDW *dw.DevWorkspaceTemplateSpec) error { - commonEnv := commonEnvironmentVariables(clusterDW.Name, clusterDW.Status.DevWorkspaceId, clusterDW.Namespace, clusterDW.Labels[constants.DevWorkspaceCreatorLabel]) +func AddCommonEnvironmentVariables(podAdditions *v1alpha1.PodAdditions, clusterDW *common.DevWorkspaceWithConfig, flattenedDW *dw.DevWorkspaceTemplateSpec) error { + commonEnv := commonEnvironmentVariables(clusterDW) workspaceEnv, err := collectWorkspaceEnv(flattenedDW) if err != nil { return err @@ -47,60 +47,60 @@ func AddCommonEnvironmentVariables(podAdditions *v1alpha1.PodAdditions, clusterD return nil } -func commonEnvironmentVariables(workspaceName, workspaceId, namespace, creator string) []corev1.EnvVar { +func commonEnvironmentVariables(workspaceWithConfig *common.DevWorkspaceWithConfig) []corev1.EnvVar { envvars := []corev1.EnvVar{ { Name: constants.DevWorkspaceNamespace, - Value: namespace, + Value: workspaceWithConfig.Namespace, }, { Name: constants.DevWorkspaceName, - Value: workspaceName, + Value: workspaceWithConfig.Name, }, { Name: constants.DevWorkspaceId, - Value: workspaceId, + Value: workspaceWithConfig.Status.DevWorkspaceId, }, { Name: constants.DevWorkspaceCreator, - Value: creator, + Value: workspaceWithConfig.Labels[constants.DevWorkspaceCreatorLabel], }, { Name: constants.DevWorkspaceIdleTimeout, - Value: config.Workspace.IdleTimeout, + Value: workspaceWithConfig.Config.Workspace.IdleTimeout, }, } - envvars = append(envvars, getProxyEnvVars()...) + envvars = append(envvars, getProxyEnvVars(workspaceWithConfig.Config.Routing.ProxyConfig)...) return envvars } -func getProxyEnvVars() []corev1.EnvVar { - if config.Routing.ProxyConfig == nil { +func getProxyEnvVars(proxyConfig *v1alpha1.Proxy) []corev1.EnvVar { + if proxyConfig == nil { return nil } - if config.Routing.ProxyConfig.HttpProxy == "" && config.Routing.ProxyConfig.HttpsProxy == "" { + if proxyConfig.HttpProxy == "" && proxyConfig.HttpsProxy == "" { return nil } // Proxy env vars are defined by consensus rather than standard; most tools use the lower-snake-case version // but some may only look at the upper-snake-case version, so we add both. var env []v1.EnvVar - if config.Routing.ProxyConfig.HttpProxy != "" { - env = append(env, v1.EnvVar{Name: "http_proxy", Value: config.Routing.ProxyConfig.HttpProxy}) - env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: config.Routing.ProxyConfig.HttpProxy}) + if proxyConfig.HttpProxy != "" { + env = append(env, v1.EnvVar{Name: "http_proxy", Value: proxyConfig.HttpProxy}) + env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: proxyConfig.HttpProxy}) } - if config.Routing.ProxyConfig.HttpsProxy != "" { - env = append(env, v1.EnvVar{Name: "https_proxy", Value: config.Routing.ProxyConfig.HttpsProxy}) - env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: config.Routing.ProxyConfig.HttpsProxy}) + if proxyConfig.HttpsProxy != "" { + env = append(env, v1.EnvVar{Name: "https_proxy", Value: proxyConfig.HttpsProxy}) + env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: proxyConfig.HttpsProxy}) } - if config.Routing.ProxyConfig.NoProxy != "" { + if proxyConfig.NoProxy != "" { // Adding 'KUBERNETES_SERVICE_HOST' env var to the 'no_proxy / NO_PROXY' list. Hot Fix for https://issues.redhat.com/browse/CRW-2820 kubernetesServiceHost := os.Getenv("KUBERNETES_SERVICE_HOST") - env = append(env, v1.EnvVar{Name: "no_proxy", Value: config.Routing.ProxyConfig.NoProxy + "," + kubernetesServiceHost}) - env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: config.Routing.ProxyConfig.NoProxy + "," + kubernetesServiceHost}) + env = append(env, v1.EnvVar{Name: "no_proxy", Value: proxyConfig.NoProxy + "," + kubernetesServiceHost}) + env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: proxyConfig.NoProxy + "," + kubernetesServiceHost}) } return env diff --git a/pkg/library/projects/clone.go b/pkg/library/projects/clone.go index ec7c806ee..e8d2c43ff 100644 --- a/pkg/library/projects/clone.go +++ b/pkg/library/projects/clone.go @@ -19,7 +19,6 @@ import ( "fmt" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/config" devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -32,7 +31,7 @@ const ( projectClonerContainerName = "project-clone" ) -func GetProjectCloneInitContainer(workspace *dw.DevWorkspaceTemplateSpec) (*corev1.Container, error) { +func GetProjectCloneInitContainer(workspace *dw.DevWorkspaceTemplateSpec, pullPolicy string) (*corev1.Container, error) { if len(workspace.Projects) == 0 { return nil, nil } @@ -92,7 +91,7 @@ func GetProjectCloneInitContainer(workspace *dw.DevWorkspaceTemplateSpec) (*core MountPath: constants.DefaultProjectsSourcesRoot, }, }, - ImagePullPolicy: corev1.PullPolicy(config.Workspace.ImagePullPolicy), + ImagePullPolicy: corev1.PullPolicy(pullPolicy), }, nil } diff --git a/pkg/library/status/check.go b/pkg/library/status/check.go index d36dc9d1f..2dad05fd1 100644 --- a/pkg/library/status/check.go +++ b/pkg/library/status/check.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/infrastructure" "github.com/devfile/devworkspace-operator/pkg/provision/sync" appsv1 "k8s.io/api/apps/v1" @@ -74,7 +73,7 @@ func CheckDeploymentConditions(deployment *appsv1.Deployment) (healthy bool, err // matching unrecoverablePodEventReasons) has the pod as the involved object. // Returns optional message with detected unrecoverable state details // error if any happens during check -func CheckPodsState(workspaceID string, namespace string, labelSelector k8sclient.MatchingLabels, +func CheckPodsState(workspaceID string, namespace string, labelSelector k8sclient.MatchingLabels, ignoredEvents []string, clusterAPI sync.ClusterAPI) (stateMsg string, checkFailure error) { podList := &corev1.PodList{} if err := clusterAPI.Client.List(context.TODO(), podList, k8sclient.InNamespace(namespace), labelSelector); err != nil { @@ -83,25 +82,25 @@ func CheckPodsState(workspaceID string, namespace string, labelSelector k8sclien for _, pod := range podList.Items { for _, containerStatus := range pod.Status.ContainerStatuses { - ok, reason := CheckContainerStatusForFailure(&containerStatus) + ok, reason := CheckContainerStatusForFailure(&containerStatus, ignoredEvents) if !ok { return fmt.Sprintf("Container %s has state %s", containerStatus.Name, reason), nil } } for _, initContainerStatus := range pod.Status.InitContainerStatuses { - ok, reason := CheckContainerStatusForFailure(&initContainerStatus) + ok, reason := CheckContainerStatusForFailure(&initContainerStatus, ignoredEvents) if !ok { return fmt.Sprintf("Init Container %s has state %s", initContainerStatus.Name, reason), nil } } - if msg, err := CheckPodEvents(&pod, workspaceID, clusterAPI); err != nil || msg != "" { + if msg, err := CheckPodEvents(&pod, workspaceID, ignoredEvents, clusterAPI); err != nil || msg != "" { return msg, err } } return "", nil } -func CheckPodEvents(pod *corev1.Pod, workspaceID string, clusterAPI sync.ClusterAPI) (msg string, err error) { +func CheckPodEvents(pod *corev1.Pod, workspaceID string, ignoredEvents []string, clusterAPI sync.ClusterAPI) (msg string, err error) { evs := &corev1.EventList{} selector, err := fields.ParseSelector(fmt.Sprintf("involvedObject.name=%s", pod.Name)) if err != nil { @@ -124,7 +123,7 @@ func CheckPodEvents(pod *corev1.Pod, workspaceID string, clusterAPI sync.Cluster } if maxCount, isUnrecoverableEvent := unrecoverablePodEventReasons[ev.Reason]; isUnrecoverableEvent { - if !checkIfUnrecoverableEventIgnored(ev.Reason) && ev.Count >= maxCount { + if !checkIfUnrecoverableEventIgnored(ev.Reason, ignoredEvents) && ev.Count >= maxCount { var msg string if ev.Count > 1 { msg = fmt.Sprintf("Detected unrecoverable event %s %d times: %s.", ev.Reason, ev.Count, ev.Message) @@ -138,11 +137,11 @@ func CheckPodEvents(pod *corev1.Pod, workspaceID string, clusterAPI sync.Cluster return "", nil } -func CheckContainerStatusForFailure(containerStatus *corev1.ContainerStatus) (ok bool, reason string) { +func CheckContainerStatusForFailure(containerStatus *corev1.ContainerStatus, ignoredEvents []string) (ok bool, reason string) { if containerStatus.State.Waiting != nil { for _, failureReason := range containerFailureStateReasons { if containerStatus.State.Waiting.Reason == failureReason { - return checkIfUnrecoverableEventIgnored(containerStatus.State.Waiting.Reason), containerStatus.State.Waiting.Reason + return checkIfUnrecoverableEventIgnored(containerStatus.State.Waiting.Reason, ignoredEvents), containerStatus.State.Waiting.Reason } } } @@ -150,15 +149,15 @@ func CheckContainerStatusForFailure(containerStatus *corev1.ContainerStatus) (ok if containerStatus.State.Terminated != nil { for _, failureReason := range containerFailureStateReasons { if containerStatus.State.Terminated.Reason == failureReason { - return checkIfUnrecoverableEventIgnored(containerStatus.State.Terminated.Reason), containerStatus.State.Terminated.Reason + return checkIfUnrecoverableEventIgnored(containerStatus.State.Terminated.Reason, ignoredEvents), containerStatus.State.Terminated.Reason } } } return true, "" } -func checkIfUnrecoverableEventIgnored(reason string) (ignored bool) { - for _, ignoredReason := range config.Workspace.IgnoredUnrecoverableEvents { +func checkIfUnrecoverableEventIgnored(reason string, ignoredEvents []string) (ignored bool) { + for _, ignoredReason := range ignoredEvents { if ignoredReason == reason { return true } diff --git a/pkg/provision/metadata/metadata.go b/pkg/provision/metadata/metadata.go index 55f1f993f..ab430f241 100644 --- a/pkg/provision/metadata/metadata.go +++ b/pkg/provision/metadata/metadata.go @@ -18,7 +18,6 @@ package metadata import ( "fmt" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,12 +44,12 @@ const ( // ProvisionWorkspaceMetadata creates a configmap on the cluster that stores metadata about the workspace and configures all // workspace containers to mount that configmap at /devworkspace-metadata. Each container has the environment // variable DEVWORKSPACE_METADATA set to the mount path for the configmap -func ProvisionWorkspaceMetadata(podAdditions *v1alpha1.PodAdditions, original, flattened *dw.DevWorkspace, api sync.ClusterAPI) error { +func ProvisionWorkspaceMetadata(podAdditions *v1alpha1.PodAdditions, original, flattened *common.DevWorkspaceWithConfig, api sync.ClusterAPI) error { cm, err := getSpecMetadataConfigMap(original, flattened) if err != nil { return err } - err = controllerutil.SetControllerReference(original, cm, api.Scheme) + err = controllerutil.SetControllerReference(original.DevWorkspace, cm, api.Scheme) if err != nil { return err } @@ -83,7 +82,7 @@ func ProvisionWorkspaceMetadata(podAdditions *v1alpha1.PodAdditions, original, f return nil } -func getSpecMetadataConfigMap(original, flattened *dw.DevWorkspace) (*corev1.ConfigMap, error) { +func getSpecMetadataConfigMap(original, flattened *common.DevWorkspaceWithConfig) (*corev1.ConfigMap, error) { originalYaml, err := yaml.Marshal(original.Spec.Template) if err != nil { return nil, fmt.Errorf("failed to marshal original DevWorkspace yaml: %w", err) diff --git a/pkg/provision/storage/asyncStorage.go b/pkg/provision/storage/asyncStorage.go index 2c457ac8a..b558d3ca3 100644 --- a/pkg/provision/storage/asyncStorage.go +++ b/pkg/provision/storage/asyncStorage.go @@ -21,6 +21,7 @@ import ( "time" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -45,7 +46,7 @@ func (*AsyncStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTemplateS return needsStorage(workspace) } -func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, 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()), @@ -88,11 +89,14 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd return err } - pvcName, err := checkForExistingCommonPVC(workspace.Namespace, clusterAPI) + usingAlternatePVC, pvcName, err := checkForAlternatePVC(workspace.Namespace, clusterAPI) if err != nil { return err } + if pvcName == "" { + pvcName = workspace.Config.Workspace.PVCName + } pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI) if err != nil { return err @@ -103,9 +107,9 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd } } - if pvcName != "" { + if !usingAlternatePVC { // Create common PVC if needed - clusterPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI) + clusterPVC, err := syncCommonPVC(workspace.Namespace, workspace.Config, clusterAPI) if err != nil { return err } @@ -113,7 +117,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd } // Create async server deployment - deploy, err := asyncstorage.SyncWorkspaceSyncDeploymentToCluster(workspace.Namespace, configmap, pvcName, clusterAPI) + deploy, err := asyncstorage.SyncWorkspaceSyncDeploymentToCluster(workspace, configmap, pvcName, clusterAPI) if err != nil { if errors.Is(err, asyncstorage.NotReadyError) { return &NotReadyError{ @@ -169,7 +173,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd return nil } -func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWorkspace, 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(workspace.Namespace, clusterAPI) if err != nil { @@ -244,7 +248,7 @@ func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWorks return nil } -func (*AsyncStorageProvisioner) addVolumesForAsyncStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace) (volumes []corev1.Volume, err error) { +func (*AsyncStorageProvisioner) addVolumesForAsyncStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig) (volumes []corev1.Volume, err error) { persistentVolumes, _, _ := getWorkspaceVolumes(workspace) addedVolumes, err := addEphemeralVolumesToPodAdditions(podAdditions, persistentVolumes) diff --git a/pkg/provision/storage/asyncstorage/cleanup.go b/pkg/provision/storage/asyncstorage/cleanup.go index 6cc38a325..4cd409117 100644 --- a/pkg/provision/storage/asyncstorage/cleanup.go +++ b/pkg/provision/storage/asyncstorage/cleanup.go @@ -14,7 +14,7 @@ package asyncstorage import ( - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" coputil "github.com/redhat-cop/operator-utils/pkg/util" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -22,7 +22,7 @@ import ( // RemoveAuthorizedKeyFromConfigMap removes the ssh key used by a given workspace from the common async storage // authorized keys configmap. -func RemoveAuthorizedKeyFromConfigMap(workspace *dw.DevWorkspace, api sync.ClusterAPI) (retry bool, err error) { +func RemoveAuthorizedKeyFromConfigMap(workspace *common.DevWorkspaceWithConfig, api sync.ClusterAPI) (retry bool, err error) { sshSecret, err := getSSHSidecarSecretCluster(workspace, api) if err != nil { if k8sErrors.IsNotFound(err) { diff --git a/pkg/provision/storage/asyncstorage/configuration.go b/pkg/provision/storage/asyncstorage/configuration.go index b6472c8d5..9731c1add 100644 --- a/pkg/provision/storage/asyncstorage/configuration.go +++ b/pkg/provision/storage/asyncstorage/configuration.go @@ -16,7 +16,7 @@ package asyncstorage import ( - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -36,7 +36,7 @@ import ( // In both cases, if the ConfigMap does not exist, it is created. // // Returns NotReadyError if changes were made to the cluster. -func GetOrCreateSSHConfig(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) (*corev1.Secret, *corev1.ConfigMap, error) { +func GetOrCreateSSHConfig(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*corev1.Secret, *corev1.ConfigMap, error) { var pubKey []byte clusterSecret, err := getSSHSidecarSecretCluster(workspace, clusterAPI) if err != nil { @@ -50,7 +50,7 @@ func GetOrCreateSSHConfig(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI return nil, nil, err } specSecret := getSSHSidecarSecretSpec(workspace, privateKey) - err := controllerutil.SetControllerReference(workspace, specSecret, clusterAPI.Scheme) + err := controllerutil.SetControllerReference(workspace.DevWorkspace, specSecret, clusterAPI.Scheme) if err != nil { return nil, nil, err } diff --git a/pkg/provision/storage/asyncstorage/deployment.go b/pkg/provision/storage/asyncstorage/deployment.go index cafdb0195..a6ada04e1 100644 --- a/pkg/provision/storage/asyncstorage/deployment.go +++ b/pkg/provision/storage/asyncstorage/deployment.go @@ -17,9 +17,10 @@ package asyncstorage import ( "github.com/devfile/devworkspace-operator/internal/images" + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" "github.com/devfile/devworkspace-operator/pkg/provision/sync" - wsprovision "github.com/devfile/devworkspace-operator/pkg/provision/workspace" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -27,13 +28,13 @@ import ( "k8s.io/apimachinery/pkg/types" ) -func SyncWorkspaceSyncDeploymentToCluster(namespace string, sshConfigMap *corev1.ConfigMap, pvcName string, clusterAPI sync.ClusterAPI) (*appsv1.Deployment, error) { - podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(namespace, clusterAPI) +func SyncWorkspaceSyncDeploymentToCluster(workspace *common.DevWorkspaceWithConfig, sshConfigMap *corev1.ConfigMap, pvcName string, clusterAPI sync.ClusterAPI) (*appsv1.Deployment, error) { + podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(workspace.Namespace, clusterAPI) if err != nil { return nil, err } - specDeployment := getWorkspaceSyncDeploymentSpec(namespace, sshConfigMap, pvcName, podTolerations, nodeSelector) + specDeployment := getWorkspaceSyncDeploymentSpec(workspace, sshConfigMap, pvcName, podTolerations, nodeSelector) clusterObj, err := sync.SyncObjectWithCluster(specDeployment, clusterAPI) switch err.(type) { case nil: @@ -54,7 +55,7 @@ func SyncWorkspaceSyncDeploymentToCluster(namespace string, sshConfigMap *corev1 } func getWorkspaceSyncDeploymentSpec( - namespace string, + workspace *common.DevWorkspaceWithConfig, sshConfigMap *corev1.ConfigMap, pvcName string, tolerations []corev1.Toleration, @@ -64,10 +65,17 @@ func getWorkspaceSyncDeploymentSpec( terminationGracePeriod := int64(1) modeReadOnly := int32(0640) + var securityContext *corev1.PodSecurityContext + if infrastructure.IsOpenShift() { + securityContext = &corev1.PodSecurityContext{} + } else { + securityContext = workspace.Config.Workspace.PodSecurityContext + } + deployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: asyncServerDeploymentName, - Namespace: namespace, + Namespace: workspace.Namespace, Labels: asyncServerLabels, }, Spec: appsv1.DeploymentSpec{ @@ -81,7 +89,7 @@ func getWorkspaceSyncDeploymentSpec( Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Name: "async-storage-server", - Namespace: namespace, + Namespace: workspace.Namespace, Labels: asyncServerLabels, }, Spec: corev1.PodSpec{ @@ -147,7 +155,7 @@ func getWorkspaceSyncDeploymentSpec( }, }, TerminationGracePeriodSeconds: &terminationGracePeriod, - SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(), + SecurityContext: securityContext, AutomountServiceAccountToken: nil, }, }, diff --git a/pkg/provision/storage/asyncstorage/secret.go b/pkg/provision/storage/asyncstorage/secret.go index 2d7cfefac..a6e3a3f5d 100644 --- a/pkg/provision/storage/asyncstorage/secret.go +++ b/pkg/provision/storage/asyncstorage/secret.go @@ -18,7 +18,7 @@ package asyncstorage import ( "fmt" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,7 +35,7 @@ func GetSSHSidecarSecretName(workspaceId string) string { return fmt.Sprintf("%s-asyncsshkey", workspaceId) } -func getSSHSidecarSecretSpec(workspace *dw.DevWorkspace, privateKey []byte) *corev1.Secret { +func getSSHSidecarSecretSpec(workspace *common.DevWorkspaceWithConfig, privateKey []byte) *corev1.Secret { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: GetSSHSidecarSecretName(workspace.Status.DevWorkspaceId), @@ -57,7 +57,7 @@ func getSSHSidecarSecretSpec(workspace *dw.DevWorkspace, privateKey []byte) *cor return secret } -func getSSHSidecarSecretCluster(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) (*corev1.Secret, error) { +func getSSHSidecarSecretCluster(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*corev1.Secret, error) { secret := &corev1.Secret{} namespacedName := types.NamespacedName{ Name: GetSSHSidecarSecretName(workspace.Status.DevWorkspaceId), diff --git a/pkg/provision/storage/cleanup.go b/pkg/provision/storage/cleanup.go index ac81e22be..d7da2ad3f 100644 --- a/pkg/provision/storage/cleanup.go +++ b/pkg/provision/storage/cleanup.go @@ -20,7 +20,7 @@ import ( "path" "time" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" "github.com/devfile/devworkspace-operator/pkg/library/status" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" "github.com/devfile/devworkspace-operator/pkg/provision/sync" @@ -35,9 +35,7 @@ import ( "github.com/devfile/devworkspace-operator/internal/images" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" - wsprovision "github.com/devfile/devworkspace-operator/pkg/provision/workspace" ) const ( @@ -54,7 +52,7 @@ var ( pvcCleanupPodCPURequest = resource.MustParse(constants.PVCCleanupPodCPURequest) ) -func runCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error { +func runCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error { PVCexists, err := commonPVCExists(workspace, clusterAPI) if err != nil { return err @@ -94,7 +92,8 @@ func runCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.ClusterA } } - msg, err := status.CheckPodsState(workspace.Status.DevWorkspaceId, clusterJob.Namespace, k8sclient.MatchingLabels{"job-name": common.PVCCleanupJobName(workspace.Status.DevWorkspaceId)}, clusterAPI) + jobLabels := k8sclient.MatchingLabels{"job-name": common.PVCCleanupJobName(workspace.Status.DevWorkspaceId)} + msg, err := status.CheckPodsState(workspace.Status.DevWorkspaceId, clusterJob.Namespace, jobLabels, workspace.Config.Workspace.IgnoredUnrecoverableEvents, clusterAPI) if err != nil { return &ProvisioningError{ Err: err, @@ -115,15 +114,15 @@ func runCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.ClusterA } } -func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) (*batchv1.Job, error) { +func getSpecCommonPVCCleanupJob(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*batchv1.Job, error) { workspaceId := workspace.Status.DevWorkspaceId - pvcName, err := checkForExistingCommonPVC(workspace.Namespace, clusterAPI) + _, pvcName, err := checkForAlternatePVC(workspace.Namespace, clusterAPI) if err != nil { return nil, err } if pvcName == "" { - pvcName = config.Workspace.PVCName + pvcName = workspace.Config.Workspace.PVCName } jobLabels := map[string]string{ @@ -135,6 +134,13 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus jobLabels[constants.DevWorkspaceRestrictedAccessAnnotation] = restrictedAccess } + var securityContext *corev1.PodSecurityContext + if infrastructure.IsOpenShift() { + securityContext = &corev1.PodSecurityContext{} + } else { + securityContext = workspace.Config.Workspace.PodSecurityContext + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: common.PVCCleanupJobName(workspaceId), @@ -150,7 +156,7 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus }, Spec: corev1.PodSpec{ RestartPolicy: "Never", - SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(), + SecurityContext: securityContext, Volumes: []corev1.Volume{ { Name: pvcName, @@ -204,15 +210,15 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus job.Spec.Template.Spec.NodeSelector = nodeSelector } - if err := controllerutil.SetControllerReference(workspace, job, clusterAPI.Scheme); err != nil { + if err := controllerutil.SetControllerReference(workspace.DevWorkspace, job, clusterAPI.Scheme); err != nil { return nil, err } return job, nil } -func commonPVCExists(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) (bool, error) { +func commonPVCExists(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (bool, error) { namespacedName := types.NamespacedName{ - Name: config.Workspace.PVCName, + Name: workspace.Config.Workspace.PVCName, Namespace: workspace.Namespace, } err := clusterAPI.Client.Get(clusterAPI.Ctx, namespacedName, &corev1.PersistentVolumeClaim{}) diff --git a/pkg/provision/storage/commonStorage.go b/pkg/provision/storage/commonStorage.go index a1de73494..19559264e 100644 --- a/pkg/provision/storage/commonStorage.go +++ b/pkg/provision/storage/commonStorage.go @@ -20,7 +20,7 @@ import ( "time" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" @@ -42,7 +42,7 @@ func (*CommonStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTemplate return needsStorage(workspace) } -func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error { +func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error { // Add ephemeral volumes if err := addEphemeralVolumesFromWorkspace(workspace, podAdditions); err != nil { return err @@ -53,11 +53,14 @@ func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAd return nil } - pvcName, err := checkForExistingCommonPVC(workspace.Namespace, clusterAPI) + usingAlternatePVC, pvcName, err := checkForAlternatePVC(workspace.Namespace, clusterAPI) if err != nil { return err } + if pvcName == "" { + pvcName = workspace.Config.Workspace.PVCName + } pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI) if err != nil { return err @@ -68,8 +71,8 @@ func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAd } } - if pvcName == "" { - commonPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI) + if !usingAlternatePVC { + commonPVC, err := syncCommonPVC(workspace.Namespace, workspace.Config, clusterAPI) if err != nil { return err } @@ -86,7 +89,7 @@ func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAd return nil } -func (p *CommonStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error { +func (p *CommonStorageProvisioner) CleanupWorkspaceStorage(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error { totalWorkspaces, err := getSharedPVCWorkspaceCount(workspace.Namespace, clusterAPI) if err != nil { return err @@ -98,7 +101,7 @@ func (p *CommonStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWork return runCommonPVCCleanupJob(workspace, clusterAPI) } else { sharedPVC := &corev1.PersistentVolumeClaim{} - namespacedName := types.NamespacedName{Name: config.Workspace.PVCName, Namespace: workspace.Namespace} + namespacedName := types.NamespacedName{Name: workspace.Config.Workspace.PVCName, Namespace: workspace.Namespace} err := clusterAPI.Client.Get(clusterAPI.Ctx, namespacedName, sharedPVC) if err != nil { diff --git a/pkg/provision/storage/commonStorage_test.go b/pkg/provision/storage/commonStorage_test.go index cfad122e2..ddcb4d364 100644 --- a/pkg/provision/storage/commonStorage_test.go +++ b/pkg/provision/storage/commonStorage_test.go @@ -23,6 +23,7 @@ import ( "testing" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,7 +48,6 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(v1alpha1.AddToScheme(scheme)) utilruntime.Must(dw.AddToScheme(scheme)) - config.SetConfigForTesting(nil) } type testCase struct { @@ -67,14 +67,17 @@ type testOutput struct { ErrRegexp *string `json:"errRegexp,omitempty"` } -var testControllerCfg = &v1alpha1.OperatorConfiguration{ +var testControllerCfg = config.GetConfigForTesting(&v1alpha1.OperatorConfiguration{ Workspace: &v1alpha1.WorkspaceConfig{ ImagePullPolicy: "Always", }, -} +}) -func setupControllerCfg() { - config.SetConfigForTesting(testControllerCfg) +func getDevWorkspaceWithConfig(workspace *dw.DevWorkspace) *common.DevWorkspaceWithConfig { + workspaceWithConfig := &common.DevWorkspaceWithConfig{} + workspaceWithConfig.DevWorkspace = workspace + workspaceWithConfig.Config = testControllerCfg + return workspaceWithConfig } func loadTestCaseOrPanic(t *testing.T, testFilepath string) testCase { @@ -112,7 +115,7 @@ func TestUseCommonStorageProvisionerForPerUserStorageClass(t *testing.T) { assert.NotNil(t, test.Input.Workspace, "Input does not define workspace") workspace := &dw.DevWorkspace{} workspace.Spec.Template = *test.Input.Workspace - storageProvisioner, err := GetProvisioner(workspace) + storageProvisioner, err := GetProvisioner(getDevWorkspaceWithConfig(workspace)) if !assert.NoError(t, err, "Should not return error") { return @@ -123,9 +126,8 @@ func TestUseCommonStorageProvisionerForPerUserStorageClass(t *testing.T) { func TestProvisionStorageForCommonStorageClass(t *testing.T) { tests := loadAllTestCasesOrPanic(t, "testdata/common-storage") - setupControllerCfg() commonStorage := CommonStorageProvisioner{} - commonPVC, err := getPVCSpec("claim-devworkspace", "test-namespace", resource.MustParse("10Gi")) + commonPVC, err := getPVCSpec("claim-devworkspace", "test-namespace", nil, resource.MustParse("10Gi")) if err != nil { t.Fatalf("Failure during setup: %s", err) } @@ -143,7 +145,7 @@ func TestProvisionStorageForCommonStorageClass(t *testing.T) { workspace.Spec.Template = *tt.Input.Workspace workspace.Status.DevWorkspaceId = tt.Input.DevWorkspaceID workspace.Namespace = "test-namespace" - err := commonStorage.ProvisionStorage(&tt.Input.PodAdditions, workspace, clusterAPI) + err := commonStorage.ProvisionStorage(&tt.Input.PodAdditions, getDevWorkspaceWithConfig(workspace), clusterAPI) if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { @@ -160,9 +162,8 @@ func TestProvisionStorageForCommonStorageClass(t *testing.T) { } func TestTerminatingPVC(t *testing.T) { - setupControllerCfg() commonStorage := CommonStorageProvisioner{} - commonPVC, err := getPVCSpec("claim-devworkspace", "test-namespace", resource.MustParse("10Gi")) + commonPVC, err := getPVCSpec("claim-devworkspace", "test-namespace", nil, resource.MustParse("10Gi")) if err != nil { t.Fatalf("Failure during setup: %s", err) } @@ -179,7 +180,7 @@ func TestTerminatingPVC(t *testing.T) { workspace.Spec.Template = *testCase.Input.Workspace workspace.Status.DevWorkspaceId = testCase.Input.DevWorkspaceID workspace.Namespace = "test-namespace" - err = commonStorage.ProvisionStorage(&testCase.Input.PodAdditions, workspace, clusterAPI) + err = commonStorage.ProvisionStorage(&testCase.Input.PodAdditions, getDevWorkspaceWithConfig(workspace), clusterAPI) if assert.Error(t, err, "Should return error when PVC is terminating") { _, ok := err.(*NotReadyError) assert.True(t, ok, "Expect NotReadyError when PVC is terminating") diff --git a/pkg/provision/storage/ephemeralStorage.go b/pkg/provision/storage/ephemeralStorage.go index e9e936007..648111936 100644 --- a/pkg/provision/storage/ephemeralStorage.go +++ b/pkg/provision/storage/ephemeralStorage.go @@ -17,6 +17,7 @@ package storage import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" @@ -35,7 +36,7 @@ func (e EphemeralStorageProvisioner) NeedsStorage(_ *dw.DevWorkspaceTemplateSpec return false } -func (e EphemeralStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, _ sync.ClusterAPI) error { +func (e EphemeralStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, _ sync.ClusterAPI) error { persistent, ephemeral, projects := getWorkspaceVolumes(workspace) if _, err := addEphemeralVolumesToPodAdditions(podAdditions, persistent); err != nil { return err @@ -59,6 +60,6 @@ func (e EphemeralStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.Pod return nil } -func (e EphemeralStorageProvisioner) CleanupWorkspaceStorage(_ *dw.DevWorkspace, _ sync.ClusterAPI) error { +func (e EphemeralStorageProvisioner) CleanupWorkspaceStorage(_ *common.DevWorkspaceWithConfig, _ sync.ClusterAPI) error { return nil } diff --git a/pkg/provision/storage/ephemeralStorage_test.go b/pkg/provision/storage/ephemeralStorage_test.go index 6c080bc06..b710c4835 100644 --- a/pkg/provision/storage/ephemeralStorage_test.go +++ b/pkg/provision/storage/ephemeralStorage_test.go @@ -25,7 +25,6 @@ import ( func TestRewriteContainerVolumeMountsForEphemeralStorageClass(t *testing.T) { tests := loadAllTestCasesOrPanic(t, "testdata/ephemeral-storage") - setupControllerCfg() commonStorage := EphemeralStorageProvisioner{} for _, tt := range tests { @@ -36,7 +35,7 @@ func TestRewriteContainerVolumeMountsForEphemeralStorageClass(t *testing.T) { workspace.Spec.Template = *tt.Input.Workspace workspace.Status.DevWorkspaceId = tt.Input.DevWorkspaceID workspace.Namespace = "test-namespace" - err := commonStorage.ProvisionStorage(&tt.Input.PodAdditions, workspace, sync.ClusterAPI{}) + err := commonStorage.ProvisionStorage(&tt.Input.PodAdditions, getDevWorkspaceWithConfig(workspace), sync.ClusterAPI{}) if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") } else { diff --git a/pkg/provision/storage/perWorkspaceStorage.go b/pkg/provision/storage/perWorkspaceStorage.go index db6af9a01..d7078ec90 100644 --- a/pkg/provision/storage/perWorkspaceStorage.go +++ b/pkg/provision/storage/perWorkspaceStorage.go @@ -22,7 +22,6 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" @@ -45,7 +44,7 @@ func (*PerWorkspaceStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTe return false } -func (p *PerWorkspaceStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error { +func (p *PerWorkspaceStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error { // Add ephemeral volumes if err := addEphemeralVolumesFromWorkspace(workspace, podAdditions); err != nil { return err @@ -82,7 +81,7 @@ func (p *PerWorkspaceStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1 } // We rely on Kubernetes to use the owner reference to automatically delete the PVC once the workspace is set for deletion. -func (*PerWorkspaceStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error { +func (*PerWorkspaceStorageProvisioner) CleanupWorkspaceStorage(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error { return nil } @@ -156,14 +155,14 @@ func (p *PerWorkspaceStorageProvisioner) rewriteContainerVolumeMounts(workspaceI return nil } -func syncPerWorkspacePVC(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) (*corev1.PersistentVolumeClaim, error) { +func syncPerWorkspacePVC(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) (*corev1.PersistentVolumeClaim, error) { namespacedConfig, err := nsconfig.ReadNamespacedConfig(workspace.Namespace, clusterAPI) if err != nil { return nil, fmt.Errorf("failed to read namespace-specific configuration: %w", err) } // TODO: Determine the storage size that is needed by iterating through workspace volumes, // adding the sizes specified and figuring out overrides/defaults - pvcSize := *config.Workspace.DefaultStorageSize.PerWorkspace + pvcSize := *workspace.Config.Workspace.DefaultStorageSize.PerWorkspace if namespacedConfig != nil && namespacedConfig.PerWorkspacePVCSize != "" { pvcSize, err = resource.ParseQuantity(namespacedConfig.PerWorkspacePVCSize) if err != nil { @@ -171,12 +170,13 @@ func syncPerWorkspacePVC(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) } } - pvc, err := getPVCSpec(common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId), workspace.Namespace, pvcSize) + storageClass := workspace.Config.Workspace.StorageClassName + pvc, err := getPVCSpec(common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId), workspace.Namespace, storageClass, pvcSize) if err != nil { return nil, err } - if err := controllerutil.SetControllerReference(workspace, pvc, clusterAPI.Scheme); err != nil { + if err := controllerutil.SetControllerReference(workspace.DevWorkspace, pvc, clusterAPI.Scheme); err != nil { return nil, err } diff --git a/pkg/provision/storage/perWorkspaceStorage_test.go b/pkg/provision/storage/perWorkspaceStorage_test.go index bd3699285..a12bd89b5 100644 --- a/pkg/provision/storage/perWorkspaceStorage_test.go +++ b/pkg/provision/storage/perWorkspaceStorage_test.go @@ -33,19 +33,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/config" ) func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(v1alpha1.AddToScheme(scheme)) utilruntime.Must(dw.AddToScheme(scheme)) - config.SetConfigForTesting(nil) } func TestRewriteContainerVolumeMountsForPerWorkspaceStorageClass(t *testing.T) { tests := loadAllTestCasesOrPanic(t, "testdata/perWorkspace-storage") - setupControllerCfg() perWorkspaceStorage := PerWorkspaceStorageProvisioner{} for _, tt := range tests { @@ -64,7 +61,7 @@ func TestRewriteContainerVolumeMountsForPerWorkspaceStorageClass(t *testing.T) { } if needsStorage(&workspace.Spec.Template) { - err := perWorkspaceStorage.ProvisionStorage(&tt.Input.PodAdditions, workspace, clusterAPI) + err := perWorkspaceStorage.ProvisionStorage(&tt.Input.PodAdditions, getDevWorkspaceWithConfig(workspace), clusterAPI) if !assert.Error(t, err, "Should get a NotReady error when creating PVC") { return } @@ -87,7 +84,7 @@ func TestRewriteContainerVolumeMountsForPerWorkspaceStorageClass(t *testing.T) { assert.Equal(t, retrievedPVC.ObjectMeta.OwnerReferences[0].Kind, "DevWorkspace") } - err := perWorkspaceStorage.ProvisionStorage(&tt.Input.PodAdditions, workspace, clusterAPI) + err := perWorkspaceStorage.ProvisionStorage(&tt.Input.PodAdditions, getDevWorkspaceWithConfig(workspace), clusterAPI) if tt.Output.ErrRegexp != nil && assert.Error(t, err) { assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") diff --git a/pkg/provision/storage/provisioner.go b/pkg/provision/storage/provisioner.go index 71185a9c6..16f794b54 100644 --- a/pkg/provision/storage/provisioner.go +++ b/pkg/provision/storage/provisioner.go @@ -17,6 +17,7 @@ package storage import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" @@ -29,18 +30,18 @@ type Provisioner interface { // out-of-pod required objects to the cluster. // Returns NotReadyError to signify that storage is not ready, ProvisioningError when a fatal issue is encountered, // and other error if there is an unexpected problem. - ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error + ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error // NeedsStorage returns whether the current workspace needs a PVC to be provisioned, given this storage strategy. NeedsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool // CleanupWorkspaceStorage removes any objects provisioned by in the ProvisionStorage step that aren't automatically removed when a // DevWorkspace is deleted (e.g. delete subfolders in a common PVC assigned to the workspace) // Returns nil on success (DevWorkspace can be deleted), NotReadyError if additional reconciles are necessary, ProvisioningError when // a fatal issue is encountered, and any other error if an unexpected problem arises. - CleanupWorkspaceStorage(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) error + CleanupWorkspaceStorage(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error } // GetProvisioner returns the storage provisioner that should be used for the current workspace -func GetProvisioner(workspace *dw.DevWorkspace) (Provisioner, error) { +func GetProvisioner(workspace *common.DevWorkspaceWithConfig) (Provisioner, error) { storageClass := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil) if storageClass == "" { return &CommonStorageProvisioner{}, nil diff --git a/pkg/provision/storage/shared.go b/pkg/provision/storage/shared.go index 31427c056..cfdcaa52f 100644 --- a/pkg/provision/storage/shared.go +++ b/pkg/provision/storage/shared.go @@ -20,6 +20,7 @@ import ( "fmt" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -29,14 +30,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" ) -func getPVCSpec(name, namespace string, size resource.Quantity) (*corev1.PersistentVolumeClaim, error) { +func getPVCSpec(name, namespace string, storageClass *string, size resource.Quantity) (*corev1.PersistentVolumeClaim, error) { return &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -52,7 +52,7 @@ func getPVCSpec(name, namespace string, size resource.Quantity) (*corev1.Persist "storage": size, }, }, - StorageClassName: config.Workspace.StorageClassName, + StorageClassName: storageClass, }, }, nil } @@ -85,7 +85,7 @@ func needsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool { return containerlib.AnyMountSources(workspace.Components) } -func syncCommonPVC(namespace string, clusterAPI sync.ClusterAPI) (*corev1.PersistentVolumeClaim, error) { +func syncCommonPVC(namespace string, config *v1alpha1.OperatorConfiguration, clusterAPI sync.ClusterAPI) (*corev1.PersistentVolumeClaim, error) { namespacedConfig, err := nsconfig.ReadNamespacedConfig(namespace, clusterAPI) if err != nil { return nil, fmt.Errorf("failed to read namespace-specific configuration: %w", err) @@ -98,7 +98,7 @@ func syncCommonPVC(namespace string, clusterAPI sync.ClusterAPI) (*corev1.Persis } } - pvc, err := getPVCSpec(config.Workspace.PVCName, namespace, pvcSize) + pvc, err := getPVCSpec(config.Workspace.PVCName, namespace, config.Workspace.StorageClassName, pvcSize) if err != nil { return nil, err } @@ -135,7 +135,7 @@ func syncCommonPVC(namespace string, clusterAPI sync.ClusterAPI) (*corev1.Persis // addEphemeralVolumesFromWorkspace adds emptyDir volumes for all ephemeral volume components required for a devworkspace. // This includes any volume components marked with the ephemeral field, including projects. // Returns a ProvisioningError if any ephemeral volume cannot be parsed (e.g. cannot parse size for kubernetes) -func addEphemeralVolumesFromWorkspace(workspace *dw.DevWorkspace, podAdditions *v1alpha1.PodAdditions) error { +func addEphemeralVolumesFromWorkspace(workspace *common.DevWorkspaceWithConfig, podAdditions *v1alpha1.PodAdditions) error { _, ephemeralVolumes, projectsVolume := getWorkspaceVolumes(workspace) _, err := addEphemeralVolumesToPodAdditions(podAdditions, ephemeralVolumes) if err != nil { @@ -179,7 +179,7 @@ func addEphemeralVolumesToPodAdditions(podAdditions *v1alpha1.PodAdditions, work // getWorkspaceVolumes returns all volumes defined in the DevWorkspace, separated out into persistent volumes, ephemeral // volumes, and the projects volume, which must be handled specially. If the workspace does not define a projects volume, // the returned value is nil. -func getWorkspaceVolumes(workspace *dw.DevWorkspace) (persistent, ephemeral []dw.Component, projects *dw.Component) { +func getWorkspaceVolumes(workspace *common.DevWorkspaceWithConfig) (persistent, ephemeral []dw.Component, projects *dw.Component) { for idx, component := range workspace.Spec.Template.Components { if component.Volume == nil { continue @@ -219,21 +219,22 @@ func processProjectsVolume(workspace *dw.DevWorkspaceTemplateSpec) (projectsComp return } -// checkForExistingCommonPVC checks the current namespace for existing PVCs that may be used for workspace storage. If +// checkForAlternatePVC checks the current namespace for existing PVCs that may be used for workspace storage. If // such a PVC is found, its name is returned and should be used in place of the configured common PVC. If no suitable // PVC is found, the returned PVC name is an empty string and a nil error is returned. If an error occurs during the lookup, // then an empty string is returned as well as the error. -func checkForExistingCommonPVC(namespace string, api sync.ClusterAPI) (string, error) { +// Currently, the only alternate PVC that can be used is named `claim-che-workspace`. +func checkForAlternatePVC(namespace string, api sync.ClusterAPI) (exists bool, name string, err error) { existingPVC := &corev1.PersistentVolumeClaim{} namespacedName := types.NamespacedName{Name: constants.CheCommonPVCName, Namespace: namespace} - err := api.Client.Get(api.Ctx, namespacedName, existingPVC) + err = api.Client.Get(api.Ctx, namespacedName, existingPVC) if err != nil { if k8sErrors.IsNotFound(err) { - return "", nil + return false, "", nil } - return "", err + return false, "", err } - return existingPVC.Name, nil + return true, existingPVC.Name, nil } // getSharedPVCWorkspaceCount returns the total number of workspaces which are using a shared PVC @@ -261,7 +262,8 @@ func getSharedPVCWorkspaceCount(namespace string, api sync.ClusterAPI) (total in func checkPVCTerminating(name, namespace string, api sync.ClusterAPI) (bool, error) { if name == "" { - name = config.Workspace.PVCName + // Should not happen + return false, fmt.Errorf("attempted to read deletion status of PVC with empty name") } pvc := &corev1.PersistentVolumeClaim{} namespacedName := types.NamespacedName{Name: name, Namespace: namespace} diff --git a/pkg/provision/workspace/deployment.go b/pkg/provision/workspace/deployment.go index bb3a35a79..96a5a065e 100644 --- a/pkg/provision/workspace/deployment.go +++ b/pkg/provision/workspace/deployment.go @@ -20,17 +20,15 @@ import ( "errors" "fmt" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" "github.com/devfile/devworkspace-operator/pkg/library/status" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" "github.com/devfile/devworkspace-operator/pkg/provision/sync" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" maputils "github.com/devfile/devworkspace-operator/internal/map" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" - "github.com/devfile/devworkspace-operator/pkg/infrastructure" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -48,7 +46,7 @@ type DeploymentProvisioningStatus struct { } func SyncDeploymentToCluster( - workspace *dw.DevWorkspace, + workspace *common.DevWorkspaceWithConfig, podAdditions []v1alpha1.PodAdditions, saName string, clusterAPI sync.ClusterAPI) DeploymentProvisioningStatus { @@ -116,9 +114,9 @@ func SyncDeploymentToCluster( } } - failureMsg, checkErr := status.CheckPodsState(workspace.Status.DevWorkspaceId, workspace.Namespace, k8sclient.MatchingLabels{ - constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, - }, clusterAPI) + workspaceIDLabel := k8sclient.MatchingLabels{constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId} + ignoredEvents := workspace.Config.Workspace.IgnoredUnrecoverableEvents + failureMsg, checkErr := status.CheckPodsState(workspace.Status.DevWorkspaceId, workspace.Namespace, workspaceIDLabel, ignoredEvents, clusterAPI) if checkErr != nil { return DeploymentProvisioningStatus{ ProvisioningStatus: ProvisioningStatus{ @@ -139,7 +137,7 @@ func SyncDeploymentToCluster( } // DeleteWorkspaceDeployment deletes the deployment for the DevWorkspace -func DeleteWorkspaceDeployment(ctx context.Context, workspace *dw.DevWorkspace, client runtimeClient.Client) (wait bool, err error) { +func DeleteWorkspaceDeployment(ctx context.Context, workspace *common.DevWorkspaceWithConfig, client runtimeClient.Client) (wait bool, err error) { err = client.Delete(ctx, &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: workspace.Namespace, @@ -156,7 +154,7 @@ func DeleteWorkspaceDeployment(ctx context.Context, workspace *dw.DevWorkspace, } // ScaleDeploymentToZero scales the cluster deployment to zero -func ScaleDeploymentToZero(ctx context.Context, workspace *dw.DevWorkspace, client runtimeClient.Client) error { +func ScaleDeploymentToZero(ctx context.Context, workspace *common.DevWorkspaceWithConfig, client runtimeClient.Client) error { patch := []byte(`{"spec":{"replicas": 0}}`) err := client.Patch(ctx, &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -172,15 +170,8 @@ func ScaleDeploymentToZero(ctx context.Context, workspace *dw.DevWorkspace, clie return nil } -func GetDevWorkspaceSecurityContext() *corev1.PodSecurityContext { - if infrastructure.IsOpenShift() { - return &corev1.PodSecurityContext{} - } - return config.Workspace.PodSecurityContext -} - func getSpecDeployment( - workspace *dw.DevWorkspace, + workspace *common.DevWorkspaceWithConfig, podAdditionsList []v1alpha1.PodAdditions, saName string, podTolerations []corev1.Toleration, @@ -207,6 +198,13 @@ func getSpecDeployment( annotations, err := getAdditionalAnnotations(workspace) + var securityContext *corev1.PodSecurityContext + if infrastructure.IsOpenShift() { + securityContext = &corev1.PodSecurityContext{} + } else { + securityContext = workspace.Config.Workspace.PodSecurityContext + } + deployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: common.DeploymentName(workspace.Status.DevWorkspaceId), @@ -240,7 +238,7 @@ func getSpecDeployment( Volumes: podAdditions.Volumes, RestartPolicy: "Always", TerminationGracePeriodSeconds: &terminationGracePeriod, - SecurityContext: GetDevWorkspaceSecurityContext(), + SecurityContext: securityContext, ServiceAccountName: saName, AutomountServiceAccountToken: nil, }, @@ -261,7 +259,7 @@ func getSpecDeployment( } } - if needPVC, pvcName := needsPVCWorkaround(podAdditions); needPVC { + if needPVC, pvcName := needsPVCWorkaround(podAdditions, workspace.Config.Workspace.PVCName); needPVC { // Kubernetes creates directories in a PVC to support subpaths such that only the leaf directory has g+rwx permissions. // This means that mounting the subpath e.g. /plugins will result in the directory being // created with 755 permissions, requiring the root UID to remove it. @@ -291,7 +289,7 @@ func getSpecDeployment( deployment.Spec.Template.Annotations = maputils.Append(deployment.Spec.Template.Annotations, constants.DevWorkspaceRestrictedAccessAnnotation, restrictedAccess) } - err = controllerutil.SetControllerReference(workspace, deployment, scheme) + err = controllerutil.SetControllerReference(workspace.DevWorkspace, deployment, scheme) if err != nil { return nil, err } @@ -368,11 +366,10 @@ func getWorkspaceSubpathVolumeMount(workspaceId, pvcName string) corev1.VolumeMo return workspaceVolumeMount } -func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions) (needs bool, pvcName string) { - commonPVCName := config.Workspace.PVCName +func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions, pvcName string) (needs bool, workaroundPvcName string) { for _, vol := range podAdditions.Volumes { - if vol.Name == commonPVCName { - return true, commonPVCName + if vol.Name == pvcName { + return true, pvcName } if vol.Name == constants.CheCommonPVCName { return true, constants.CheCommonPVCName @@ -381,7 +378,7 @@ func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions) (needs bool, pvcNam return false, "" } -func getAdditionalAnnotations(workspace *dw.DevWorkspace) (map[string]string, error) { +func getAdditionalAnnotations(workspace *common.DevWorkspaceWithConfig) (map[string]string, error) { annotations := map[string]string{} for _, component := range workspace.Spec.Template.Components { diff --git a/pkg/provision/workspace/rbac.go b/pkg/provision/workspace/rbac.go index 296492ddf..096015e51 100644 --- a/pkg/provision/workspace/rbac.go +++ b/pkg/provision/workspace/rbac.go @@ -16,7 +16,6 @@ package workspace import ( - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/provision/sync" @@ -26,7 +25,7 @@ import ( ) // SyncRBAC generates RBAC and synchronizes the runtime objects -func SyncRBAC(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) ProvisioningStatus { +func SyncRBAC(workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) ProvisioningStatus { rbacs := generateRBAC(workspace.Namespace) requeue := false for _, rbac := range rbacs { diff --git a/pkg/provision/workspace/routing.go b/pkg/provision/workspace/routing.go index 212f7906f..c78441207 100644 --- a/pkg/provision/workspace/routing.go +++ b/pkg/provision/workspace/routing.go @@ -18,14 +18,12 @@ package workspace import ( "strings" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/conversion" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" maputils "github.com/devfile/devworkspace-operator/internal/map" "github.com/devfile/devworkspace-operator/pkg/common" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -40,7 +38,7 @@ type RoutingProvisioningStatus struct { } func SyncRoutingToCluster( - workspace *dw.DevWorkspace, + workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) RoutingProvisioningStatus { specRouting, err := getSpecRouting(workspace, clusterAPI.Scheme) @@ -88,7 +86,7 @@ func SyncRoutingToCluster( } func getSpecRouting( - workspace *dw.DevWorkspace, + workspace *common.DevWorkspaceWithConfig, scheme *runtime.Scheme) (*v1alpha1.DevWorkspaceRouting, error) { endpoints := map[string]v1alpha1.EndpointList{} @@ -118,7 +116,7 @@ func getSpecRouting( routingClass := workspace.Spec.RoutingClass if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + routingClass = workspace.Config.Routing.DefaultRoutingClass } routing := &v1alpha1.DevWorkspaceRouting{ @@ -139,7 +137,7 @@ func getSpecRouting( }, }, } - err := controllerutil.SetControllerReference(workspace, routing, scheme) + err := controllerutil.SetControllerReference(workspace.DevWorkspace, routing, scheme) if err != nil { return nil, err } diff --git a/pkg/provision/workspace/serviceaccount.go b/pkg/provision/workspace/serviceaccount.go index 2a0f8643d..677041c06 100644 --- a/pkg/provision/workspace/serviceaccount.go +++ b/pkg/provision/workspace/serviceaccount.go @@ -39,7 +39,7 @@ type ServiceAcctProvisioningStatus struct { } func SyncServiceAccount( - workspace *dw.DevWorkspace, + workspace *common.DevWorkspaceWithConfig, additionalAnnotations map[string]string, clusterAPI sync.ClusterAPI) ServiceAcctProvisioningStatus { // note: autoMountServiceAccount := true comes from a hardcoded value in prerequisites.go @@ -65,7 +65,7 @@ func SyncServiceAccount( } } - err := controllerutil.SetControllerReference(workspace, specSA, clusterAPI.Scheme) + err := controllerutil.SetControllerReference(workspace.DevWorkspace, specSA, clusterAPI.Scheme) if err != nil { return ServiceAcctProvisioningStatus{ ProvisioningStatus: ProvisioningStatus{ @@ -115,7 +115,7 @@ func NeedsServiceAccountFinalizer(workspace *dw.DevWorkspaceTemplateSpec) bool { return true } -func FinalizeServiceAccount(workspace *dw.DevWorkspace, ctx context.Context, nonCachingClient crclient.Client) (retry bool, err error) { +func FinalizeServiceAccount(workspace *common.DevWorkspaceWithConfig, ctx context.Context, nonCachingClient crclient.Client) (retry bool, err error) { saName := common.ServiceAccountName(workspace.Status.DevWorkspaceId) namespace := workspace.Namespace if !workspace.Spec.Template.Attributes.Exists(constants.WorkspaceSCCAttribute) { diff --git a/pkg/timing/annotations.go b/pkg/timing/annotations.go index 7fa83c9bb..33949077a 100644 --- a/pkg/timing/annotations.go +++ b/pkg/timing/annotations.go @@ -18,7 +18,7 @@ package timing import ( "strconv" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" ) const ( @@ -59,7 +59,7 @@ type workspaceTimes struct { serversReady int64 } -func getTimestamps(workspace *dw.DevWorkspace) (*workspaceTimes, error) { +func getTimestamps(workspace *common.DevWorkspaceWithConfig) (*workspaceTimes, error) { times := &workspaceTimes{} // Will return an error if the annotation is unset t, err := strconv.ParseInt(workspace.Annotations[DevWorkspaceStarted], 10, 0) diff --git a/pkg/timing/timing.go b/pkg/timing/timing.go index 8a91432da..a73271964 100644 --- a/pkg/timing/timing.go +++ b/pkg/timing/timing.go @@ -20,7 +20,7 @@ import ( "strconv" "time" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/config" ) @@ -52,7 +52,7 @@ func CurrentTime() string { // SummarizeStartup applies aggregate annotations based off event annotations set by // SetTime(). No-op if timing is disabled or if not all event annotations are present // on the devworkspace. -func SummarizeStartup(workspace *dw.DevWorkspace) { +func SummarizeStartup(workspace *common.DevWorkspaceWithConfig) { if !IsEnabled() { return } @@ -75,7 +75,7 @@ func SummarizeStartup(workspace *dw.DevWorkspace) { // ClearAnnotations removes all timing-related annotations from a DevWorkspace. // It's necessary to call this before setting new times via SetTime(), as SetTime() // does not overwrite existing annotations. -func ClearAnnotations(workspace *dw.DevWorkspace) { +func ClearAnnotations(workspace *common.DevWorkspaceWithConfig) { if !IsEnabled() { return }