Skip to content

Commit 8fbbc1d

Browse files
cnvergencecnvergence
committed
requeue on not found Logical Cluster while shard is alive
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> On-behalf-of: @SAP karol.szwaj@sap.com
1 parent e8ac3ef commit 8fbbc1d

File tree

3 files changed

+102
-7
lines changed

3 files changed

+102
-7
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ func (c *Controller) reconcile(ctx context.Context, ws *tenancyv1alpha1.Workspac
108108
getLogicalCluster: func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error) {
109109
return c.kcpExternalClient.Cluster(cluster).CoreV1alpha1().LogicalClusters().Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{})
110110
},
111+
getShardByHash: getShardByName,
111112
requeueAfter: func(workspace *tenancyv1alpha1.Workspace, after time.Duration) {
112113
c.queue.AddAfter(kcpcache.ToClusterAwareKey(logicalcluster.From(workspace).String(), "", workspace.Name), after)
113114
},

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434

3535
type phaseReconciler struct {
3636
getLogicalCluster func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error)
37+
getShardByHash func(hash string) (*corev1alpha1.Shard, error)
3738

3839
requeueAfter func(workspace *tenancyv1alpha1.Workspace, after time.Duration)
3940
}
@@ -54,8 +55,24 @@ func (r *phaseReconciler) reconcile(ctx context.Context, workspace *tenancyv1alp
5455
if err != nil && !apierrors.IsNotFound(err) {
5556
return reconcileStatusStopAndRequeue, err
5657
} else if apierrors.IsNotFound(err) {
57-
logger.Info("LogicalCluster disappeared")
58-
conditions.MarkFalse(workspace, tenancyv1alpha1.WorkspaceInitialized, tenancyv1alpha1.WorkspaceInitializedWorkspaceDisappeared, conditionsv1alpha1.ConditionSeverityError, "LogicalCluster disappeared")
58+
// The LogicalCluster may not be visible through the front-proxy yet
59+
// because the shard index hasn't caught up.
60+
shardHash, hasShard := workspace.Annotations[WorkspaceShardHashAnnotationKey]
61+
if hasShard {
62+
shard, shardErr := r.getShardByHash(shardHash)
63+
if shardErr == nil && shard.DeletionTimestamp.IsZero() {
64+
if workspace.CreationTimestamp.IsZero() || time.Since(workspace.CreationTimestamp.Time) < 10*time.Second {
65+
logger.V(3).Info("LogicalCluster not found but shard is alive, requeueing", "shard", shard.Name)
66+
r.requeueAfter(workspace, 1*time.Second)
67+
return reconcileStatusContinue, nil
68+
}
69+
}
70+
}
71+
conditions.MarkFalse(workspace, tenancyv1alpha1.WorkspaceInitialized,
72+
tenancyv1alpha1.WorkspaceInitializedWorkspaceDisappeared,
73+
conditionsv1alpha1.ConditionSeverityError,
74+
"LogicalCluster %s not found", workspace.Spec.Cluster,
75+
)
5976
return reconcileStatusContinue, nil
6077
}
6178

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

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
corev1 "k8s.io/api/core/v1"
2727
apierrors "k8s.io/apimachinery/pkg/api/errors"
28+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829

2930
"github.com/kcp-dev/logicalcluster/v3"
3031
corev1alpha1 "github.com/kcp-dev/sdk/apis/core/v1alpha1"
@@ -37,10 +38,12 @@ func TestReconcilePhase(t *testing.T) {
3738
name string
3839
input *tenancyv1alpha1.Workspace
3940
getLogicalCluster func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error)
41+
getShardByHash func(hash string) (*corev1alpha1.Shard, error)
4042

4143
wantPhase corev1alpha1.LogicalClusterPhaseType
4244
wantStatus reconcileStatus
4345
wantCondition conditionsv1alpha1.Condition
46+
wantRequeue bool
4447
}{
4548
{
4649
name: "workspace is scheduling but not yet initialized",
@@ -71,8 +74,39 @@ func TestReconcilePhase(t *testing.T) {
7174
wantStatus: reconcileStatusContinue,
7275
},
7376
{
74-
name: "workspace is initializing and logical cluster is not found",
77+
name: "workspace is initializing and logical cluster is not found but shard is alive",
7578
input: &tenancyv1alpha1.Workspace{
79+
ObjectMeta: metav1.ObjectMeta{
80+
Annotations: map[string]string{
81+
WorkspaceShardHashAnnotationKey: "shard-hash-1",
82+
},
83+
},
84+
Spec: tenancyv1alpha1.WorkspaceSpec{
85+
URL: "http://example.com",
86+
Cluster: "cluster-1",
87+
},
88+
Status: tenancyv1alpha1.WorkspaceStatus{
89+
Phase: corev1alpha1.LogicalClusterPhaseInitializing,
90+
},
91+
},
92+
getLogicalCluster: func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error) {
93+
return nil, apierrors.NewNotFound(corev1alpha1.Resource("logicalcluster"), "cluster-1")
94+
},
95+
getShardByHash: func(hash string) (*corev1alpha1.Shard, error) {
96+
return &corev1alpha1.Shard{ObjectMeta: metav1.ObjectMeta{Name: "shard-1"}}, nil
97+
},
98+
wantPhase: corev1alpha1.LogicalClusterPhaseInitializing,
99+
wantStatus: reconcileStatusContinue,
100+
wantRequeue: true,
101+
},
102+
{
103+
name: "workspace is initializing and logical cluster is not found and shard is gone",
104+
input: &tenancyv1alpha1.Workspace{
105+
ObjectMeta: metav1.ObjectMeta{
106+
Annotations: map[string]string{
107+
WorkspaceShardHashAnnotationKey: "shard-hash-1",
108+
},
109+
},
76110
Spec: tenancyv1alpha1.WorkspaceSpec{
77111
URL: "http://example.com",
78112
Cluster: "cluster-1",
@@ -84,8 +118,42 @@ func TestReconcilePhase(t *testing.T) {
84118
getLogicalCluster: func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error) {
85119
return nil, apierrors.NewNotFound(corev1alpha1.Resource("logicalcluster"), "cluster-1")
86120
},
121+
getShardByHash: func(hash string) (*corev1alpha1.Shard, error) {
122+
return nil, apierrors.NewNotFound(corev1alpha1.Resource("shard"), "shard-hash-1")
123+
},
87124
wantPhase: corev1alpha1.LogicalClusterPhaseInitializing,
88125
wantStatus: reconcileStatusContinue,
126+
wantCondition: conditionsv1alpha1.Condition{
127+
Type: tenancyv1alpha1.WorkspaceInitialized,
128+
Status: corev1.ConditionFalse,
129+
Severity: conditionsv1alpha1.ConditionSeverityError,
130+
Reason: tenancyv1alpha1.WorkspaceInitializedWorkspaceDisappeared,
131+
Message: "LogicalCluster cluster-1 not found",
132+
},
133+
},
134+
{
135+
name: "workspace is initializing and logical cluster is not found and no shard annotation",
136+
input: &tenancyv1alpha1.Workspace{
137+
Spec: tenancyv1alpha1.WorkspaceSpec{
138+
URL: "http://example.com",
139+
Cluster: "cluster-1",
140+
},
141+
Status: tenancyv1alpha1.WorkspaceStatus{
142+
Phase: corev1alpha1.LogicalClusterPhaseInitializing,
143+
},
144+
},
145+
getLogicalCluster: func(ctx context.Context, cluster logicalcluster.Path) (*corev1alpha1.LogicalCluster, error) {
146+
return nil, apierrors.NewNotFound(corev1alpha1.Resource("logicalcluster"), "cluster-1")
147+
},
148+
wantPhase: corev1alpha1.LogicalClusterPhaseInitializing,
149+
wantStatus: reconcileStatusContinue,
150+
wantCondition: conditionsv1alpha1.Condition{
151+
Type: tenancyv1alpha1.WorkspaceInitialized,
152+
Status: corev1.ConditionFalse,
153+
Severity: conditionsv1alpha1.ConditionSeverityError,
154+
Reason: tenancyv1alpha1.WorkspaceInitializedWorkspaceDisappeared,
155+
Message: "LogicalCluster cluster-1 not found",
156+
},
89157
},
90158
{
91159
name: "workspace is initializing and logicalCluster not yet initialized",
@@ -107,8 +175,9 @@ func TestReconcilePhase(t *testing.T) {
107175
},
108176
}, nil
109177
},
110-
wantPhase: corev1alpha1.LogicalClusterPhaseInitializing,
111-
wantStatus: reconcileStatusContinue,
178+
wantPhase: corev1alpha1.LogicalClusterPhaseInitializing,
179+
wantStatus: reconcileStatusContinue,
180+
wantRequeue: true,
112181
},
113182
{
114183
name: "workspace is initializing and logicalCluster initialized",
@@ -235,19 +304,27 @@ func TestReconcilePhase(t *testing.T) {
235304
},
236305
} {
237306
t.Run(testCase.name, func(t *testing.T) {
307+
requeued := false
238308
reconciler := phaseReconciler{
239309
getLogicalCluster: testCase.getLogicalCluster,
240-
requeueAfter: func(workspace *tenancyv1alpha1.Workspace, after time.Duration) {},
310+
getShardByHash: testCase.getShardByHash,
311+
requeueAfter: func(workspace *tenancyv1alpha1.Workspace, after time.Duration) {
312+
requeued = true
313+
},
241314
}
242315
status, err := reconciler.reconcile(context.Background(), testCase.input)
243316

244317
require.NoError(t, err)
245318
require.Equal(t, testCase.wantStatus, status)
246319
require.Equal(t, testCase.wantPhase, testCase.input.Status.Phase)
320+
require.Equal(t, testCase.wantRequeue, requeued, "unexpected requeue state")
247321

248322
for _, condition := range testCase.input.Status.Conditions {
249323
if condition.Type == testCase.wantCondition.Type {
250-
require.Equal(t, testCase.wantCondition, condition)
324+
require.Equal(t, testCase.wantCondition.Status, condition.Status)
325+
require.Equal(t, testCase.wantCondition.Severity, condition.Severity)
326+
require.Equal(t, testCase.wantCondition.Reason, condition.Reason)
327+
require.Equal(t, testCase.wantCondition.Message, condition.Message)
251328
}
252329
}
253330
})

0 commit comments

Comments
 (0)