Skip to content

Commit

Permalink
feat: Use labels on PVCs for better reconciling
Browse files Browse the repository at this point in the history
Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
  • Loading branch information
mkuznyetsov committed May 16, 2024
1 parent 04f5369 commit 08f9082
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 11 deletions.
53 changes: 42 additions & 11 deletions controllers/workspace/eventhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package controllers

import (
"context"
"fmt"

dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
wkspConfig "github.com/devfile/devworkspace-operator/pkg/config"
Expand Down Expand Up @@ -47,7 +48,14 @@ func dwRelatedPodsHandler(obj client.Object) []reconcile.Request {
}

func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Request {
if obj.GetDeletionTimestamp() == nil {
// Do not reconcile unless PVC is being deleted.
return []reconcile.Request{}
}

// Check if PVC is owned by a DevWorkspace (per-workspace storage case)
// TODO: Ensure all new and existing PVC's get the `controller.devfile.io/devworkspace_pvc_type` label.
// See: https://github.com/devfile/devworkspace-operator/issues/1250
for _, ownerref := range obj.GetOwnerReferences() {
if ownerref.Kind != "DevWorkspace" {
continue
Expand All @@ -62,26 +70,49 @@ 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() != wkspConfig.GetGlobalConfig().Workspace.PVCName || obj.GetDeletionTimestamp() == nil {
// We're looking for a deleted common PVC
return []reconcile.Request{}
pvcLabel := obj.GetLabels()[constants.DevWorkspacePVCTypeLabel]

// TODO: This check is for legacy reasons as existing PVCs might not have the `controller.devfile.io/devworkspace_pvc_type` label.
// Remove once https://github.com/devfile/devworkspace-operator/issues/1250 is resolved
if pvcLabel == "" {
if obj.GetName() != wkspConfig.GetGlobalConfig().Workspace.PVCName {
// No need to reconcile if PVC doesn't have a PVC type label
// and it doesn't have a name of PVC from global config.
return []reconcile.Request{}
}
}

dwList := &dw.DevWorkspaceList{}
if err := r.Client.List(context.Background(), dwList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil {
return []reconcile.Request{}
}
var reconciles []reconcile.Request

for _, workspace := range dwList.Items {
storageType := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil)
if storageType == constants.CommonStorageClassType || storageType == constants.PerUserStorageClassType || storageType == "" {
reconciles = append(reconciles, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: workspace.GetName(),
Namespace: workspace.GetNamespace(),
},
})

// Determine workspaces to reconcile that use the current common PVC.
// Workspaces can either use the common PVC where the PVC name
// is coming from the global config, or from an external config the workspace might use
workspacePVCName := wkspConfig.GetGlobalConfig().Workspace.PVCName

if workspace.Spec.Template.Attributes.Exists(constants.ExternalDevWorkspaceConfiguration) {
externalConfig, err := wkspConfig.ResolveConfigForWorkspace(&workspace, r.Client)
if err != nil {
r.Log.Info(fmt.Sprintf("Couldn't resolve PVC name for workspace '%s' in namespace '%s', using PVC name '%s' from global config instead: %s.", workspace.Name, workspace.Namespace, workspacePVCName, err.Error()))
} else {
workspacePVCName = externalConfig.Workspace.PVCName
}
}
if obj.GetName() == workspacePVCName {
reconciles = append(reconciles, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: workspace.GetName(),
Namespace: workspace.GetNamespace(),
},
})
}
}
}
return reconciles
Expand Down
3 changes: 3 additions & 0 deletions pkg/constants/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ const (
// DevWorkspaceIDLabel is the label key to store workspace identifier
DevWorkspaceIDLabel = "controller.devfile.io/devworkspace_id"

// DevWorkspacePVCTypeLabel is the label key to identify PVCs used by DevWorkspaces and indicate their storage strategy.
DevWorkspacePVCTypeLabel = "controller.devfile.io/devworkspace_pvc_type"

// WorkspaceIdOverrideAnnotation is an annotation that can be applied to DevWorkspaces
// to override the default DevWorkspace ID assigned by the Operator. Is only respected
// when a DevWorkspace is created. Once a DevWorkspace has an ID set, it cannot be changed.
Expand Down
1 change: 1 addition & 0 deletions pkg/provision/storage/perWorkspaceStorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ func syncPerWorkspacePVC(workspace *common.DevWorkspaceWithConfig, clusterAPI sy
pvc.Labels = map[string]string{}
}
pvc.Labels[constants.DevWorkspaceIDLabel] = workspace.Status.DevWorkspaceId
pvc.Labels[constants.DevWorkspacePVCTypeLabel] = constants.PerWorkspaceStorageClassType

if err := controllerutil.SetControllerReference(workspace.DevWorkspace, pvc, clusterAPI.Scheme); err != nil {
return nil, err
Expand Down
4 changes: 4 additions & 0 deletions pkg/provision/storage/shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ func syncCommonPVC(namespace string, config *v1alpha1.OperatorConfiguration, clu
if err != nil {
return nil, err
}
if pvc.Labels == nil {
pvc.Labels = map[string]string{}
}
pvc.Labels[constants.DevWorkspacePVCTypeLabel] = constants.PerUserStorageClassType
currObject, err := sync.SyncObjectWithCluster(pvc, clusterAPI)
switch t := err.(type) {
case nil:
Expand Down

0 comments on commit 08f9082

Please sign in to comment.