chore: retention mechanism for synchronized Kubernetes objects #2052

tolusha · 2025-10-09T14:30:38Z

What does this PR do?

This PR implements a retention mechanism for synchronized Kubernetes objects to prevent automatic deletion of PVCs and other resources in user namespaces when their source objects are removed. The implementation introduces a che.eclipse.org/sync-retain-on-delete annotation that allows objects to be retained in destination namespaces even when source objects are deleted.

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

eclipse-che/che#23569

How to test this PR?

Deploy the operator (OpenShift): chectl server:deploy -p openshift --che-operator-image quay.io/abazko/che-operator:23569
Log into Eclipse Che
Define a user namespace name USER_NAMEPSACE=<..>
Create PVC

oc apply -f - <<EOF
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: test
  namespace: eclipse-che
  labels:
    app.kubernetes.io/part-of: che.eclipse.org
    app.kubernetes.io/component: workspaces-config
spec:
  accessModes:
    - ReadWriteOnce
  volumeMode: Filesystem
  resources:
    requests:
      storage: 1Gi
EOF

Ensure that pvc is created in a user namespace a well

$ oc get pvc -n $USER_NAMESPACE test
NAME      STATUS    VOLUME   CAPACITY   ACCESS MODES   STORAGECLASS   VOLUMEATTRIBUTESCLASS   AGE
test   Pending

Delete pvc from eclipse-che namespace oc delete pvc -n eclipse-che test
Check that pvc still exists in the user namespace

$ oc get pvc -n $USER_NAMESPACE test
NAME      STATUS    VOLUME   CAPACITY   ACCESS MODES   STORAGECLASS   VOLUMEATTRIBUTESCLASS   AGE
test   Pending

Create a CM in eclipse-che namespace to sync into user namespace

oc apply -f - <<EOF
apiVersion: v1
kind: ConfigMap
metadata:
  name: test
  namespace:  eclipse-che
  annotations:
    che.eclipse.org/sync-retain-on-delete: "true"
  labels:
    app.kubernetes.io/part-of: che.eclipse.org
    app.kubernetes.io/component: workspaces-config
EOF

Check that CM exists in a user namespace

$ oc get configmaps -n $USER_NAMESPACE test
NAME   DATA   AGE
test   0      5s

Delete CM from eclipse-che namespace oc delete configmaps -n eclipse-che test
Check that CM still exists in a user namespace

$ oc get configmaps -n $USER_NAMESPACE test
NAME   DATA   AGE
test   0      5s

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Anatolii Bazko <[email protected]>

openshift-ci · 2025-10-09T14:30:42Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: Anatolii Bazko <[email protected]>

Copilot

Pull Request Overview

This PR implements a retention mechanism for synchronized Kubernetes objects to prevent automatic deletion of PVCs and other resources in user workspaces when their source objects are removed. The implementation introduces a che.eclipse.org/sync-retain annotation that allows objects to be retained in destination namespaces even when source objects are deleted.

Adds shouldRetain method to check retention annotation and default retention behavior
Implements defaultRetention() method across all Object2Sync types, with PVCs defaulting to true (retain by default)
Refactors object creation functions and improves error handling and logging

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
controllers/workspaceconfig/workspaces_config_controller.go	Core logic for retention checking and sync behavior modifications
controllers/workspaceconfig/pvc2sync.go	PVC-specific sync implementation with default retention enabled
controllers/workspaceconfig/configmap2sync.go	ConfigMap sync implementation with retention support
controllers/workspaceconfig/secret2sync.go	Secret sync implementation with retention support
controllers/workspaceconfig/unstructured2sync.go	Generic object sync with retention support
controllers/workspaceconfig/object2sync_factory.go	Factory method refactoring for object creation
controllers/workspaceconfig/pvc2sync_test.go	Test cases for PVC retention behavior
controllers/workspaceconfig/configmap2sync_test.go	Test cases for ConfigMap retention behavior
controllers/workspaceconfig/unstructured2sync_test.go	Test cases for unstructured object retention behavior

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

controllers/workspaceconfig/pvc2sync_test.go

Signed-off-by: Anatolii Bazko <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

controllers/workspaceconfig/object2sync_factory.go:1

Relying on obj.GetObjectKind().GroupVersionKind() can return an empty GVK for typed objects fetched via client/scheme, causing all objects to fall through to unstructured2Sync. This breaks defaultRetention (e.g., PVCs should default to true) and may mis-handle sync behavior. Use a type switch on the concrete type instead of GVK, or explicitly set the GVK before switching. For example: switch o := obj.(type) { case *corev1.ConfigMap: return &configMap2Sync{cm:o} ... }.

//

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-13T10:49:23Z

controllers/workspaceconfig/workspaces_config_controller.go

+// shouldRetain returns true if object in the destination namespace
+// should be retained if source one is deleted.
+func (r *WorkspacesConfigReconciler) shouldRetain(
+	ctx context.Context,
+	key client.ObjectKey,
+	gkv schema.GroupVersionKind,
+	clientWrapper *k8sclient.K8sClientWrapper,
+) (bool, error) {
+	blueprint, err := r.scheme.New(gkv)
+	if err != nil {
+		return false, err
+	}
+
+	exists, err := clientWrapper.GetIgnoreNotFound(ctx, key, blueprint.(client.Object))
+	if !exists {
+		return false, err
+	}
+
+	retainOnDelete := blueprint.(metav1.Object).GetAnnotations()[syncRetainOnDeleteAnnotation]
+	if retainOnDelete != "" {
+		return strconv.ParseBool(retainOnDelete)
+	}
+
+	obj2Sync := createObject2SyncFromObject(blueprint.(client.Object))
+	return obj2Sync.defaultRetention(), nil
+}


If gkv is not registered in the scheme (e.g., CRDs synced as unstructured), r.scheme.New(gkv) returns an error and aborts reconciliation, preventing cleanup of obsolete records. Instead, fall back to an unstructured.Unstructured with SetGroupVersionKind(gvk) so GetIgnoreNotFound can proceed and retention defaults can be applied; use meta.Accessor to read annotations from both typed and unstructured objects.

Copilot · 2025-10-13T10:49:23Z

controllers/workspaceconfig/workspaces_config_controller.go

+
+	retainOnDelete := blueprint.(metav1.Object).GetAnnotations()[syncRetainOnDeleteAnnotation]
+	if retainOnDelete != "" {
+		return strconv.ParseBool(retainOnDelete)


Parsing an invalid boolean value here will bubble an error up and fail the reconcile. Consider treating invalid values as default (false) and logging a warning, so a mis-typed annotation doesn't block reconciliation.

Suggested change

return strconv.ParseBool(retainOnDelete)

val, err := strconv.ParseBool(retainOnDelete)

if err != nil {

fmt.Fprintf(os.Stderr, "Warning: invalid value for annotation %q on %s: %q. Defaulting to false.\n", syncRetainOnDeleteAnnotation, key.String(), retainOnDelete)

return false, nil

}

return val, nil

Copilot · 2025-10-13T10:49:23Z

controllers/workspaceconfig/configmap2sync_test.go

+	assert.Nil(t, err)
+	assertSyncConfig(t, workspaceConfigReconciler, 0, v1ConfigMapGKV)
+
+	// Check that destination ConfigMap in a user namespace NOT is deleted


Grammar improvement.

Suggested change

// Check that destination ConfigMap in a user namespace NOT is deleted

// Check that destination ConfigMap in a user namespace is NOT deleted

openshift-ci · 2025-10-13T12:57:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, rohanKanojia, tolusha

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chore: do not remove synced users pvc

51b1bcb

Signed-off-by: Anatolii Bazko <[email protected]>

openshift-ci bot added the do-not-merge/work-in-progress label Oct 9, 2025

tolusha added 2 commits October 10, 2025 09:58

Fixup

025efbc

Signed-off-by: Anatolii Bazko <[email protected]>

fixup

b3cb1ae

Signed-off-by: Anatolii Bazko <[email protected]>

tolusha requested a review from Copilot October 10, 2025 10:28

Copilot AI reviewed Oct 10, 2025

View reviewed changes

controllers/workspaceconfig/pvc2sync_test.go Outdated Show resolved Hide resolved

controllers/workspaceconfig/pvc2sync_test.go Outdated Show resolved Hide resolved

controllers/workspaceconfig/pvc2sync_test.go Outdated Show resolved Hide resolved

tolusha changed the title ~~chore: do not remove synced users pvc~~ chore: retention mechanism for synchronized Kubernetes objects Oct 10, 2025

tolusha added 2 commits October 10, 2025 16:09

fixup

f8063d1

Signed-off-by: Anatolii Bazko <[email protected]>

fixup

0b40a5f

Signed-off-by: Anatolii Bazko <[email protected]>

tolusha marked this pull request as ready for review October 13, 2025 08:46

tolusha requested review from SDawley, dkwon17, ibuziuk and mkuznyetsov as code owners October 13, 2025 08:46

openshift-ci bot removed the do-not-merge/work-in-progress label Oct 13, 2025

tolusha requested review from akurinnoy, Copilot, rohanKanojia and vinokurig October 13, 2025 08:47

Copilot AI reviewed Oct 13, 2025

View reviewed changes

vinokurig self-assigned this Oct 13, 2025

akurinnoy approved these changes Oct 13, 2025

View reviewed changes

openshift-ci bot assigned akurinnoy Oct 13, 2025

openshift-ci bot added the lgtm label Oct 13, 2025

rohanKanojia approved these changes Oct 13, 2025

View reviewed changes

openshift-ci bot assigned rohanKanojia Oct 13, 2025

tolusha merged commit 19e9b35 into main Oct 15, 2025
21 checks passed

tolusha deleted the 23569 branch October 15, 2025 08:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: retention mechanism for synchronized Kubernetes objects #2052

chore: retention mechanism for synchronized Kubernetes objects #2052

Uh oh!

tolusha commented Oct 9, 2025 •

edited

Loading

Uh oh!

openshift-ci bot commented Oct 9, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 13, 2025

Uh oh!

Copilot AI Oct 13, 2025

Uh oh!

Copilot AI Oct 13, 2025

Uh oh!

openshift-ci bot commented Oct 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-		return strconv.ParseBool(retainOnDelete)
+		val, err := strconv.ParseBool(retainOnDelete)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Warning: invalid value for annotation %q on %s: %q. Defaulting to false.\n", syncRetainOnDeleteAnnotation, key.String(), retainOnDelete)
+			return false, nil
+		}
+		return val, nil

	// Check that destination ConfigMap in a user namespace NOT is deleted
	// Check that destination ConfigMap in a user namespace is NOT deleted

chore: retention mechanism for synchronized Kubernetes objects #2052

chore: retention mechanism for synchronized Kubernetes objects #2052

Uh oh!

Conversation

tolusha commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

Uh oh!

openshift-ci bot commented Oct 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tolusha commented Oct 9, 2025 •

edited

Loading