Skip to content

Commit

Permalink
consolidate constants for workspace and ragengine controller
Browse files Browse the repository at this point in the history
Signed-off-by: Bangqi Zhu <[email protected]>
  • Loading branch information
Bangqi Zhu committed Oct 16, 2024
1 parent 1d99028 commit 51124c3
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 50 deletions.
3 changes: 2 additions & 1 deletion pkg/controllers/ragengine_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"

kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1"
"github.com/azure/kaito/pkg/utils/consts"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
Expand All @@ -29,5 +30,5 @@ func (c *RAGEngineReconciler) updateStatusConditionIfNotMatch(ctx context.Contex
ObservedGeneration: ragObj.GetGeneration(),
Message: cMessage,
}
return updateObjStatus(ctx, c.Client, &client.ObjectKey{Name: ragObj.Name, Namespace: ragObj.Namespace}, "ragengine", &cObj, nil)
return updateObjStatus(ctx, c.Client, &client.ObjectKey{Name: ragObj.Name, Namespace: ragObj.Namespace}, consts.RAGEngineString, &cObj, nil)
}
9 changes: 5 additions & 4 deletions pkg/controllers/ragengine_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1"
"github.com/azure/kaito/pkg/utils/consts"
"github.com/azure/kaito/pkg/utils/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -38,7 +39,7 @@ func TestUpdateRAGEngineStatus(t *testing.T) {
mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.RAGEngine{}), mock.Anything).Return(nil)
mockClient.StatusMock.On("Update", mock.IsType(context.Background()), mock.IsType(&kaitov1alpha1.RAGEngine{}), mock.Anything).Return(nil)

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, "ragengine", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, consts.RAGEngineString, &condition, workerNodes)
assert.Nil(t, err)
})

Expand All @@ -60,7 +61,7 @@ func TestUpdateRAGEngineStatus(t *testing.T) {

mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.RAGEngine{}), mock.Anything).Return(errors.New("Get operation failed"))

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, "ragengine", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, consts.RAGEngineString, &condition, workerNodes)
assert.NotNil(t, err)
})

Expand All @@ -80,9 +81,9 @@ func TestUpdateRAGEngineStatus(t *testing.T) {
}
workerNodes := []string{"node1", "node2"}

mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.RAGEngine{}), mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{}, "ragengine"))
mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.RAGEngine{}), mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{}, consts.RAGEngineString))

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, "ragengine", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: ragengine.Name, Namespace: ragengine.Namespace}, consts.RAGEngineString, &condition, workerNodes)
assert.Nil(t, err)
})
}
Expand Down
64 changes: 32 additions & 32 deletions pkg/controllers/workspace_controller.go

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions pkg/controllers/workspace_gc_finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

// garbageCollectWorkspace remove finalizer associated with workspace object.
func (c *WorkspaceReconciler) garbageCollectWorkspace(ctx context.Context, wObj *kaitov1alpha1.Workspace) (ctrl.Result, error) {
klog.InfoS("garbageCollectWorkspace", "workspace", klog.KObj(wObj))
klog.InfoS("garbageCollectWorkspace", consts.WorkspaceString, klog.KObj(wObj))

// Check if there are any machines associated with this workspace.
mList, err := machine.ListMachines(ctx, wObj, c.Client)
Expand Down Expand Up @@ -54,11 +54,11 @@ func (c *WorkspaceReconciler) garbageCollectWorkspace(ctx context.Context, wObj
staleWObj.SetFinalizers(nil)
if updateErr := c.Update(ctx, staleWObj, &client.UpdateOptions{}); updateErr != nil {
klog.ErrorS(updateErr, "failed to remove the finalizer from the workspace",
"workspace", klog.KObj(wObj), "workspace", klog.KObj(staleWObj))
consts.WorkspaceString, klog.KObj(wObj), consts.WorkspaceString, klog.KObj(staleWObj))
return ctrl.Result{}, updateErr
}
klog.InfoS("successfully removed the workspace finalizers",
"workspace", klog.KObj(wObj))
consts.WorkspaceString, klog.KObj(wObj))
controllerutil.RemoveFinalizer(wObj, consts.WorkspaceFinalizer)
return ctrl.Result{}, nil
}
7 changes: 4 additions & 3 deletions pkg/controllers/workspace_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sort"

kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1"
"github.com/azure/kaito/pkg/utils/consts"
"github.com/samber/lo"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -31,12 +32,12 @@ func updateObjStatus(ctx context.Context, c client.Client, name *client.ObjectKe
var conditions *[]metav1.Condition
var workerNodesField *[]string
switch objType {
case "workspace":
case consts.WorkspaceString:
ragObj := &kaitov1alpha1.Workspace{}
obj = ragObj
conditions = &ragObj.Status.Conditions
workerNodesField = &ragObj.Status.WorkerNodes
case "ragengine":
case consts.RAGEngineString:
wObj := &kaitov1alpha1.RAGEngine{}
obj = wObj
conditions = &wObj.Status.Conditions
Expand Down Expand Up @@ -91,5 +92,5 @@ func (c *WorkspaceReconciler) updateStatusNodeListIfNotMatch(ctx context.Context
return nil
}
klog.InfoS("updateStatusNodeList", "workspace", klog.KObj(wObj))
return updateObjStatus(ctx, c.Client, &client.ObjectKey{Name: wObj.Name, Namespace: wObj.Namespace}, "workspace", nil, nodeNameList)
return updateObjStatus(ctx, c.Client, &client.ObjectKey{Name: wObj.Name, Namespace: wObj.Namespace}, consts.WorkspaceString, nil, nodeNameList)
}
15 changes: 8 additions & 7 deletions pkg/controllers/workspace_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

kaitov1alpha1 "github.com/azure/kaito/api/v1alpha1"
"github.com/azure/kaito/pkg/utils/consts"
"github.com/azure/kaito/pkg/utils/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -38,7 +39,7 @@ func TestUpdateWorkspaceStatus(t *testing.T) {
mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(nil)
mockClient.StatusMock.On("Update", mock.IsType(context.Background()), mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(nil)

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, "workspace", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, consts.WorkspaceString, &condition, workerNodes)
assert.Nil(t, err)
})

Expand All @@ -60,7 +61,7 @@ func TestUpdateWorkspaceStatus(t *testing.T) {

mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(errors.New("Get operation failed"))

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, "workspace", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, consts.WorkspaceString, &condition, workerNodes)
assert.NotNil(t, err)
})

Expand All @@ -80,9 +81,9 @@ func TestUpdateWorkspaceStatus(t *testing.T) {
}
workerNodes := []string{"node1", "node2"}

mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{}, "workspace"))
mockClient.On("Get", mock.IsType(context.Background()), mock.Anything, mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(apierrors.NewNotFound(schema.GroupResource{}, consts.WorkspaceString))

err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, "workspace", &condition, workerNodes)
err := updateObjStatus(ctx, reconciler.Client, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace}, consts.WorkspaceString, &condition, workerNodes)
assert.Nil(t, err)
})
}
Expand Down Expand Up @@ -111,7 +112,7 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
}

err := updateStatusConditionIfNotMatch(ctx, workspace, reconciler, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace},
workspace.Status, conditionType, conditionStatus, "workspace", conditionReason, conditionMessage)
workspace.Status, conditionType, conditionStatus, consts.WorkspaceString, conditionReason, conditionMessage)
assert.Nil(t, err)
})

Expand Down Expand Up @@ -140,7 +141,7 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
mockClient.StatusMock.On("Update", mock.IsType(context.Background()), mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(nil)

err := updateStatusConditionIfNotMatch(ctx, workspace, reconciler, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace},
workspace.Status, conditionType, conditionStatus, "workspace", conditionReason, conditionMessage)
workspace.Status, conditionType, conditionStatus, consts.WorkspaceString, conditionReason, conditionMessage)
assert.Nil(t, err)
})

Expand Down Expand Up @@ -169,7 +170,7 @@ func TestUpdateStatusConditionIfNotMatch(t *testing.T) {
mockClient.StatusMock.On("Update", mock.IsType(context.Background()), mock.IsType(&kaitov1alpha1.Workspace{}), mock.Anything).Return(nil)

err := updateStatusConditionIfNotMatch(ctx, workspace, reconciler, &client.ObjectKey{Name: workspace.Name, Namespace: workspace.Namespace},
workspace.Status, conditionType, conditionStatus, "workspace", conditionReason, conditionMessage)
workspace.Status, conditionType, conditionStatus, consts.WorkspaceString, conditionReason, conditionMessage)
assert.Nil(t, err)
})
}
5 changes: 5 additions & 0 deletions pkg/utils/consts/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,9 @@ const (
MaxRevisionHistoryLimit = 10
GiBToBytes = 1024 * 1024 * 1024 // Conversion factor from GiB to bytes
NvidiaGPU = "nvidia.com/gpu"
WorkspaceString = "workspace"
RAGEngineString = "ragengine"
WorkspaceFailed = "workspaceFailed"
WorkspaceSucceeded = "workspaceSucceeded"
WorkspaceResourceStatusFailed = "workspaceResourceStatusFailed"
)

0 comments on commit 51124c3

Please sign in to comment.