Skip to content

Commit 7acf553

Browse files
SimonTheLegSimonTheLeg
committed
move terminators from metadata.finalizers to status.terminators
This also introduces a new logicalcluster terminators finalizer reconciler, which is responsible for setting a finalizer on logicalclusters if they have terminators, so they don't get deleted prematurely. On-behalf-of: SAP <[email protected]> Signed-off-by: Simon Bein <[email protected]>
1 parent 90e67a9 commit 7acf553

File tree

18 files changed

+294
-249
lines changed

18 files changed

+294
-249
lines changed

‎config/crds/core.kcp.io_logicalclusters.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ spec:
208208
- Ready
209209
- Unavailable
210210
type: string
211-
terminator:
211+
terminators:
212212
description: |-
213213
Terminators are set on creation by the system and must be cleared
214214
by a controller before the logical cluster can be deleted. The LogicalCluster object

‎config/root-phase0/apiresourceschema-logicalclusters.core.kcp.io.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
22
kind: APIResourceSchema
33
metadata:
44
creationTimestamp: null
5-
name: v251015-1d163d0e5.logicalclusters.core.kcp.io
5+
name: v251016-6e24e0d49.logicalclusters.core.kcp.io
66
spec:
77
group: core.kcp.io
88
names:
@@ -206,7 +206,7 @@ spec:
206206
- Ready
207207
- Unavailable
208208
type: string
209-
terminator:
209+
terminators:
210210
description: |-
211211
Terminators are set on creation by the system and must be cleared
212212
by a controller before the logical cluster can be deleted. The LogicalCluster object

‎pkg/openapi/zz_generated.openapi.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/reconciler/core/logicalcluster/logicalcluster_reconcile.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,11 @@ type reconciler interface {
3636
}
3737

3838
func (c *Controller) reconcile(ctx context.Context, logicalCluster *corev1alpha1.LogicalCluster) (bool, error) {
39+
// reconcilers which modify Status should be last
40+
// reconcilers which modify ObjectMeta, need to return reconcileStatusStopAndRequeue on change
3941
reconcilers := []reconciler{
4042
&metaDataReconciler{},
43+
&terminatorReconciler{},
4144
&phaseReconciler{},
4245
&urlReconciler{shardExternalURL: c.shardExternalURL},
4346
}

‎pkg/reconciler/core/logicalcluster/logicalcluster_reconcile_terminator.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
Copyright 2025 The KCP Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package logicalcluster
18+
19+
import (
20+
"context"
21+
22+
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
23+
)
24+
25+
const LogicalClusterHasTerminatorFinalizer = "kcp.io/has-terminators"
26+
27+
// terminatorReconciler will place the LogicalClusterHasTerminatorFinalizer finalizer on the LogicalCluster
28+
// in order to prevent a logicalcluster from being deleted while it still has terminators set.
29+
type terminatorReconciler struct{}
30+
31+
func (r *terminatorReconciler) reconcile(ctx context.Context, logicalCluster *corev1alpha1.LogicalCluster) (reconcileStatus, error) {
32+
var changed bool
33+
// if there are still terminators, ensure that the finalizer is present
34+
if len(logicalCluster.Status.Terminators) != 0 {
35+
logicalCluster.Finalizers, changed = addUnique(logicalCluster.Finalizers, LogicalClusterHasTerminatorFinalizer)
36+
// if not make sure that it is removed
37+
} else {
38+
logicalCluster.Finalizers, changed = removeByValue(logicalCluster.Finalizers, LogicalClusterHasTerminatorFinalizer)
39+
}
40+
41+
if changed {
42+
// first update ObjectMeta before other reconcilers change status
43+
return reconcileStatusStopAndRequeue, nil
44+
}
45+
46+
return reconcileStatusContinue, nil
47+
}
48+
49+
// addUnique adds t to s if not already present, returning the new slice and whether it was added.
50+
func addUnique[T comparable](s []T, t T) ([]T, bool) {
51+
for _, elem := range s {
52+
if elem == t {
53+
return s, false
54+
}
55+
}
56+
return append(s, t), true
57+
}
58+
59+
// removeByValue removes t from s if present, returning the new slice and whether it was removed.
60+
func removeByValue[T comparable](s []T, t T) ([]T, bool) {
61+
for i, other := range s {
62+
if other == t {
63+
return append(s[:i], s[i+1:]...), true
64+
}
65+
}
66+
return s, false
67+
}

‎pkg/reconciler/core/logicalcluster/logicalcluster_reconcile_terminator_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
Copyright 2025 The KCP Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package logicalcluster
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
24+
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
27+
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
28+
)
29+
30+
func TestReconcile(t *testing.T) {
31+
tests := []struct {
32+
name string
33+
logicalCluster *corev1alpha1.LogicalCluster
34+
expectedFinalizers []string
35+
}{
36+
{
37+
name: "remove finalizer if there are no terminators",
38+
logicalCluster: &corev1alpha1.LogicalCluster{
39+
Status: corev1alpha1.LogicalClusterStatus{
40+
Terminators: []corev1alpha1.LogicalClusterTerminator{},
41+
},
42+
ObjectMeta: metav1.ObjectMeta{
43+
Finalizers: []string{LogicalClusterHasTerminatorFinalizer},
44+
},
45+
},
46+
expectedFinalizers: []string{},
47+
},
48+
{
49+
name: "has terminators, finalizer added",
50+
logicalCluster: &corev1alpha1.LogicalCluster{
51+
Status: corev1alpha1.LogicalClusterStatus{
52+
Terminators: []corev1alpha1.LogicalClusterTerminator{"terminator1"},
53+
},
54+
ObjectMeta: metav1.ObjectMeta{
55+
Finalizers: []string{},
56+
},
57+
},
58+
expectedFinalizers: []string{LogicalClusterHasTerminatorFinalizer},
59+
},
60+
{
61+
name: "do nothing if finalizers already present and terminators exist",
62+
logicalCluster: &corev1alpha1.LogicalCluster{
63+
Status: corev1alpha1.LogicalClusterStatus{
64+
Terminators: []corev1alpha1.LogicalClusterTerminator{"terminator1"},
65+
},
66+
ObjectMeta: metav1.ObjectMeta{
67+
Finalizers: []string{LogicalClusterHasTerminatorFinalizer},
68+
},
69+
},
70+
expectedFinalizers: []string{LogicalClusterHasTerminatorFinalizer},
71+
},
72+
{
73+
name: "do nothing if finalizers already absent and no terminators exist",
74+
logicalCluster: &corev1alpha1.LogicalCluster{
75+
Status: corev1alpha1.LogicalClusterStatus{
76+
Terminators: []corev1alpha1.LogicalClusterTerminator{},
77+
},
78+
ObjectMeta: metav1.ObjectMeta{
79+
Finalizers: []string{},
80+
},
81+
},
82+
expectedFinalizers: []string{},
83+
},
84+
}
85+
86+
for _, tt := range tests {
87+
t.Run(tt.name, func(t *testing.T) {
88+
reconciler := &terminatorReconciler{}
89+
_, err := reconciler.reconcile(context.Background(), tt.logicalCluster)
90+
if err != nil {
91+
t.Errorf("unexpected reconcile error %v", err)
92+
}
93+
if diff := cmp.Diff(tt.expectedFinalizers, tt.logicalCluster.Finalizers); diff != "" {
94+
t.Errorf("unexpected finalizer diff:\n%s", diff)
95+
}
96+
})
97+
}
98+
}

‎pkg/reconciler/tenancy/workspace/workspace_reconcile_scheduling.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,10 @@ func (r *schedulingReconciler) createLogicalCluster(ctx context.Context, shard *
321321
}
322322

323323
// add terminators
324-
terminators, err := LogicalClusterTerminators(r.transitiveTypeResolver, r.getWorkspaceType, logicalcluster.NewPath(workspace.Spec.Type.Path), string(workspace.Spec.Type.Name))
324+
logicalCluster.Spec.Terminators, err = LogicalClusterTerminators(r.transitiveTypeResolver, r.getWorkspaceType, logicalcluster.NewPath(workspace.Spec.Type.Path), string(workspace.Spec.Type.Name))
325325
if err != nil {
326326
return err
327327
}
328-
logicalCluster.Spec.Terminators = terminators
329-
// append our terminators to already existing ObjectMeta finalizers
330-
logicalCluster.ObjectMeta.Finalizers = termination.MergeTerminatorsUnique(terminators, logicalCluster.ObjectMeta.Finalizers)
331328

332329
logicalClusterAdminClient, err := r.kcpLogicalClusterAdminClientFor(shard)
333330
if err != nil {

‎pkg/reconciler/tenancy/workspacetype/workspacetype_controller_reconcile_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,12 @@ func TestReconcile(t *testing.T) {
138138
},
139139
Status: tenancyv1alpha1.WorkspaceTypeStatus{
140140
VirtualWorkspaces: []tenancyv1alpha1.VirtualWorkspace{
141-
{URL: "https://item.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
142141
{URL: "https://item.com/services/initializingworkspaces/root:org:team:ws:sometype"},
143-
{URL: "https://something.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
142+
{URL: "https://item.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
144143
{URL: "https://something.com/services/initializingworkspaces/root:org:team:ws:sometype"},
145-
{URL: "https://whatever.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
144+
{URL: "https://something.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
146145
{URL: "https://whatever.com/services/initializingworkspaces/root:org:team:ws:sometype"},
146+
{URL: "https://whatever.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
147147
},
148148
Conditions: conditionsv1alpha1.Conditions{
149149
{
@@ -197,12 +197,12 @@ func TestReconcile(t *testing.T) {
197197
},
198198
Status: tenancyv1alpha1.WorkspaceTypeStatus{
199199
VirtualWorkspaces: []tenancyv1alpha1.VirtualWorkspace{
200-
{URL: "https://item.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
201200
{URL: "https://item.com/services/initializingworkspaces/root:org:team:ws:sometype"},
202-
{URL: "https://something.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
201+
{URL: "https://item.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
203202
{URL: "https://something.com/services/initializingworkspaces/root:org:team:ws:sometype"},
204-
{URL: "https://whatever.com/services/finalizingworkspaces/root:org:team:ws:sometype"},
203+
{URL: "https://something.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
205204
{URL: "https://whatever.com/services/initializingworkspaces/root:org:team:ws:sometype"},
205+
{URL: "https://whatever.com/services/terminatingworkspaces/root:org:team:ws:sometype"},
206206
},
207207
Conditions: conditionsv1alpha1.Conditions{
208208
{

‎pkg/virtual/framework/internalapis/fixtures/workspaces.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ spec:
204204
description: Phase of the workspace (Scheduling, Initializing, Ready).
205205
type: string
206206
terminators:
207-
description: finalizers must be cleared by a controller before the workspace
208-
is being deleted.
207+
description: terminators must be cleared by a controller before the
208+
workspace is being deleted.
209209
items:
210210
type: string
211211
type: array

‎pkg/virtual/terminatingworkspaces/builder/build.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func BuildVirtualWorkspace(
148148
dynamicClusterClient: dynamicClusterClient,
149149
exposeSubresources: true,
150150
resource: &logicalClusterResource,
151-
storageProvider: filteredLogicalClusterReadWriteRestStorage,
151+
storageProvider: filteredLogicalClusterStatusWriteOnly,
152152
}, nil
153153
},
154154
}

0 commit comments

Comments
 (0)