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