Skip to content

Commit f53237d

Browse files
Copilotandyzhangx
andcommitted
Apply observedGeneration fix to Workspace and InferenceSet controllers
- Add observedGeneration check to UpdateStatusConditionIfNotMatch in workspace.go - Add observedGeneration check to UpdateStatusConditionIfNotMatch in inferenceset.go - Update workspace_test.go with observedGeneration in matching condition - Update inferenceset_test.go with observedGeneration in matching condition - Add test cases for stale observedGeneration scenario in both test files Fixes the same kubectl wait timeout issue for Workspace and InferenceSet resources. Co-authored-by: andyzhangx <[email protected]>
1 parent 389936b commit f53237d

File tree

4 files changed

+110
-10
lines changed

4 files changed

+110
-10
lines changed

pkg/utils/inferenceset/inferenceset.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import (
3737
func UpdateStatusConditionIfNotMatch(ctx context.Context, c client.Client, iObj *kaitov1alpha1.InferenceSet, cType kaitov1alpha1.ConditionType,
3838
cStatus metav1.ConditionStatus, cReason, cMessage string) error {
3939
if curCondition := meta.FindStatusCondition(iObj.Status.Conditions, string(cType)); curCondition != nil {
40-
if curCondition.Status == cStatus && curCondition.Reason == cReason && curCondition.Message == cMessage {
40+
if curCondition.Status == cStatus && curCondition.Reason == cReason && curCondition.Message == cMessage && curCondition.ObservedGeneration == iObj.GetGeneration() {
4141
// Nothing to change
4242
return nil
4343
}

pkg/utils/inferenceset/inferenceset_test.go

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
4646
Status: kaitov1alpha1.InferenceSetStatus{
4747
Conditions: []metav1.Condition{
4848
{
49-
Type: string(kaitov1beta1.ConditionTypeResourceStatus),
50-
Status: metav1.ConditionTrue,
51-
Reason: "ResourcesReady",
52-
Message: "All resources are ready",
49+
Type: string(kaitov1beta1.ConditionTypeResourceStatus),
50+
Status: metav1.ConditionTrue,
51+
Reason: "ResourcesReady",
52+
Message: "All resources are ready",
53+
ObservedGeneration: 1,
5354
},
5455
},
5556
},
@@ -275,6 +276,55 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
275276
assert.Contains(t, err.Error(), "get error")
276277
mockClient.AssertExpectations(t)
277278
})
279+
280+
t.Run("Should update when observedGeneration does not match current generation", func(t *testing.T) {
281+
mockClient := test.NewClient()
282+
283+
inferenceset := &kaitov1alpha1.InferenceSet{
284+
ObjectMeta: metav1.ObjectMeta{
285+
Name: "test-inferenceset",
286+
Namespace: "default",
287+
Generation: 4, // Current generation
288+
},
289+
Status: kaitov1alpha1.InferenceSetStatus{
290+
Conditions: []metav1.Condition{
291+
{
292+
Type: string(kaitov1alpha1.ConditionTypeResourceStatus),
293+
Status: metav1.ConditionTrue,
294+
Reason: "ResourcesReady",
295+
Message: "All resources are ready",
296+
ObservedGeneration: 1, // Stale generation
297+
},
298+
},
299+
},
300+
}
301+
302+
// Mock the Get call for UpdateInferenceSetStatus
303+
mockClient.On("Get", mock.IsType(context.Background()),
304+
client.ObjectKey{Name: "test-inferenceset", Namespace: "default"},
305+
mock.IsType(&kaitov1alpha1.InferenceSet{}), mock.Anything).Run(func(args mock.Arguments) {
306+
is := args.Get(2).(*kaitov1alpha1.InferenceSet)
307+
*is = *inferenceset
308+
}).Return(nil)
309+
310+
// Mock the Status().Update call
311+
mockClient.StatusMock.On("Update", mock.IsType(context.Background()),
312+
mock.IsType(&kaitov1alpha1.InferenceSet{}), mock.Anything).Run(func(args mock.Arguments) {
313+
is := args.Get(1).(*kaitov1alpha1.InferenceSet)
314+
// Verify the condition was updated with new observedGeneration
315+
condition := meta.FindStatusCondition(is.Status.Conditions, string(kaitov1alpha1.ConditionTypeResourceStatus))
316+
assert.NotNil(t, condition)
317+
assert.Equal(t, int64(4), condition.ObservedGeneration)
318+
}).Return(nil)
319+
320+
ctx := context.Background()
321+
err := UpdateStatusConditionIfNotMatch(ctx, mockClient, inferenceset,
322+
kaitov1alpha1.ConditionTypeResourceStatus, metav1.ConditionTrue, "ResourcesReady", "All resources are ready")
323+
324+
assert.NoError(t, err)
325+
mockClient.AssertExpectations(t)
326+
mockClient.StatusMock.AssertExpectations(t)
327+
})
278328
}
279329

280330
func TestUpdateInferenceSetStatus(t *testing.T) {

pkg/utils/workspace/workspace.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ var (
9393
func UpdateStatusConditionIfNotMatch(ctx context.Context, c client.Client, wObj *kaitov1beta1.Workspace, cType kaitov1beta1.ConditionType,
9494
cStatus metav1.ConditionStatus, cReason, cMessage string) error {
9595
if curCondition := meta.FindStatusCondition(wObj.Status.Conditions, string(cType)); curCondition != nil {
96-
if curCondition.Status == cStatus && curCondition.Reason == cReason && curCondition.Message == cMessage {
96+
if curCondition.Status == cStatus && curCondition.Reason == cReason && curCondition.Message == cMessage && curCondition.ObservedGeneration == wObj.GetGeneration() {
9797
// Nothing to change
9898
return nil
9999
}

pkg/utils/workspace/workspace_test.go

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
4343
Status: kaitov1beta1.WorkspaceStatus{
4444
Conditions: []metav1.Condition{
4545
{
46-
Type: string(kaitov1beta1.ConditionTypeResourceStatus),
47-
Status: metav1.ConditionTrue,
48-
Reason: "ResourcesReady",
49-
Message: "All resources are ready",
46+
Type: string(kaitov1beta1.ConditionTypeResourceStatus),
47+
Status: metav1.ConditionTrue,
48+
Reason: "ResourcesReady",
49+
Message: "All resources are ready",
50+
ObservedGeneration: 1,
5051
},
5152
},
5253
},
@@ -272,6 +273,55 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
272273
assert.Contains(t, err.Error(), "get error")
273274
mockClient.AssertExpectations(t)
274275
})
276+
277+
t.Run("Should update when observedGeneration does not match current generation", func(t *testing.T) {
278+
mockClient := test.NewClient()
279+
280+
workspace := &kaitov1beta1.Workspace{
281+
ObjectMeta: metav1.ObjectMeta{
282+
Name: "test-workspace",
283+
Namespace: "default",
284+
Generation: 4, // Current generation
285+
},
286+
Status: kaitov1beta1.WorkspaceStatus{
287+
Conditions: []metav1.Condition{
288+
{
289+
Type: string(kaitov1beta1.ConditionTypeResourceStatus),
290+
Status: metav1.ConditionTrue,
291+
Reason: "ResourcesReady",
292+
Message: "All resources are ready",
293+
ObservedGeneration: 1, // Stale generation
294+
},
295+
},
296+
},
297+
}
298+
299+
// Mock the Get call for UpdateWorkspaceStatus
300+
mockClient.On("Get", mock.IsType(context.Background()),
301+
client.ObjectKey{Name: "test-workspace", Namespace: "default"},
302+
mock.IsType(&kaitov1beta1.Workspace{}), mock.Anything).Run(func(args mock.Arguments) {
303+
ws := args.Get(2).(*kaitov1beta1.Workspace)
304+
*ws = *workspace
305+
}).Return(nil)
306+
307+
// Mock the Status().Update call
308+
mockClient.StatusMock.On("Update", mock.IsType(context.Background()),
309+
mock.IsType(&kaitov1beta1.Workspace{}), mock.Anything).Run(func(args mock.Arguments) {
310+
ws := args.Get(1).(*kaitov1beta1.Workspace)
311+
// Verify the condition was updated with new observedGeneration
312+
condition := meta.FindStatusCondition(ws.Status.Conditions, string(kaitov1beta1.ConditionTypeResourceStatus))
313+
assert.NotNil(t, condition)
314+
assert.Equal(t, int64(4), condition.ObservedGeneration)
315+
}).Return(nil)
316+
317+
ctx := context.Background()
318+
err := UpdateStatusConditionIfNotMatch(ctx, mockClient, workspace,
319+
kaitov1beta1.ConditionTypeResourceStatus, metav1.ConditionTrue, "ResourcesReady", "All resources are ready")
320+
321+
assert.NoError(t, err)
322+
mockClient.AssertExpectations(t)
323+
mockClient.StatusMock.AssertExpectations(t)
324+
})
275325
}
276326

277327
func TestUpdateWorkspaceStatus(t *testing.T) {

0 commit comments

Comments
 (0)