Skip to content

feat: Allow external DWOC to be merged with internal DWOC on a per-workspace basis #909

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

Closed
wants to merge 14 commits into from
Closed
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 @@ -17,7 +17,6 @@ package solvers

import (
controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
"github.com/devfile/devworkspace-operator/pkg/config"
"github.com/devfile/devworkspace-operator/pkg/constants"
"github.com/devfile/devworkspace-operator/pkg/infrastructure"
)
Expand Down Expand Up @@ -57,7 +56,7 @@ func (s *BasicSolver) Finalize(*controllerv1alpha1.DevWorkspaceRouting) error {
func (s *BasicSolver) GetSpecObjects(routing *controllerv1alpha1.DevWorkspaceRouting, workspaceMeta DevWorkspaceMetadata) (RoutingObjects, error) {
routingObjects := RoutingObjects{}

routingSuffix := config.Routing.ClusterHostSuffix
routingSuffix := routing.Annotations[constants.ClusterHostSuffixAnnotation]
if routingSuffix == "" {
return routingObjects, &RoutingInvalid{"basic routing requires .config.routing.clusterHostSuffix to be set in operator config"}
}
Expand Down
88 changes: 50 additions & 38 deletions controllers/workspace/devworkspace_controller.go

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions controllers/workspace/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package controllers
import (
"context"

dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/devworkspace-operator/pkg/common"
"github.com/devfile/devworkspace-operator/pkg/conditions"
"github.com/devfile/devworkspace-operator/pkg/constants"

dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/devworkspace-operator/pkg/provision/sync"

"github.com/go-logr/logr"
Expand All @@ -46,7 +46,7 @@ func (r *DevWorkspaceReconciler) workspaceNeedsFinalize(workspace *dw.DevWorkspa
return false
}

func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace) (finalizeResult reconcile.Result, finalizeErr error) {
func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig) (finalizeResult reconcile.Result, finalizeErr error) {
// Tracked state for the finalize process; we update the workspace status in a deferred function (and pass the
// named return value for finalize()) to update the workspace's status with whatever is in finalizeStatus
// when this function returns.
Expand All @@ -68,15 +68,15 @@ func (r *DevWorkspaceReconciler) finalize(ctx context.Context, log logr.Logger,
case constants.StorageCleanupFinalizer:
return r.finalizeStorage(ctx, log, workspace, finalizeStatus)
case constants.ServiceAccountCleanupFinalizer:
return r.finalizeServiceAccount(ctx, log, workspace, finalizeStatus)
return r.finalizeServiceAccount(ctx, log, &workspace.DevWorkspace, finalizeStatus)
}
}
return reconcile.Result{}, nil
}

func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace, finalizeStatus *currentStatus) (reconcile.Result, error) {
func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.Logger, workspace *common.DevWorkspaceWithConfig, finalizeStatus *currentStatus) (reconcile.Result, error) {
// Need to make sure Deployment is cleaned up before starting job to avoid mounting issues for RWO PVCs
wait, err := wsprovision.DeleteWorkspaceDeployment(ctx, workspace, r.Client)
wait, err := wsprovision.DeleteWorkspaceDeployment(ctx, &workspace.DevWorkspace, r.Client)
if err != nil {
return reconcile.Result{}, err
}
Expand All @@ -91,10 +91,10 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L
// Namespace is terminating, it's redundant to clean PVC files since it's going to be removed
log.Info("Namespace is terminating; clearing storage finalizer")
coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
return reconcile.Result{}, r.Update(ctx, &workspace.DevWorkspace)
}

storageProvisioner, err := storage.GetProvisioner(workspace)
storageProvisioner, err := storage.GetProvisioner(&workspace.DevWorkspace)
if err != nil {
log.Error(err, "Failed to clean up DevWorkspace storage")
finalizeStatus.phase = dw.DevWorkspaceStatusError
Expand Down Expand Up @@ -126,7 +126,7 @@ func (r *DevWorkspaceReconciler) finalizeStorage(ctx context.Context, log logr.L
}
log.Info("PVC clean up successful; clearing finalizer")
coputil.RemoveFinalizer(workspace, constants.StorageCleanupFinalizer)
return reconcile.Result{}, r.Update(ctx, workspace)
return reconcile.Result{}, r.Update(ctx, &workspace.DevWorkspace)
}

func (r *DevWorkspaceReconciler) finalizeServiceAccount(ctx context.Context, log logr.Logger, workspace *dw.DevWorkspace, finalizeStatus *currentStatus) (reconcile.Result, error) {
Expand Down
12 changes: 8 additions & 4 deletions controllers/workspace/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,15 @@ func setupHttpClients() {
InsecureSkipVerify: true,
}

if config.Routing != nil && config.Routing.ProxyConfig != nil {
routingConfig := config.GetGlobalConfig().Routing

// Note: per-workspace routing settings (that have been set with an external DWOC)
// are ignored for the HTTP Clients
if routingConfig != nil && routingConfig.ProxyConfig != nil {
proxyConf := httpproxy.Config{
HTTPProxy: config.Routing.ProxyConfig.HttpProxy,
HTTPSProxy: config.Routing.ProxyConfig.HttpsProxy,
NoProxy: config.Routing.ProxyConfig.NoProxy,
HTTPProxy: routingConfig.ProxyConfig.HttpProxy,
HTTPSProxy: routingConfig.ProxyConfig.HttpsProxy,
NoProxy: routingConfig.ProxyConfig.NoProxy,
}
proxyFunc := func(req *http.Request) (*url.URL, error) {
return proxyConf.ProxyFunc()(req.URL)
Expand Down
14 changes: 7 additions & 7 deletions controllers/workspace/metrics/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import (
"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"

"github.com/devfile/devworkspace-operator/pkg/common"
"github.com/devfile/devworkspace-operator/pkg/conditions"
"github.com/devfile/devworkspace-operator/pkg/config"
"github.com/devfile/devworkspace-operator/pkg/constants"
)

// WorkspaceStarted updates metrics for workspaces entering the 'Starting' phase, given a workspace. If an error is
// encountered, the provided logger is used to log the error.
func WorkspaceStarted(wksp *dw.DevWorkspace, log logr.Logger) {
func WorkspaceStarted(wksp *common.DevWorkspaceWithConfig, log logr.Logger) {
_, ok := wksp.GetAnnotations()[constants.DevWorkspaceStartedAtAnnotation]
if !ok {
incrementMetricForWorkspace(workspaceTotal, wksp, log)
Expand All @@ -37,7 +37,7 @@ func WorkspaceStarted(wksp *dw.DevWorkspace, log logr.Logger) {
// WorkspaceRunning updates metrics for workspaces entering the 'Running' phase, given a workspace. If an error is
// encountered, the provided logger is used to log the error. This function assumes the provided workspace has
// fully-synced conditions (i.e. the WorkspaceReady condition is present).
func WorkspaceRunning(wksp *dw.DevWorkspace, log logr.Logger) {
func WorkspaceRunning(wksp *common.DevWorkspaceWithConfig, log logr.Logger) {
_, ok := wksp.GetAnnotations()[constants.DevWorkspaceStartedAtAnnotation]
if !ok {
incrementMetricForWorkspace(workspaceStarts, wksp, log)
Expand All @@ -51,14 +51,14 @@ func WorkspaceFailed(wksp *dw.DevWorkspace, log logr.Logger) {
incrementMetricForWorkspaceFailure(workspaceFailures, wksp, log)
}

func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *dw.DevWorkspace, log logr.Logger) {
func incrementMetricForWorkspace(metric *prometheus.CounterVec, wksp *common.DevWorkspaceWithConfig, log logr.Logger) {
sourceLabel := wksp.Labels[workspaceSourceLabel]
if sourceLabel == "" {
sourceLabel = "unknown"
}
routingClass := wksp.Spec.RoutingClass
if routingClass == "" {
routingClass = config.Routing.DefaultRoutingClass
routingClass = wksp.Config.Routing.DefaultRoutingClass
}
ctr, err := metric.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass})
if err != nil {
Expand All @@ -80,14 +80,14 @@ func incrementMetricForWorkspaceFailure(metric *prometheus.CounterVec, wksp *dw.
ctr.Inc()
}

func incrementStartTimeBucketForWorkspace(wksp *dw.DevWorkspace, log logr.Logger) {
func incrementStartTimeBucketForWorkspace(wksp *common.DevWorkspaceWithConfig, log logr.Logger) {
sourceLabel := wksp.Labels[workspaceSourceLabel]
if sourceLabel == "" {
sourceLabel = "unknown"
}
routingClass := wksp.Spec.RoutingClass
if routingClass == "" {
routingClass = config.Routing.DefaultRoutingClass
routingClass = wksp.Config.Routing.DefaultRoutingClass
}
hist, err := workspaceStartupTimesHist.GetMetricWith(map[string]string{metricSourceLabel: sourceLabel, metricsRoutingClassLabel: routingClass})
if err != nil {
Expand Down
19 changes: 9 additions & 10 deletions controllers/workspace/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (
"time"

dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/devworkspace-operator/pkg/common"
"github.com/devfile/devworkspace-operator/pkg/provision/sync"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -35,7 +35,6 @@ import (
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
"github.com/devfile/devworkspace-operator/controllers/workspace/metrics"
"github.com/devfile/devworkspace-operator/pkg/conditions"
"github.com/devfile/devworkspace-operator/pkg/config"
)

const (
Expand Down Expand Up @@ -64,20 +63,20 @@ var clock kubeclock.Clock = &kubeclock.RealClock{}
// updateWorkspaceStatus updates the current workspace's status field with conditions and phase from the passed in status.
// Parameters for result and error are returned unmodified, unless error is nil and another error is encountered while
// updating the status.
func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspace *dw.DevWorkspace, logger logr.Logger, status *currentStatus, reconcileResult reconcile.Result, reconcileError error) (reconcile.Result, error) {
func (r *DevWorkspaceReconciler) updateWorkspaceStatus(workspace *common.DevWorkspaceWithConfig, logger logr.Logger, status *currentStatus, reconcileResult reconcile.Result, reconcileError error) (reconcile.Result, error) {
syncConditions(&workspace.Status, status)
oldPhase := workspace.Status.Phase
workspace.Status.Phase = status.phase

infoMessage := getInfoMessage(workspace, status)
infoMessage := getInfoMessage(&workspace.DevWorkspace, status)
if warn := conditions.GetConditionByType(workspace.Status.Conditions, conditions.DevWorkspaceWarning); warn != nil && warn.Status == corev1.ConditionTrue {
infoMessage = fmt.Sprintf("%s %s", warningPresentInfoMessage, infoMessage)
}
if workspace.Status.Message != infoMessage {
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")
Expand Down Expand Up @@ -225,23 +224,23 @@ 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
}
switch newPhase {
case dw.DevWorkspaceStatusRunning:
metrics.WorkspaceRunning(workspace, logger)
case dw.DevWorkspaceStatusFailed:
metrics.WorkspaceFailed(workspace, logger)
metrics.WorkspaceFailed(&workspace.DevWorkspace, logger)
}
}

// checkForStartTimeout checks if the provided workspace has not progressed for longer than the configured
// startup timeout. This is determined by checking to see if the last condition transition time is more
// than [timeout] duration ago. Workspaces that are not in the "Starting" phase cannot timeout. Returns
// an error with message when timeout is reached.
func checkForStartTimeout(workspace *dw.DevWorkspace) error {
func checkForStartTimeout(workspace *dw.DevWorkspace, config v1alpha1.OperatorConfiguration) error {
if workspace.Status.Phase != dw.DevWorkspaceStatusStarting {
return nil
}
Expand All @@ -267,11 +266,11 @@ func checkForStartTimeout(workspace *dw.DevWorkspace) error {
// configured progress timeout. If the workspace is not in the Failing state or does not have a DevWorkspaceFailed
// condition set, returns false. Otherwise, returns true if the workspace has timed out. Returns an error if
// timeout is configured with an unparsable duration.
func checkForFailingTimeout(workspace *dw.DevWorkspace) (isTimedOut bool, err error) {
func checkForFailingTimeout(workspace *common.DevWorkspaceWithConfig) (isTimedOut bool, err error) {
if workspace.Status.Phase != devworkspacePhaseFailing {
return false, nil
}
timeout, err := time.ParseDuration(config.Workspace.ProgressTimeout)
timeout, err := time.ParseDuration(workspace.Config.Workspace.ProgressTimeout)
if err != nil {
return false, fmt.Errorf("invalid duration specified for timeout: %w", err)
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/common/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) 2019-2022 Red Hat, Inc.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package common

import (
dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
)

//TODO: Rename?
// TODO: Move
type DevWorkspaceWithConfig struct {
dw.DevWorkspace `json:",inline"`
Config controllerv1alpha1.OperatorConfiguration
}
10 changes: 10 additions & 0 deletions pkg/config/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,13 @@ func buildConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceO
Config: config,
}
}

func buildExternalConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceOperatorConfig {
return &v1alpha1.DevWorkspaceOperatorConfig{
ObjectMeta: metav1.ObjectMeta{
Name: ExternalConfigName,
Namespace: ExternalConfigNamespace,
},
Config: config,
}
}
Loading