diff --git a/controllers/controller/devworkspacerouting/solvers/basic_solver.go b/controllers/controller/devworkspacerouting/solvers/basic_solver.go index 23beeccc3..c43797585 100644 --- a/controllers/controller/devworkspacerouting/solvers/basic_solver.go +++ b/controllers/controller/devworkspacerouting/solvers/basic_solver.go @@ -17,7 +17,6 @@ package solvers import ( controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) @@ -57,7 +56,7 @@ func (s *BasicSolver) Finalize(*controllerv1alpha1.DevWorkspaceRouting) error { func (s *BasicSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) (RoutingObjects, error) { routingObjects := RoutingObjects{} - routingSuffix := config.Routing.ClusterHostSuffix + routingSuffix := routing.Annotations[constants.ClusterHostSuffixAnnotation] if routingSuffix == "" { return routingObjects, &RoutingInvalid{"basic routing requires .config.routing.clusterHostSuffix to be set in operator config"} } diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index eab63771d..919202822 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -109,8 +109,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) + workspace := &common.DevWorkspaceWithConfig{} + err = r.Get(ctx, req.NamespacedName, &workspace.DevWorkspace) if err != nil { if k8sErrors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -121,6 +121,14 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Error reading the object - requeue the request. return reconcile.Result{}, err } + + // Get the config for the workspace + resolvedConfig, err := config.ResolveConfigForWorkspace(&workspace.DevWorkspace, clusterAPI.Client) + workspace.Config = *resolvedConfig + if err != nil { + reqLogger.Error(err, "Unable to apply external DevWorkspace-Operator configuration") + } + reqLogger = reqLogger.WithValues(constants.DevWorkspaceIDLoggerKey, workspace.Status.DevWorkspaceId) reqLogger.Info("Reconciling Workspace") @@ -133,14 +141,14 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Ensure workspaceID is set. if workspace.Status.DevWorkspaceId == "" { - workspaceId, err := r.getWorkspaceId(ctx, workspace) + workspaceId, err := r.getWorkspaceId(ctx, &workspace.DevWorkspace) 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 +165,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 } @@ -168,9 +176,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Handle stopped workspaces if !workspace.Spec.Started { - timing.ClearAnnotations(workspace) - r.removeStartedAtFromCluster(ctx, workspace, reqLogger) - r.syncTimingToCluster(ctx, workspace, map[string]string{}, reqLogger) + timing.ClearAnnotations(&workspace.DevWorkspace) + r.removeStartedAtFromCluster(ctx, &workspace.DevWorkspace, reqLogger) + r.syncTimingToCluster(ctx, &workspace.DevWorkspace, map[string]string{}, reqLogger) return r.stopWorkspace(ctx, workspace, reqLogger) } @@ -188,7 +196,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,30 +206,33 @@ 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{ + DevWorkspace: *workspace.DevWorkspace.DeepCopy(), + Config: *workspace.Config.DeepCopy(), + } timingInfo := map[string]string{} timing.SetTime(timingInfo, timing.DevWorkspaceStarted) defer func() (reconcile.Result, error) { - r.syncTimingToCluster(ctx, clusterWorkspace, timingInfo, reqLogger) + r.syncTimingToCluster(ctx, &clusterWorkspace.DevWorkspace, timingInfo, reqLogger) // Don't accidentally suppress errors by overwriting here; only check for timeout when no error // encountered in main reconcile loop. if err == nil { - if timeoutErr := checkForStartTimeout(clusterWorkspace); timeoutErr != nil { + if timeoutErr := checkForStartTimeout(&clusterWorkspace.DevWorkspace, clusterWorkspace.Config); timeoutErr != nil { reconcileResult, err = r.failWorkspace(workspace, timeoutErr.Error(), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus) } } if reconcileStatus.phase == dw.DevWorkspaceStatusRunning { metrics.WorkspaceRunning(clusterWorkspace, reqLogger) - r.syncStartedAtToCluster(ctx, clusterWorkspace, reqLogger) + r.syncStartedAtToCluster(ctx, &clusterWorkspace.DevWorkspace, reqLogger) } return r.updateWorkspaceStatus(clusterWorkspace, reqLogger, &reconcileStatus, reconcileResult, err) }() if workspace.Annotations[constants.DevWorkspaceRestrictedAccessAnnotation] == "true" { - msg, err := r.validateCreatorLabel(clusterWorkspace) + msg, err := r.validateCreatorLabel(&clusterWorkspace.DevWorkspace) if err != nil { return r.failWorkspace(workspace, msg, metrics.ReasonWorkspaceEngineFailure, reqLogger, &reconcileStatus) } @@ -229,7 +240,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 } @@ -266,7 +277,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } } - storageProvisioner, err := storage.GetProvisioner(workspace) + storageProvisioner, err := storage.GetProvisioner(&workspace.DevWorkspace) if err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Error provisioning storage: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus) } @@ -274,24 +285,24 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Set finalizer on DevWorkspace if necessary // 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 { + coputil.AddFinalizer(&clusterWorkspace.DevWorkspace, constants.StorageCleanupFinalizer) + 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) if err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Error processing devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus) } // Add common environment variables and env vars defined via workspaceEnv attribute - if err := env.AddCommonEnvironmentVariables(devfilePodAdditions, clusterWorkspace, &workspace.Spec.Template); err != nil { + if err := env.AddCommonEnvironmentVariables(devfilePodAdditions, &clusterWorkspace.DevWorkspace, &workspace.Spec.Template, clusterWorkspace.Config); err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Failed to process workspace environment variables: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus) } // Add init container to clone projects - if projectClone, err := projects.GetProjectCloneInitContainer(&workspace.Spec.Template); err != nil { + if projectClone, err := projects.GetProjectCloneInitContainer(workspace); 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) @@ -328,7 +339,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request timing.SetTime(timingInfo, timing.ComponentsReady) - rbacStatus := wsprovision.SyncRBAC(workspace, clusterAPI) + rbacStatus := wsprovision.SyncRBAC(&workspace.DevWorkspace, clusterAPI) if rbacStatus.Err != nil || !rbacStatus.Continue { return reconcile.Result{Requeue: true}, rbacStatus.Err } @@ -354,7 +365,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request reconcileStatus.setConditionTrue(dw.DevWorkspaceRoutingReady, "Networking ready") timing.SetTime(timingInfo, timing.RoutingReady) - statusOk, err := syncWorkspaceMainURL(clusterWorkspace, routingStatus.ExposedEndpoints, clusterAPI) + statusOk, err := syncWorkspaceMainURL(&clusterWorkspace.DevWorkspace, routingStatus.ExposedEndpoints, clusterAPI) if err != nil { return reconcile.Result{}, err } @@ -366,7 +377,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request annotate.AddURLAttributesToEndpoints(&workspace.Spec.Template, routingStatus.ExposedEndpoints) // Step three: provision a configmap on the cluster to mount the flattened devfile in deployment containers - err = metadata.ProvisionWorkspaceMetadata(devfilePodAdditions, clusterWorkspace, workspace, clusterAPI) + err = metadata.ProvisionWorkspaceMetadata(devfilePodAdditions, &clusterWorkspace.DevWorkspace, &workspace.DevWorkspace, clusterAPI) if err != nil { switch provisionErr := err.(type) { case *metadata.NotReadyError: @@ -391,7 +402,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request if routingPodAdditions != nil { saAnnotations = routingPodAdditions.ServiceAccountAnnotations } - serviceAcctStatus := wsprovision.SyncServiceAccount(workspace, saAnnotations, clusterAPI) + serviceAcctStatus := wsprovision.SyncServiceAccount(&workspace.DevWorkspace, saAnnotations, clusterAPI) if !serviceAcctStatus.Continue { if serviceAcctStatus.FailStartup { return r.failWorkspace(workspace, serviceAcctStatus.Message, metrics.ReasonBadRequest, reqLogger, &reconcileStatus) @@ -404,8 +415,8 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request return reconcile.Result{Requeue: serviceAcctStatus.Requeue}, serviceAcctStatus.Err } if wsprovision.NeedsServiceAccountFinalizer(&workspace.Spec.Template) { - coputil.AddFinalizer(clusterWorkspace, constants.ServiceAccountCleanupFinalizer) - if err := r.Update(ctx, clusterWorkspace); err != nil { + coputil.AddFinalizer(&clusterWorkspace.DevWorkspace, constants.ServiceAccountCleanupFinalizer) + if err := r.Update(ctx, &clusterWorkspace.DevWorkspace); err != nil { return reconcile.Result{}, err } } @@ -426,7 +437,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request // Step six: Create deployment and wait for it to be ready timing.SetTime(timingInfo, timing.DeploymentCreated) - deploymentStatus := wsprovision.SyncDeploymentToCluster(workspace, allPodAdditions, serviceAcctName, clusterAPI) + deploymentStatus := wsprovision.SyncDeploymentToCluster(&workspace.DevWorkspace, allPodAdditions, serviceAcctName, clusterAPI, &workspace.Config) if !deploymentStatus.Continue { if deploymentStatus.FailStartup { failureReason := metrics.DetermineProvisioningFailureReason(deploymentStatus) @@ -442,7 +453,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request reconcileStatus.setConditionTrue(conditions.DeploymentReady, "DevWorkspace deployment ready") timing.SetTime(timingInfo, timing.DeploymentReady) - serverReady, err := checkServerStatus(clusterWorkspace) + serverReady, err := checkServerStatus(&clusterWorkspace.DevWorkspace) if err != nil { return reconcile.Result{}, err } @@ -451,13 +462,13 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request return reconcile.Result{RequeueAfter: 1 * time.Second}, nil } timing.SetTime(timingInfo, timing.DevWorkspaceReady) - timing.SummarizeStartup(clusterWorkspace) + timing.SummarizeStartup(&clusterWorkspace.DevWorkspace) reconcileStatus.setConditionTrue(dw.DevWorkspaceReady, "") reconcileStatus.phase = dw.DevWorkspaceStatusRunning 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 +499,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,11 +536,11 @@ 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") - err = wsprovision.ScaleDeploymentToZero(ctx, workspace, r.Client) + err = wsprovision.ScaleDeploymentToZero(ctx, &workspace.DevWorkspace, r.Client) if err != nil && !k8sErrors.IsConflict(err) { return false, err } @@ -539,7 +550,7 @@ func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWo return workspaceDeployment.Status.Replicas == 0, nil } else { logger.Info("Cleaning up workspace-owned objects") - requeue, err := r.deleteWorkspaceOwnedObjects(ctx, workspace) + requeue, err := r.deleteWorkspaceOwnedObjects(ctx, &workspace.DevWorkspace) return !requeue, err } } @@ -547,7 +558,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)) @@ -676,8 +687,9 @@ func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Req } } + // TODO: Address this usage of global config. It'd be better to check if the PVC had a DWO-specific annotation that we can use to filter for // 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..6b027580b 100644 --- a/controllers/workspace/finalize.go +++ b/controllers/workspace/finalize.go @@ -18,10 +18,10 @@ package controllers import ( "context" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/conditions" "github.com/devfile/devworkspace-operator/pkg/constants" - - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/provision/sync" "github.com/go-logr/logr" @@ -46,7 +46,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. @@ -68,15 +68,15 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, case constants.StorageCleanupFinalizer: return r.finalizeStorage(ctx, log, workspace, finalizeStatus) case constants.ServiceAccountCleanupFinalizer: - return r.finalizeServiceAccount(ctx, log, workspace, finalizeStatus) + return r.finalizeServiceAccount(ctx, log, &workspace.DevWorkspace, finalizeStatus) } } 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) + wait, err := wsprovision.DeleteWorkspaceDeployment(ctx, &workspace.DevWorkspace, r.Client) if err != nil { return reconcile.Result{}, err } @@ -91,10 +91,10 @@ 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) + storageProvisioner, err := storage.GetProvisioner(&workspace.DevWorkspace) if err != nil { log.Error(err, "Failed to clean up DevWorkspace storage") finalizeStatus.phase = dw.DevWorkspaceStatusError @@ -126,7 +126,7 @@ 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) { diff --git a/controllers/workspace/http.go b/controllers/workspace/http.go index 5e032d440..e1bd0ad3c 100644 --- a/controllers/workspace/http.go +++ b/controllers/workspace/http.go @@ -34,11 +34,15 @@ func setupHttpClients() { InsecureSkipVerify: true, } - if config.Routing != nil && config.Routing.ProxyConfig != nil { + routingConfig := config.GetGlobalConfig().Routing + + // Note: per-workspace routing settings (that have been set with an external DWOC) + // are ignored for the HTTP Clients + if routingConfig != nil && routingConfig.ProxyConfig != nil { proxyConf := httpproxy.Config{ - HTTPProxy: config.Routing.ProxyConfig.HttpProxy, - HTTPSProxy: config.Routing.ProxyConfig.HttpsProxy, - NoProxy: config.Routing.ProxyConfig.NoProxy, + HTTPProxy: routingConfig.ProxyConfig.HttpProxy, + HTTPSProxy: routingConfig.ProxyConfig.HttpsProxy, + NoProxy: routingConfig.ProxyConfig.NoProxy, } proxyFunc := func(req *http.Request) (*url.URL, error) { return proxyConf.ProxyFunc()(req.URL) diff --git a/controllers/workspace/metrics/update.go b/controllers/workspace/metrics/update.go index 40c3f6af0..1eebef66e 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) @@ -51,14 +51,14 @@ func WorkspaceFailed(wksp *dw.DevWorkspace, log logr.Logger) { incrementMetricForWorkspaceFailure(workspaceFailures, wksp, log) } -func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *dw.DevWorkspace, log logr.Logger) { +func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *common.DevWorkspaceWithConfig, log logr.Logger) { sourceLabel := wksp.Labels[workspaceSourceLabel] if sourceLabel == "" { sourceLabel = "unknown" } routingClass := wksp.Spec.RoutingClass if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + routingClass = wksp.Config.Routing.DefaultRoutingClass } ctr, err := metric.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass}) if err != nil { @@ -80,14 +80,14 @@ func incrementMetricForWorkspaceFailure(metric *prometheus.CounterVec, wksp *dw. ctr.Inc() } -func incrementStartTimeBucketForWorkspace(wksp *dw.DevWorkspace, log logr.Logger) { +func incrementStartTimeBucketForWorkspace(wksp *common.DevWorkspaceWithConfig, log logr.Logger) { sourceLabel := wksp.Labels[workspaceSourceLabel] if sourceLabel == "" { sourceLabel = "unknown" } routingClass := wksp.Spec.RoutingClass if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + routingClass = wksp.Config.Routing.DefaultRoutingClass } hist, err := workspaceStartupTimesHist.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass}) if err != nil { diff --git a/controllers/workspace/status.go b/controllers/workspace/status.go index 2eb36ec89..ba4cd8473 100644 --- a/controllers/workspace/status.go +++ b/controllers/workspace/status.go @@ -23,8 +23,8 @@ 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" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -35,7 +35,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,12 +63,12 @@ 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 - infoMessage := getInfoMessage(workspace, status) + infoMessage := getInfoMessage(&workspace.DevWorkspace, status) if warn := conditions.GetConditionByType(workspace.Status.Conditions, conditions.DevWorkspaceWarning); warn != nil && warn.Status == corev1.ConditionTrue { infoMessage = fmt.Sprintf("%s %s", warningPresentInfoMessage, infoMessage) } @@ -77,7 +76,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") @@ -225,7 +224,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 } @@ -233,7 +232,7 @@ func updateMetricsForPhase(workspace *dw.DevWorkspace, oldPhase, newPhase dw.Dev case dw.DevWorkspaceStatusRunning: metrics.WorkspaceRunning(workspace, logger) case dw.DevWorkspaceStatusFailed: - metrics.WorkspaceFailed(workspace, logger) + metrics.WorkspaceFailed(&workspace.DevWorkspace, logger) } } @@ -241,7 +240,7 @@ 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 *dw.DevWorkspace, config v1alpha1.OperatorConfiguration) error { if workspace.Status.Phase != dw.DevWorkspaceStatusStarting { return nil } @@ -267,11 +266,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/pkg/common/types.go b/pkg/common/types.go new file mode 100644 index 000000000..3c679ada7 --- /dev/null +++ b/pkg/common/types.go @@ -0,0 +1,26 @@ +// 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" +) + +//TODO: Rename? +// TODO: Move +type DevWorkspaceWithConfig struct { + dw.DevWorkspace `json:",inline"` + Config controllerv1alpha1.OperatorConfiguration +} diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index f05ead396..3aae90e47 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -68,3 +68,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..b844e65ed 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -21,38 +21,75 @@ import ( "strings" "sync" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/config/proxy" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" routeV1 "github.com/openshift/api/route/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" crclient "sigs.k8s.io/controller-runtime/pkg/client" - - controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" - "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) const ( - OperatorConfigName = "devworkspace-operator-config" - openShiftTestRouteName = "devworkspace-controller-test-route" + OperatorConfigName = "devworkspace-operator-config" + openShiftTestRouteName = "devworkspace-controller-test-route" + ExternalConfigName = "external-config-name" + ExternalConfigNamespace = "external-config-namespace" ) var ( - Routing *controller.RoutingConfig - Workspace *controller.WorkspaceConfig + 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 SetConfigForTesting(config *controller.OperatorConfiguration) *controller.OperatorConfiguration { configMutex.Lock() defer configMutex.Unlock() internalConfig = defaultConfig.DeepCopy() mergeConfig(config, internalConfig) updatePublicConfig() + return internalConfig +} + +// Checks for a external DevWorkspace-Operator configuration specified by the optional workspace attribute "controller.devfile.io/devworkspace-config" +// If the external DWOC is found, it is merged with the internal/global DWOC and returned. +// If the exteranl DWOC is not found, or the "controller.devfile.io/devworkspace-config" attribute is not set, then the internal/global DWOC is returned. +func ResolveConfigForWorkspace(workspace *dw.DevWorkspace, client crclient.Client) (*controller.OperatorConfiguration, error) { + internalConfigCopy := internalConfig.DeepCopy() + if workspace.Spec.Template.Attributes.Exists(constants.ExternalDevWorkspaceConfiguration) { + namespacedName := types.NamespacedName{} + + err := workspace.Spec.Template.Attributes.GetInto(constants.ExternalDevWorkspaceConfiguration, &namespacedName) + if err != nil { + return internalConfigCopy, fmt.Errorf("failed to read attribute %s in DevWorkspace attributes: %w", constants.ExternalDevWorkspaceConfiguration, err) + } + + if namespacedName.Name == "" { + return internalConfigCopy, fmt.Errorf("'name' must be set for attribute %s in DevWorkspace attributes", constants.ExternalDevWorkspaceConfiguration) + } + + if namespacedName.Namespace == "" { + return internalConfigCopy, 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 internalConfigCopy, 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 + } + + return internalConfigCopy, nil } func SetupControllerConfig(client crclient.Client) error { @@ -108,6 +145,11 @@ func ExperimentalFeaturesEnabled() bool { return *internalConfig.EnableExperimentalFeatures } +func GetGlobalConfig() *controller.OperatorConfiguration { + // TODO: Could make this return a deepcopy to prevent modifying the global config when we only intend for reads + return internalConfig +} + func getClusterConfig(namespace string, client crclient.Client) (*controller.DevWorkspaceOperatorConfig, error) { clusterConfig := &controller.DevWorkspaceOperatorConfig{} if err := client.Get(context.Background(), types.NamespacedName{Name: OperatorConfigName, Namespace: namespace}, clusterConfig); err != nil { @@ -130,6 +172,14 @@ func syncConfigFrom(newConfig *controller.DevWorkspaceOperatorConfig) { updatePublicConfig() } +func getMergedConfig(from, to *controller.OperatorConfiguration) *controller.OperatorConfiguration { + mergedConfig := to.DeepCopy() + // TODO: Remove this comment: technically we don't need to deepCopy 'from', but we may use this function elsewhere in the future, so just incase.. + fromCopy := from.DeepCopy() + mergeConfig(fromCopy, mergedConfig) + return mergedConfig +} + func restoreDefaultConfig() { configMutex.Lock() defer configMutex.Unlock() @@ -138,8 +188,8 @@ func restoreDefaultConfig() { } func updatePublicConfig() { - Routing = internalConfig.Routing.DeepCopy() - Workspace = internalConfig.Workspace.DeepCopy() + routing = internalConfig.Routing.DeepCopy() + workspace = internalConfig.Workspace.DeepCopy() logCurrentConfig() } @@ -266,40 +316,40 @@ func logCurrentConfig() { return } 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)) + 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 { + 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 { + config = append(config, fmt.Sprintf("workspace.defaultStorageSize.perWorkspace=%s", workspace.DefaultStorageSize.PerWorkspace.String())) } } - if Workspace.DefaultTemplate != nil { + if workspace.DefaultTemplate != nil { config = append(config, fmt.Sprintf("workspace.defaultTemplate is set")) } } diff --git a/pkg/config/sync_test.go b/pkg/config/sync_test.go index 12af19df3..0aaf222ff 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" ) @@ -48,8 +54,8 @@ func TestSetupControllerDefaultsAreExported(t *testing.T) { 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") + 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) { @@ -92,8 +98,101 @@ 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")) + 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, + } + + clusterConfig := buildConfig(defaultConfig.DeepCopy()) + originalInternalConfig := clusterConfig.Config.DeepCopy() + + // 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, + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(clusterConfig).WithObjects(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(originalInternalConfig, internalConfig) { + t.Error("Internal config should remain the same after merge") + } + + // 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 +218,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", routing.ClusterHostSuffix, "Should use value from config for clusterRoutingSuffix") } func TestDetectsOpenShiftRouteSuffix(t *testing.T) { @@ -196,10 +295,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", 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", routing.ClusterHostSuffix, "Should restore default clusterRoutingSuffix if it is available") } func TestSyncConfigDoesNotEraseClusterRoutingSuffix(t *testing.T) { @@ -219,7 +318,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", routing.ClusterHostSuffix, "Should get clusterHostSuffix from route on OpenShift") syncConfigFrom(buildConfig(&v1alpha1.OperatorConfiguration{ Routing: &v1alpha1.RoutingConfig{ DefaultRoutingClass: "test-routingClass", @@ -231,7 +330,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", 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..f381b6315 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 DevWorkspace-Operator configuration (DWOC) + // which will merged with the internal/global DevWorkspace-Operator configuration. The DWOC resulting from the merge will be used for the workspace. + // The fields which are set in the external DevWorkspace-Operator configuration will overwrite those existing in the + // internal/global DevWorkspace-Operator configuration + // The structure of the attribute value should contain two strings: name and namespace. + // 'name' specifies the metadata.name of the external DevWorkspace-Operator configuration + // 'namespace' specifies the metadata.namespace of the external DevWorkspace-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/constants/metadata.go b/pkg/constants/metadata.go index 4e99b4bba..2288942bc 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -117,6 +117,9 @@ const ( // The full annotation name is supposed to be ".routing.controller.devfile.io/" RoutingAnnotationInfix = ".routing.controller.devfile.io/" + // ClusterHostSuffixAnnotation is an annotation key for storing the value of the cluster host suffix to be used by the Basic DevWorkspace Routing Class. + ClusterHostSuffixAnnotation = "basic.routing.controller.devfile.io/cluster-host-suffix" + // DevWorkspaceEndpointNameAnnotation is the annotation key for storing an endpoint's name from the devfile representation DevWorkspaceEndpointNameAnnotation = "controller.devfile.io/endpoint_name" diff --git a/pkg/library/container/container.go b/pkg/library/container/container.go index c55eb8aaa..4b2c6a7a9 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, config v1alpha1.OperatorConfiguration) (*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, config) 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, config) if err != nil { return nil, err } diff --git a/pkg/library/container/container_test.go b/pkg/library/container/container_test.go index a30e96fda..ffaf68118 100644 --- a/pkg/library/container/container_test.go +++ b/pkg/library/container/container_test.go @@ -88,7 +88,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) { 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, *testControllerCfg) 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..e7c1d35f4 100644 --- a/pkg/library/container/conversion.go +++ b/pkg/library/container/conversion.go @@ -19,16 +19,16 @@ import ( "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/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, config controllerv1alpha1.OperatorConfiguration) (*v1.Container, error) { if devfileComponent.Container == nil { return nil, fmt.Errorf("cannot get k8s container from non-container component") } 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..ed92a74d0 100644 --- a/pkg/library/env/workspaceenv.go +++ b/pkg/library/env/workspaceenv.go @@ -21,17 +21,17 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" 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/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 *dw.DevWorkspace, flattenedDW *dw.DevWorkspaceTemplateSpec, config controllerv1alpha1.OperatorConfiguration) error { + commonEnv := commonEnvironmentVariables(clusterDW.Name, clusterDW.Status.DevWorkspaceId, clusterDW.Namespace, clusterDW.Labels[constants.DevWorkspaceCreatorLabel], config) workspaceEnv, err := collectWorkspaceEnv(flattenedDW) if err != nil { return err @@ -47,7 +47,7 @@ func AddCommonEnvironmentVariables(podAdditions *v1alpha1.PodAdditions, clusterD return nil } -func commonEnvironmentVariables(workspaceName, workspaceId, namespace, creator string) []corev1.EnvVar { +func commonEnvironmentVariables(workspaceName, workspaceId, namespace, creator string, config controllerv1alpha1.OperatorConfiguration) []corev1.EnvVar { envvars := []corev1.EnvVar{ { Name: constants.DevWorkspaceNamespace, @@ -71,12 +71,12 @@ func commonEnvironmentVariables(workspaceName, workspaceId, namespace, creator s }, } - envvars = append(envvars, getProxyEnvVars()...) + envvars = append(envvars, getProxyEnvVars(config)...) return envvars } -func getProxyEnvVars() []corev1.EnvVar { +func getProxyEnvVars(config controllerv1alpha1.OperatorConfiguration) []corev1.EnvVar { if config.Routing.ProxyConfig == nil { return nil } diff --git a/pkg/library/projects/clone.go b/pkg/library/projects/clone.go index ec7c806ee..79a312146 100644 --- a/pkg/library/projects/clone.go +++ b/pkg/library/projects/clone.go @@ -19,12 +19,12 @@ 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" "github.com/devfile/devworkspace-operator/internal/images" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" ) @@ -32,14 +32,15 @@ const ( projectClonerContainerName = "project-clone" ) -func GetProjectCloneInitContainer(workspace *dw.DevWorkspaceTemplateSpec) (*corev1.Container, error) { - if len(workspace.Projects) == 0 { +func GetProjectCloneInitContainer(workspace *common.DevWorkspaceWithConfig) (*corev1.Container, error) { + + if len(workspace.Spec.Template.Projects) == 0 { return nil, nil } - if workspace.Attributes.GetString(constants.ProjectCloneAttribute, nil) == constants.ProjectCloneDisable { + if workspace.Spec.Template.Attributes.GetString(constants.ProjectCloneAttribute, nil) == constants.ProjectCloneDisable { return nil, nil } - if !hasContainerComponents(workspace) { + if !hasContainerComponents(&workspace.Spec.Template) { // Avoid adding project-clone init container when DevWorkspace does not define any containers return nil, nil } @@ -92,7 +93,7 @@ func GetProjectCloneInitContainer(workspace *dw.DevWorkspaceTemplateSpec) (*core MountPath: constants.DefaultProjectsSourcesRoot, }, }, - ImagePullPolicy: corev1.PullPolicy(config.Workspace.ImagePullPolicy), + ImagePullPolicy: corev1.PullPolicy(workspace.Config.Workspace.ImagePullPolicy), }, nil } diff --git a/pkg/library/status/check.go b/pkg/library/status/check.go index d36dc9d1f..c86bd147f 100644 --- a/pkg/library/status/check.go +++ b/pkg/library/status/check.go @@ -20,8 +20,8 @@ import ( "fmt" "strings" + controllerv1alpha1 "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/infrastructure" "github.com/devfile/devworkspace-operator/pkg/provision/sync" appsv1 "k8s.io/api/apps/v1" @@ -75,7 +75,7 @@ func CheckDeploymentConditions(deployment *appsv1.Deployment) (healthy bool, err // Returns optional message with detected unrecoverable state details // error if any happens during check func CheckPodsState(workspaceID string, namespace string, labelSelector k8sclient.MatchingLabels, - clusterAPI sync.ClusterAPI) (stateMsg string, checkFailure error) { + clusterAPI sync.ClusterAPI, config *controllerv1alpha1.OperatorConfiguration) (stateMsg string, checkFailure error) { podList := &corev1.PodList{} if err := clusterAPI.Client.List(context.TODO(), podList, k8sclient.InNamespace(namespace), labelSelector); err != nil { return "", err @@ -83,25 +83,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, config) 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, config) 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, clusterAPI, config); 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, clusterAPI sync.ClusterAPI, config *controllerv1alpha1.OperatorConfiguration) (msg string, err error) { evs := &corev1.EventList{} selector, err := fields.ParseSelector(fmt.Sprintf("involvedObject.name=%s", pod.Name)) if err != nil { @@ -124,7 +124,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, config) && 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 +138,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, config *controllerv1alpha1.OperatorConfiguration) (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, config), containerStatus.State.Waiting.Reason } } } @@ -150,14 +150,14 @@ 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, config), containerStatus.State.Terminated.Reason } } } return true, "" } -func checkIfUnrecoverableEventIgnored(reason string) (ignored bool) { +func checkIfUnrecoverableEventIgnored(reason string, config *controllerv1alpha1.OperatorConfiguration) (ignored bool) { for _, ignoredReason := range config.Workspace.IgnoredUnrecoverableEvents { if ignoredReason == reason { return true diff --git a/pkg/provision/storage/asyncStorage.go b/pkg/provision/storage/asyncStorage.go index 2c457ac8a..ed101c68a 100644 --- a/pkg/provision/storage/asyncStorage.go +++ b/pkg/provision/storage/asyncStorage.go @@ -29,6 +29,7 @@ import ( "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/internal/images" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" "github.com/devfile/devworkspace-operator/pkg/provision/storage/asyncstorage" @@ -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()), @@ -67,7 +68,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd } // Add ephemeral volumes - if err := addEphemeralVolumesFromWorkspace(workspace, podAdditions); err != nil { + if err := addEphemeralVolumesFromWorkspace(&workspace.DevWorkspace, podAdditions); err != nil { return err } @@ -77,7 +78,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd } // Sync SSH keypair to cluster - secret, configmap, err := asyncstorage.GetOrCreateSSHConfig(workspace, clusterAPI) + secret, configmap, err := asyncstorage.GetOrCreateSSHConfig(&workspace.DevWorkspace, clusterAPI) if err != nil { if errors.Is(err, asyncstorage.NotReadyError) { return &NotReadyError{ @@ -93,7 +94,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd return err } - pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI) + pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI, workspace.Config) if err != nil { return err } else if pvcTerminating { @@ -105,7 +106,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd if pvcName != "" { // Create common PVC if needed - clusterPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI) + clusterPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI, workspace.Config) if err != nil { return err } @@ -113,7 +114,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.Namespace, configmap, pvcName, clusterAPI, workspace.Config) if err != nil { if errors.Is(err, asyncstorage.NotReadyError) { return &NotReadyError{ @@ -156,7 +157,7 @@ func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdd return err } - volumes, err := p.addVolumesForAsyncStorage(podAdditions, workspace) + volumes, err := p.addVolumesForAsyncStorage(podAdditions, &workspace.DevWorkspace) if err != nil { return err } @@ -169,7 +170,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 { @@ -223,7 +224,7 @@ func (p *AsyncStorageProvisioner) CleanupWorkspaceStorage(workspace *dw.DevWorks return err } - retry, err := asyncstorage.RemoveAuthorizedKeyFromConfigMap(workspace, clusterAPI) + retry, err := asyncstorage.RemoveAuthorizedKeyFromConfigMap(&workspace.DevWorkspace, clusterAPI) if err != nil { return &ProvisioningError{ Message: "Failed to remove authorized key from async storage configmap", diff --git a/pkg/provision/storage/asyncstorage/deployment.go b/pkg/provision/storage/asyncstorage/deployment.go index cafdb0195..1b6a5a8eb 100644 --- a/pkg/provision/storage/asyncstorage/deployment.go +++ b/pkg/provision/storage/asyncstorage/deployment.go @@ -16,6 +16,7 @@ package asyncstorage import ( + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/internal/images" nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config" "github.com/devfile/devworkspace-operator/pkg/provision/sync" @@ -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) { +func SyncWorkspaceSyncDeploymentToCluster(namespace string, sshConfigMap *corev1.ConfigMap, pvcName string, clusterAPI sync.ClusterAPI, config controllerv1alpha1.OperatorConfiguration) (*appsv1.Deployment, error) { podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(namespace, clusterAPI) if err != nil { return nil, err } - specDeployment := getWorkspaceSyncDeploymentSpec(namespace, sshConfigMap, pvcName, podTolerations, nodeSelector) + specDeployment := getWorkspaceSyncDeploymentSpec(namespace, sshConfigMap, pvcName, podTolerations, nodeSelector, config) clusterObj, err := sync.SyncObjectWithCluster(specDeployment, clusterAPI) switch err.(type) { case nil: @@ -58,7 +59,7 @@ func getWorkspaceSyncDeploymentSpec( sshConfigMap *corev1.ConfigMap, pvcName string, tolerations []corev1.Toleration, - nodeSelector map[string]string) *appsv1.Deployment { + nodeSelector map[string]string, config controllerv1alpha1.OperatorConfiguration) *appsv1.Deployment { replicas := int32(1) terminationGracePeriod := int64(1) @@ -147,7 +148,7 @@ func getWorkspaceSyncDeploymentSpec( }, }, TerminationGracePeriodSeconds: &terminationGracePeriod, - SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(), + SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(config), AutomountServiceAccountToken: nil, }, }, diff --git a/pkg/provision/storage/cleanup.go b/pkg/provision/storage/cleanup.go index ac81e22be..61f8377cf 100644 --- a/pkg/provision/storage/cleanup.go +++ b/pkg/provision/storage/cleanup.go @@ -20,7 +20,6 @@ import ( "path" "time" - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "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,7 +34,6 @@ 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" ) @@ -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,7 @@ 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) + msg, err := status.CheckPodsState(workspace.Status.DevWorkspaceId, clusterJob.Namespace, k8sclient.MatchingLabels{"job-name": common.PVCCleanupJobName(workspace.Status.DevWorkspaceId)}, clusterAPI, &workspace.Config) if err != nil { return &ProvisioningError{ Err: err, @@ -115,7 +113,7 @@ 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) @@ -123,7 +121,7 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus return nil, err } if pvcName == "" { - pvcName = config.Workspace.PVCName + pvcName = workspace.Config.Workspace.PVCName } jobLabels := map[string]string{ @@ -150,7 +148,7 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus }, Spec: corev1.PodSpec{ RestartPolicy: "Never", - SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(), + SecurityContext: wsprovision.GetDevWorkspaceSecurityContext(workspace.Config), Volumes: []corev1.Volume{ { Name: pvcName, @@ -210,9 +208,9 @@ func getSpecCommonPVCCleanupJob(workspace *dw.DevWorkspace, clusterAPI sync.Clus 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..7dddb4068 100644 --- a/pkg/provision/storage/commonStorage.go +++ b/pkg/provision/storage/commonStorage.go @@ -20,9 +20,9 @@ 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/provision/sync" + "github.com/devfile/devworkspace-operator/pkg/common" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -42,9 +42,9 @@ 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 { + if err := addEphemeralVolumesFromWorkspace(&workspace.DevWorkspace, podAdditions); err != nil { return err } @@ -58,7 +58,7 @@ func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAd return err } - pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI) + pvcTerminating, err := checkPVCTerminating(pvcName, workspace.Namespace, clusterAPI, workspace.Config) if err != nil { return err } else if pvcTerminating { @@ -69,7 +69,7 @@ func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAd } if pvcName == "" { - commonPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI) + commonPVC, err := syncCommonPVC(workspace.Namespace, clusterAPI, workspace.Config) if err != nil { return err } @@ -86,7 +86,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 +98,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..21b49aa72 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" @@ -74,7 +75,7 @@ var testControllerCfg = &v1alpha1.OperatorConfiguration{ } func setupControllerCfg() { - config.SetConfigForTesting(testControllerCfg) + testControllerCfg = config.SetConfigForTesting(testControllerCfg) } func loadTestCaseOrPanic(t *testing.T, testFilepath string) testCase { @@ -125,7 +126,7 @@ 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", resource.MustParse("10Gi"), *testControllerCfg) if err != nil { t.Fatalf("Failure during setup: %s", err) } @@ -139,7 +140,8 @@ func TestProvisionStorageForCommonStorageClass(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") - workspace := &dw.DevWorkspace{} + workspace := &common.DevWorkspaceWithConfig{} + workspace.Config = *testControllerCfg workspace.Spec.Template = *tt.Input.Workspace workspace.Status.DevWorkspaceId = tt.Input.DevWorkspaceID workspace.Namespace = "test-namespace" @@ -162,7 +164,7 @@ 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", resource.MustParse("10Gi"), *testControllerCfg) if err != nil { t.Fatalf("Failure during setup: %s", err) } @@ -175,7 +177,8 @@ func TestTerminatingPVC(t *testing.T) { } testCase := loadTestCaseOrPanic(t, "testdata/common-storage/rewrites-volumes-for-common-pvc-strategy.yaml") assert.NotNil(t, testCase.Input.Workspace, "Input does not define workspace") - workspace := &dw.DevWorkspace{} + workspace := &common.DevWorkspaceWithConfig{} + workspace.Config = *testControllerCfg workspace.Spec.Template = *testCase.Input.Workspace workspace.Status.DevWorkspaceId = testCase.Input.DevWorkspaceID workspace.Namespace = "test-namespace" diff --git a/pkg/provision/storage/ephemeralStorage.go b/pkg/provision/storage/ephemeralStorage.go index e9e936007..abed5880b 100644 --- a/pkg/provision/storage/ephemeralStorage.go +++ b/pkg/provision/storage/ephemeralStorage.go @@ -17,10 +17,10 @@ package storage import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/provision/sync" - "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/library/container" + "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) // The EphemeralStorageProvisioner provisions all workspace storage as emptyDir volumes. @@ -35,8 +35,8 @@ func (e EphemeralStorageProvisioner) NeedsStorage(_ *dw.DevWorkspaceTemplateSpec return false } -func (e EphemeralStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *dw.DevWorkspace, _ sync.ClusterAPI) error { - persistent, ephemeral, projects := getWorkspaceVolumes(workspace) +func (e EphemeralStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, _ sync.ClusterAPI) error { + persistent, ephemeral, projects := getWorkspaceVolumes(&workspace.DevWorkspace) if _, err := addEphemeralVolumesToPodAdditions(podAdditions, persistent); err != nil { return err } @@ -59,6 +59,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..5ff3bbd36 100644 --- a/pkg/provision/storage/ephemeralStorage_test.go +++ b/pkg/provision/storage/ephemeralStorage_test.go @@ -18,7 +18,7 @@ package storage 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/stretchr/testify/assert" ) @@ -32,7 +32,8 @@ func TestRewriteContainerVolumeMountsForEphemeralStorageClass(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") - workspace := &dw.DevWorkspace{} + workspace := &common.DevWorkspaceWithConfig{} + workspace.Config = *testControllerCfg workspace.Spec.Template = *tt.Input.Workspace workspace.Status.DevWorkspaceId = tt.Input.DevWorkspaceID workspace.Namespace = "test-namespace" diff --git a/pkg/provision/storage/perWorkspaceStorage.go b/pkg/provision/storage/perWorkspaceStorage.go index db6af9a01..79fac25ad 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,9 +44,9 @@ 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 { + if err := addEphemeralVolumesFromWorkspace(&workspace.DevWorkspace, 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,12 @@ func syncPerWorkspacePVC(workspace *dw.DevWorkspace, clusterAPI sync.ClusterAPI) } } - pvc, err := getPVCSpec(common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId), workspace.Namespace, pvcSize) + pvc, err := getPVCSpec(common.PerWorkspacePVCName(workspace.Status.DevWorkspaceId), workspace.Namespace, pvcSize, workspace.Config) 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..e6f7e69ca 100644 --- a/pkg/provision/storage/perWorkspaceStorage_test.go +++ b/pkg/provision/storage/perWorkspaceStorage_test.go @@ -52,7 +52,8 @@ func TestRewriteContainerVolumeMountsForPerWorkspaceStorageClass(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { // sanity check that file is read correctly. assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") - workspace := &dw.DevWorkspace{} + workspace := &common.DevWorkspaceWithConfig{} + workspace.Config = *testControllerCfg workspace.Spec.Template = *tt.Input.Workspace workspace.Status.DevWorkspaceId = tt.Input.DevWorkspaceID workspace.Namespace = "test-namespace" diff --git a/pkg/provision/storage/provisioner.go b/pkg/provision/storage/provisioner.go index 71185a9c6..8ca5165ab 100644 --- a/pkg/provision/storage/provisioner.go +++ b/pkg/provision/storage/provisioner.go @@ -17,10 +17,10 @@ package storage import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/provision/sync" - "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) // Provisioner is an interface for rewriting volumeMounts in a pod according to a storage policy (e.g. common PVC for all mounts, etc.) @@ -29,14 +29,14 @@ 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 diff --git a/pkg/provision/storage/shared.go b/pkg/provision/storage/shared.go index 31427c056..3fadaa489 100644 --- a/pkg/provision/storage/shared.go +++ b/pkg/provision/storage/shared.go @@ -29,14 +29,14 @@ 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) { +// TODO: Maybe just pass in *string storageClassName? +func getPVCSpec(name, namespace string, size resource.Quantity, config v1alpha1.OperatorConfiguration) (*corev1.PersistentVolumeClaim, error) { return &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -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, clusterAPI sync.ClusterAPI, config v1alpha1.OperatorConfiguration) (*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, pvcSize, config) if err != nil { return nil, err } @@ -259,7 +259,7 @@ func getSharedPVCWorkspaceCount(namespace string, api sync.ClusterAPI) (total in return total, nil } -func checkPVCTerminating(name, namespace string, api sync.ClusterAPI) (bool, error) { +func checkPVCTerminating(name, namespace string, api sync.ClusterAPI, config v1alpha1.OperatorConfiguration) (bool, error) { if name == "" { name = config.Workspace.PVCName } diff --git a/pkg/provision/workspace/deployment.go b/pkg/provision/workspace/deployment.go index bb3a35a79..6706f726b 100644 --- a/pkg/provision/workspace/deployment.go +++ b/pkg/provision/workspace/deployment.go @@ -28,10 +28,10 @@ import ( "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" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -51,7 +51,7 @@ func SyncDeploymentToCluster( workspace *dw.DevWorkspace, podAdditions []v1alpha1.PodAdditions, saName string, - clusterAPI sync.ClusterAPI) DeploymentProvisioningStatus { + clusterAPI sync.ClusterAPI, config *controllerv1alpha1.OperatorConfiguration) DeploymentProvisioningStatus { podTolerations, nodeSelector, err := nsconfig.GetNamespacePodTolerationsAndNodeSelector(workspace.Namespace, clusterAPI) if err != nil { @@ -66,7 +66,7 @@ func SyncDeploymentToCluster( // [design] we have to pass components and routing pod additions separately because we need mountsources from each // component. - specDeployment, err := getSpecDeployment(workspace, podAdditions, saName, podTolerations, nodeSelector, clusterAPI.Scheme) + specDeployment, err := getSpecDeployment(workspace, podAdditions, saName, podTolerations, nodeSelector, clusterAPI.Scheme, *config) if err != nil { return DeploymentProvisioningStatus{ ProvisioningStatus{ @@ -118,7 +118,7 @@ func SyncDeploymentToCluster( failureMsg, checkErr := status.CheckPodsState(workspace.Status.DevWorkspaceId, workspace.Namespace, k8sclient.MatchingLabels{ constants.DevWorkspaceIDLabel: workspace.Status.DevWorkspaceId, - }, clusterAPI) + }, clusterAPI, config) if checkErr != nil { return DeploymentProvisioningStatus{ ProvisioningStatus: ProvisioningStatus{ @@ -172,7 +172,7 @@ func ScaleDeploymentToZero(ctx context.Context, workspace *dw.DevWorkspace, clie return nil } -func GetDevWorkspaceSecurityContext() *corev1.PodSecurityContext { +func GetDevWorkspaceSecurityContext(config controllerv1alpha1.OperatorConfiguration) *corev1.PodSecurityContext { if infrastructure.IsOpenShift() { return &corev1.PodSecurityContext{} } @@ -185,7 +185,7 @@ func getSpecDeployment( saName string, podTolerations []corev1.Toleration, nodeSelector map[string]string, - scheme *runtime.Scheme) (*appsv1.Deployment, error) { + scheme *runtime.Scheme, config controllerv1alpha1.OperatorConfiguration) (*appsv1.Deployment, error) { replicas := int32(1) terminationGracePeriod := int64(10) @@ -240,7 +240,7 @@ func getSpecDeployment( Volumes: podAdditions.Volumes, RestartPolicy: "Always", TerminationGracePeriodSeconds: &terminationGracePeriod, - SecurityContext: GetDevWorkspaceSecurityContext(), + SecurityContext: GetDevWorkspaceSecurityContext(config), ServiceAccountName: saName, AutomountServiceAccountToken: nil, }, @@ -261,7 +261,7 @@ func getSpecDeployment( } } - if needPVC, pvcName := needsPVCWorkaround(podAdditions); needPVC { + if needPVC, pvcName := needsPVCWorkaround(podAdditions, config); 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. @@ -368,7 +368,7 @@ func getWorkspaceSubpathVolumeMount(workspaceId, pvcName string) corev1.VolumeMo return workspaceVolumeMount } -func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions) (needs bool, pvcName string) { +func needsPVCWorkaround(podAdditions *v1alpha1.PodAdditions, config controllerv1alpha1.OperatorConfiguration) (needs bool, pvcName string) { commonPVCName := config.Workspace.PVCName for _, vol := range podAdditions.Volumes { if vol.Name == commonPVCName { diff --git a/pkg/provision/workspace/routing.go b/pkg/provision/workspace/routing.go index 212f7906f..fed240d06 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{} @@ -108,17 +106,21 @@ func getSpecRouting( } annotations = maputils.Append(annotations, constants.DevWorkspaceStartedStatusAnnotation, "true") + routingClass := workspace.Spec.RoutingClass + if routingClass == "" { + routingClass = workspace.Config.Routing.DefaultRoutingClass + } + // copy the annotations for the specific routingClass from the workspace object to the routing - expectedAnnotationPrefix := workspace.Spec.RoutingClass + constants.RoutingAnnotationInfix + expectedAnnotationPrefix := routingClass + constants.RoutingAnnotationInfix for k, v := range workspace.GetAnnotations() { if strings.HasPrefix(k, expectedAnnotationPrefix) { annotations = maputils.Append(annotations, k, v) } } - routingClass := workspace.Spec.RoutingClass - if routingClass == "" { - routingClass = config.Routing.DefaultRoutingClass + if v1alpha1.DevWorkspaceRoutingClass(routingClass) == v1alpha1.DevWorkspaceRoutingBasic { + annotations = maputils.Append(annotations, constants.ClusterHostSuffixAnnotation, workspace.Config.Routing.ClusterHostSuffix) } routing := &v1alpha1.DevWorkspaceRouting{ @@ -139,7 +141,7 @@ func getSpecRouting( }, }, } - err := controllerutil.SetControllerReference(workspace, routing, scheme) + err := controllerutil.SetControllerReference(&workspace.DevWorkspace, routing, scheme) if err != nil { return nil, err }