Skip to content

Allow external DWOC to be merged with internal DWOC on a per-workspace basis #918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
}
Expand Down
4 changes: 2 additions & 2 deletions controllers/workspace/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
65 changes: 40 additions & 25 deletions controllers/workspace/devworkspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -121,8 +122,19 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
// Error reading the object - requeue the request.
return reconcile.Result{}, err
}

config, err := wkspConfig.ResolveConfigForWorkspace(rawWorkspace, clusterAPI.Client)
if err != nil {
reqLogger.Error(err, "Error applying external DevWorkspace-Operator configuration")
config = wkspConfig.GetGlobalConfig()
}
configString := wkspConfig.GetCurrentConfigString(config)
workspace := &common.DevWorkspaceWithConfig{}
workspace.DevWorkspace = rawWorkspace
workspace.Config = config

reqLogger = reqLogger.WithValues(constants.DevWorkspaceIDLoggerKey, workspace.Status.DevWorkspaceId)
reqLogger.Info("Reconciling Workspace")
reqLogger.Info("Reconciling Workspace", "resolvedConfig", configString)

// Check if the DevWorkspaceRouting instance is marked to be deleted, which is
// indicated by the deletion timestamp being set.
Expand All @@ -137,10 +149,10 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
if err != nil {
workspace.Status.Phase = dw.DevWorkspaceStatusFailed
workspace.Status.Message = fmt.Sprintf("Failed to set DevWorkspace ID: %s", err.Error())
return reconcile.Result{}, r.Status().Update(ctx, workspace)
return reconcile.Result{}, r.Status().Update(ctx, workspace.DevWorkspace)
}
workspace.Status.DevWorkspaceId = workspaceId
err = r.Status().Update(ctx, workspace)
err = r.Status().Update(ctx, workspace.DevWorkspace)
return reconcile.Result{Requeue: true}, err
}

Expand All @@ -157,7 +169,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

patch := []byte(`{"spec":{"started": false}}`)
err := r.Client.Patch(context.Background(), workspace, client.RawPatch(types.MergePatchType, patch))
err := r.Client.Patch(context.Background(), workspace.DevWorkspace, client.RawPatch(types.MergePatchType, patch))
if err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -188,7 +200,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
Message: "DevWorkspace is starting",
},
}
err = r.Status().Update(ctx, workspace)
err = r.Status().Update(ctx, workspace.DevWorkspace)
if err == nil {
metrics.WorkspaceStarted(workspace, reqLogger)
}
Expand All @@ -198,7 +210,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
// Prepare handling workspace status and condition
reconcileStatus := currentStatus{phase: dw.DevWorkspaceStatusStarting}
reconcileStatus.setConditionTrue(conditions.Started, "DevWorkspace is starting")
clusterWorkspace := workspace.DeepCopy()
clusterWorkspace := &common.DevWorkspaceWithConfig{}
clusterWorkspace.DevWorkspace = workspace.DevWorkspace.DeepCopy()
clusterWorkspace.Config = workspace.Config
timingInfo := map[string]string{}
timing.SetTime(timingInfo, timing.DevWorkspaceStarted)

Expand Down Expand Up @@ -229,7 +243,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request

if _, ok := clusterWorkspace.Annotations[constants.DevWorkspaceStopReasonAnnotation]; ok {
delete(clusterWorkspace.Annotations, constants.DevWorkspaceStopReasonAnnotation)
err = r.Update(context.TODO(), clusterWorkspace)
err = r.Update(context.TODO(), clusterWorkspace.DevWorkspace)
return reconcile.Result{Requeue: true}, err
}

Expand Down Expand Up @@ -275,12 +289,12 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
// Note: we need to check the flattened workspace to see if a finalizer is needed, as plugins could require storage
if storageProvisioner.NeedsStorage(&workspace.Spec.Template) {
coputil.AddFinalizer(clusterWorkspace, constants.StorageCleanupFinalizer)
if err := r.Update(ctx, clusterWorkspace); err != nil {
if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil {
return reconcile.Result{}, err
}
}

devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(&workspace.Spec.Template)
devfilePodAdditions, err := containerlib.GetKubeContainersFromDevfile(&workspace.Spec.Template, workspace.Config.Workspace.ImagePullPolicy)
if err != nil {
return r.failWorkspace(workspace, fmt.Sprintf("Error processing devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus)
}
Expand All @@ -291,7 +305,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

// Add init container to clone projects
if projectClone, err := projects.GetProjectCloneInitContainer(&workspace.Spec.Template); err != nil {
if projectClone, err := projects.GetProjectCloneInitContainer(&workspace.Spec.Template, workspace.Config.Workspace.ImagePullPolicy); err != nil {
return r.failWorkspace(workspace, fmt.Sprintf("Failed to set up project-clone init container: %s", err), metrics.ReasonInfrastructureFailure, reqLogger, &reconcileStatus)
} else if projectClone != nil {
devfilePodAdditions.InitContainers = append(devfilePodAdditions.InitContainers, *projectClone)
Expand Down Expand Up @@ -405,7 +419,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
}
if wsprovision.NeedsServiceAccountFinalizer(&workspace.Spec.Template) {
coputil.AddFinalizer(clusterWorkspace, constants.ServiceAccountCleanupFinalizer)
if err := r.Update(ctx, clusterWorkspace); err != nil {
if err := r.Update(ctx, clusterWorkspace.DevWorkspace); err != nil {
return reconcile.Result{}, err
}
}
Expand Down Expand Up @@ -457,7 +471,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
return reconcile.Result{}, nil
}

func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *dw.DevWorkspace, logger logr.Logger) (reconcile.Result, error) {
func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *common.DevWorkspaceWithConfig, logger logr.Logger) (reconcile.Result, error) {
status := currentStatus{phase: dw.DevWorkspaceStatusStopping}
if workspace.Status.Phase == devworkspacePhaseFailing || workspace.Status.Phase == dw.DevWorkspaceStatusFailed {
status.phase = workspace.Status.Phase
Expand Down Expand Up @@ -488,7 +502,7 @@ func (r *DevWorkspaceReconciler) stopWorkspace(ctx context.Context, workspace *d
return r.updateWorkspaceStatus(workspace, logger, &status, reconcile.Result{}, nil)
}

func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWorkspace, logger logr.Logger) (stopped bool, err error) {
func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *common.DevWorkspaceWithConfig, logger logr.Logger) (stopped bool, err error) {
workspaceDeployment := &appsv1.Deployment{}
namespaceName := types.NamespacedName{
Name: common.DeploymentName(workspace.Status.DevWorkspaceId),
Expand Down Expand Up @@ -525,7 +539,7 @@ func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWo
}

// CleanupOnStop should never be nil as a default is always set
if config.Workspace.CleanupOnStop == nil || !*config.Workspace.CleanupOnStop {
if workspace.Config.Workspace.CleanupOnStop == nil || !*workspace.Config.Workspace.CleanupOnStop {
replicas := workspaceDeployment.Spec.Replicas
if replicas == nil || *replicas > 0 {
logger.Info("Stopping workspace")
Expand All @@ -547,7 +561,7 @@ func (r *DevWorkspaceReconciler) doStop(ctx context.Context, workspace *dw.DevWo
// failWorkspace marks a workspace as failed by setting relevant fields in the status struct.
// These changes are not synced to cluster immediately, and are intended to be synced to the cluster via a deferred function
// in the main reconcile loop. If needed, changes can be flushed to the cluster immediately via `updateWorkspaceStatus()`
func (r *DevWorkspaceReconciler) failWorkspace(workspace *dw.DevWorkspace, msg string, reason metrics.FailureReason, logger logr.Logger, status *currentStatus) (reconcile.Result, error) {
func (r *DevWorkspaceReconciler) failWorkspace(workspace *common.DevWorkspaceWithConfig, msg string, reason metrics.FailureReason, logger logr.Logger, status *currentStatus) (reconcile.Result, error) {
logger.Info("DevWorkspace failed to start: " + msg)
status.phase = devworkspacePhaseFailing
status.setConditionTrueWithReason(dw.DevWorkspaceFailedStart, msg, string(reason))
Expand All @@ -558,7 +572,7 @@ func (r *DevWorkspaceReconciler) failWorkspace(workspace *dw.DevWorkspace, msg s
}

func (r *DevWorkspaceReconciler) syncTimingToCluster(
ctx context.Context, workspace *dw.DevWorkspace, timingInfo map[string]string, reqLogger logr.Logger) {
ctx context.Context, workspace *common.DevWorkspaceWithConfig, timingInfo map[string]string, reqLogger logr.Logger) {
if timing.IsEnabled() {
if workspace.Annotations == nil {
workspace.Annotations = map[string]string{}
Expand All @@ -568,7 +582,7 @@ func (r *DevWorkspaceReconciler) syncTimingToCluster(
workspace.Annotations[timingEvent] = timestamp
}
}
if err := r.Update(ctx, workspace); err != nil {
if err := r.Update(ctx, workspace.DevWorkspace); err != nil {
if k8sErrors.IsConflict(err) {
reqLogger.Info("Got conflict when trying to apply timing annotations to workspace")
} else {
Expand All @@ -579,7 +593,7 @@ func (r *DevWorkspaceReconciler) syncTimingToCluster(
}

func (r *DevWorkspaceReconciler) syncStartedAtToCluster(
ctx context.Context, workspace *dw.DevWorkspace, reqLogger logr.Logger) {
ctx context.Context, workspace *common.DevWorkspaceWithConfig, reqLogger logr.Logger) {

if workspace.Annotations == nil {
workspace.Annotations = map[string]string{}
Expand All @@ -590,7 +604,7 @@ func (r *DevWorkspaceReconciler) syncStartedAtToCluster(
}

workspace.Annotations[constants.DevWorkspaceStartedAtAnnotation] = timing.CurrentTime()
if err := r.Update(ctx, workspace); err != nil {
if err := r.Update(ctx, workspace.DevWorkspace); err != nil {
if k8sErrors.IsConflict(err) {
reqLogger.Info("Got conflict when trying to apply started-at annotations to workspace")
} else {
Expand All @@ -600,12 +614,12 @@ func (r *DevWorkspaceReconciler) syncStartedAtToCluster(
}

func (r *DevWorkspaceReconciler) removeStartedAtFromCluster(
ctx context.Context, workspace *dw.DevWorkspace, reqLogger logr.Logger) {
ctx context.Context, workspace *common.DevWorkspaceWithConfig, reqLogger logr.Logger) {
if workspace.Annotations == nil {
workspace.Annotations = map[string]string{}
}
delete(workspace.Annotations, constants.DevWorkspaceStartedAtAnnotation)
if err := r.Update(ctx, workspace); err != nil {
if err := r.Update(ctx, workspace.DevWorkspace); err != nil {
if k8sErrors.IsConflict(err) {
reqLogger.Info("Got conflict when trying to apply timing annotations to workspace")
} else {
Expand All @@ -614,7 +628,7 @@ func (r *DevWorkspaceReconciler) removeStartedAtFromCluster(
}
}

func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace *dw.DevWorkspace) (string, error) {
func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace *common.DevWorkspaceWithConfig) (string, error) {
if idOverride := workspace.Annotations[constants.WorkspaceIdOverrideAnnotation]; idOverride != "" {
if len(idOverride) > 25 {
return "", fmt.Errorf("maximum length for DevWorkspace ID override is 25 characters")
Expand Down Expand Up @@ -676,8 +690,9 @@ func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Req
}
}

// TODO: Label PVCs used for workspace storage so that they can be cleaned up if non-default name is used.
// Otherwise, check if common PVC is deleted to make sure all DevWorkspaces see it happen
if obj.GetName() != config.Workspace.PVCName || obj.GetDeletionTimestamp() == nil {
if obj.GetName() != config.GetGlobalConfig().Workspace.PVCName || obj.GetDeletionTimestamp() == nil {
// We're looking for a deleted common PVC
return []reconcile.Request{}
}
Expand Down
15 changes: 8 additions & 7 deletions controllers/workspace/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -91,7 +92,7 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L
// Namespace is terminating, it's redundant to clean PVC files since it's going to be removed
log.Info("Namespace is terminating; clearing storage finalizer")
coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace)
}

storageProvisioner, err := storage.GetProvisioner(workspace)
Expand Down Expand Up @@ -126,10 +127,10 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L
}
log.Info("PVC clean up successful; clearing finalizer")
coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace)
}

func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace, finalizeStatus *currentStatus) (reconcile.Result, error) {
func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) {
retry, err := wsprovision.FinalizeServiceAccount(workspace, ctx, r.NonCachingClient)
if err != nil {
log.Error(err, "Failed to finalize workspace ServiceAccount")
Expand All @@ -142,7 +143,7 @@ func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log
}
log.Info("ServiceAccount clean up successful; clearing finalizer")
coputil.RemoveFinalizer(workspace, constants.ServiceAccountCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
return reconcile.Result{}, r.Update(ctx, workspace.DevWorkspace)
}

func (r *DevWorkspaceReconciler) namespaceIsTerminating(ctx context.Context, namespace string) (bool, error) {
Expand Down
10 changes: 6 additions & 4 deletions controllers/workspace/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion controllers/workspace/metrics/failure_reason.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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 {
Expand Down
Loading