Skip to content
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

feat: Use labels on PVCs for better reconciling #1233

Merged
merged 1 commit into from
May 16, 2024
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
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{}
AObuchow marked this conversation as resolved.
Show resolved Hide resolved
}

// Check if PVC is owned by a DevWorkspace (per-workspace storage case)
AObuchow marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading