Skip to content

Commit 710cbd4

Browse files
thesuperzapperthesuperzapper
authored
feat(ws): refactor backend models and repositories (#192)
Signed-off-by: Mathew Wicks <[email protected]>
1 parent 16f97f8 commit 710cbd4

25 files changed

+1736
-914
lines changed

‎workspaces/backend/api/app.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,20 @@ func (a *App) Routes() http.Handler {
7777
router.NotFound = http.HandlerFunc(a.notFoundResponse)
7878
router.MethodNotAllowed = http.HandlerFunc(a.methodNotAllowedResponse)
7979

80-
router.GET(HealthCheckPath, a.HealthcheckHandler)
80+
// healthcheck
81+
router.GET(HealthCheckPath, a.GetHealthcheckHandler)
82+
83+
// namespaces
8184
router.GET(AllNamespacesPath, a.GetNamespacesHandler)
8285

86+
// workspaces
8387
router.GET(AllWorkspacesPath, a.GetWorkspacesHandler)
8488
router.GET(WorkspacesByNamespacePath, a.GetWorkspacesHandler)
85-
8689
router.GET(WorkspacesByNamePath, a.GetWorkspaceHandler)
8790
router.POST(WorkspacesByNamespacePath, a.CreateWorkspaceHandler)
8891
router.DELETE(WorkspacesByNamePath, a.DeleteWorkspaceHandler)
8992

93+
// workspacekinds
9094
router.GET(AllWorkspaceKindsPath, a.GetWorkspaceKindsHandler)
9195
router.GET(WorkspaceKindsByNamePath, a.GetWorkspaceKindHandler)
9296

‎workspaces/backend/api/healthcheck__handler_test.go

-70
This file was deleted.

‎workspaces/backend/api/healthcheck_handler.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"github.com/julienschmidt/httprouter"
2323
)
2424

25-
func (a *App) HealthcheckHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
25+
func (a *App) GetHealthcheckHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
2626

2727
healthCheck, err := a.repositories.HealthCheck.HealthCheck(Version)
2828
if err != nil {
@@ -34,5 +34,4 @@ func (a *App) HealthcheckHandler(w http.ResponseWriter, r *http.Request, ps http
3434
if err != nil {
3535
a.serverErrorResponse(w, r, err)
3636
}
37-
3837
}

‎workspaces/backend/api/healthcheck_handler_test.go

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
Copyright 2024.
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 api
18+
19+
import (
20+
"encoding/json"
21+
"io"
22+
"net/http"
23+
"net/http/httptest"
24+
25+
"github.com/julienschmidt/httprouter"
26+
. "github.com/onsi/ginkgo/v2"
27+
. "github.com/onsi/gomega"
28+
29+
"github.com/kubeflow/notebooks/workspaces/backend/internal/config"
30+
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/health_check"
31+
"github.com/kubeflow/notebooks/workspaces/backend/internal/repositories"
32+
)
33+
34+
var _ = Describe("HealthCheck Handler", func() {
35+
var (
36+
a App
37+
)
38+
39+
Context("when backend is healthy", func() {
40+
41+
BeforeEach(func() {
42+
repos := repositories.NewRepositories(k8sClient)
43+
a = App{
44+
Config: config.EnvConfig{
45+
Port: 4000,
46+
},
47+
repositories: repos,
48+
}
49+
})
50+
51+
It("should return a health check response", func() {
52+
By("creating the HTTP request")
53+
req, err := http.NewRequest(http.MethodGet, HealthCheckPath, http.NoBody)
54+
Expect(err).NotTo(HaveOccurred())
55+
56+
By("executing GetHealthCheckHandler")
57+
ps := httprouter.Params{}
58+
rr := httptest.NewRecorder()
59+
a.GetHealthcheckHandler(rr, req, ps)
60+
rs := rr.Result()
61+
defer rs.Body.Close()
62+
63+
By("verifying the HTTP response status code")
64+
Expect(rs.StatusCode).To(Equal(http.StatusOK))
65+
66+
By("reading the HTTP response body")
67+
body, err := io.ReadAll(rs.Body)
68+
Expect(err).NotTo(HaveOccurred())
69+
70+
By("unmarshalling the response JSON to HealthCheck")
71+
var response models.HealthCheck
72+
err = json.Unmarshal(body, &response)
73+
Expect(err).NotTo(HaveOccurred())
74+
75+
By("ensuring that the health check is as expected")
76+
expected := models.HealthCheck{
77+
Status: models.ServiceStatusHealthy,
78+
SystemInfo: models.SystemInfo{
79+
Version: Version,
80+
},
81+
}
82+
Expect(response).To(BeComparableTo(expected))
83+
})
84+
})
85+
})

‎workspaces/backend/api/namespaces_handler.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ import (
2121

2222
"github.com/julienschmidt/httprouter"
2323

24-
"github.com/kubeflow/notebooks/workspaces/backend/internal/models"
24+
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/namespaces"
2525
)
2626

27-
type NamespacesEnvelope Envelope[[]models.NamespaceModel]
27+
type NamespacesEnvelope Envelope[[]models.Namespace]
2828

2929
func (a *App) GetNamespacesHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
3030

‎workspaces/backend/api/namespaces_handler_test.go

+41-37
Original file line numberDiff line numberDiff line change
@@ -22,70 +22,66 @@ import (
2222
"net/http"
2323
"net/http/httptest"
2424

25-
corev1 "k8s.io/api/core/v1"
26-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27-
2825
"github.com/julienschmidt/httprouter"
2926
. "github.com/onsi/ginkgo/v2"
3027
. "github.com/onsi/gomega"
28+
corev1 "k8s.io/api/core/v1"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/types"
3131

3232
"github.com/kubeflow/notebooks/workspaces/backend/internal/config"
33-
"github.com/kubeflow/notebooks/workspaces/backend/internal/models"
33+
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/namespaces"
3434
"github.com/kubeflow/notebooks/workspaces/backend/internal/repositories"
3535
)
3636

3737
var _ = Describe("Namespaces Handler", func() {
3838
var (
39-
a App
40-
testRouter *httprouter.Router
39+
a App
4140
)
4241

43-
BeforeEach(func() {
44-
repos := repositories.NewRepositories(k8sClient)
45-
a = App{
46-
Config: config.EnvConfig{
47-
Port: 4000,
48-
},
49-
repositories: repos,
50-
}
51-
52-
testRouter = httprouter.New()
53-
testRouter.GET("/api/namespaces", a.GetNamespacesHandler)
54-
})
42+
// NOTE: these tests assume a specific state of the cluster, so cannot be run in parallel with other tests.
43+
// therefore, we run them using the `Serial` Ginkgo decorators.
44+
Context("when namespaces exist", Serial, func() {
5545

56-
Context("when namespaces exist", func() {
57-
const namespaceName1 = "namespaceone"
58-
const namespaceName2 = "namespacetwo"
46+
const namespaceName1 = "get-ns-test-ns1"
47+
const namespaceName2 = "get-ns-test-ns2"
5948

6049
BeforeEach(func() {
61-
By("creating namespaces")
50+
repos := repositories.NewRepositories(k8sClient)
51+
a = App{
52+
Config: config.EnvConfig{
53+
Port: 4000,
54+
},
55+
repositories: repos,
56+
}
57+
58+
By("creating Namespace 1")
6259
namespace1 := &corev1.Namespace{
6360
ObjectMeta: metav1.ObjectMeta{
6461
Name: namespaceName1,
6562
},
6663
}
6764
Expect(k8sClient.Create(ctx, namespace1)).To(Succeed())
6865

66+
By("creating Namespace 2")
6967
namespace2 := &corev1.Namespace{
7068
ObjectMeta: metav1.ObjectMeta{
7169
Name: namespaceName2,
7270
},
7371
}
7472
Expect(k8sClient.Create(ctx, namespace2)).To(Succeed())
75-
7673
})
7774

7875
AfterEach(func() {
79-
By("deleting namespaces")
80-
By("deleting the namespace1")
76+
By("deleting Namespace 1")
8177
namespace1 := &corev1.Namespace{
8278
ObjectMeta: metav1.ObjectMeta{
8379
Name: namespaceName1,
8480
},
8581
}
8682
Expect(k8sClient.Delete(ctx, namespace1)).To(Succeed())
8783

88-
By("deleting the namespace2")
84+
By("deleting Namespace 2")
8985
namespace2 := &corev1.Namespace{
9086
ObjectMeta: metav1.ObjectMeta{
9187
Name: namespaceName2,
@@ -96,32 +92,40 @@ var _ = Describe("Namespaces Handler", func() {
9692

9793
It("should retrieve all namespaces successfully", func() {
9894
By("creating the HTTP request")
99-
req, err := http.NewRequest(http.MethodGet, "/api/namespaces", http.NoBody)
100-
Expect(err).NotTo(HaveOccurred(), "Failed to create HTTP request")
95+
req, err := http.NewRequest(http.MethodGet, AllNamespacesPath, http.NoBody)
96+
Expect(err).NotTo(HaveOccurred())
10197

10298
By("executing GetNamespacesHandler")
99+
ps := httprouter.Params{}
103100
rr := httptest.NewRecorder()
104-
testRouter.ServeHTTP(rr, req)
101+
a.GetNamespacesHandler(rr, req, ps)
105102
rs := rr.Result()
106103
defer rs.Body.Close()
107104

108105
By("verifying the HTTP response status code")
109-
Expect(rs.StatusCode).To(Equal(http.StatusOK), "Expected HTTP status 200 OK")
106+
Expect(rs.StatusCode).To(Equal(http.StatusOK))
110107

111108
By("reading the HTTP response body")
112109
body, err := io.ReadAll(rs.Body)
113-
Expect(err).NotTo(HaveOccurred(), "Failed to read HTTP response body")
110+
Expect(err).NotTo(HaveOccurred())
114111

115-
By("unmarshalling the response JSON")
112+
By("unmarshalling the response JSON to NamespacesEnvelope")
116113
var response NamespacesEnvelope
117114
err = json.Unmarshal(body, &response)
118-
Expect(err).NotTo(HaveOccurred(), "Error unmarshalling response JSON")
115+
Expect(err).NotTo(HaveOccurred())
116+
117+
By("getting the Namespaces from the Kubernetes API")
118+
namespace1 := &corev1.Namespace{}
119+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: namespaceName1}, namespace1)).To(Succeed())
120+
namespace2 := &corev1.Namespace{}
121+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: namespaceName2}, namespace2)).To(Succeed())
119122

120-
By("asserting that the created namespaces are in the response")
123+
By("ensuring the response contains the expected Namespaces")
124+
// NOTE: we use `ContainElements` instead of `ConsistOf` because envtest creates some namespaces by default
121125
Expect(response.Data).To(ContainElements(
122-
models.NamespaceModel{Name: namespaceName1},
123-
models.NamespaceModel{Name: namespaceName2},
124-
), "Expected created namespaces to be in the response")
126+
models.NewNamespaceModelFromNamespace(namespace1),
127+
models.NewNamespaceModelFromNamespace(namespace2),
128+
))
125129
})
126130
})
127131
})

‎workspaces/backend/api/workspacekinds_handler.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ import (
2323

2424
"github.com/julienschmidt/httprouter"
2525

26-
"github.com/kubeflow/notebooks/workspaces/backend/internal/models"
27-
"github.com/kubeflow/notebooks/workspaces/backend/internal/repositories"
26+
models "github.com/kubeflow/notebooks/workspaces/backend/internal/models/workspacekinds"
27+
repository "github.com/kubeflow/notebooks/workspaces/backend/internal/repositories/workspacekinds"
2828
)
2929

30-
type WorkspaceKindsEnvelope Envelope[[]models.WorkspaceKindModel]
31-
type WorkspaceKindEnvelope Envelope[models.WorkspaceKindModel]
30+
type WorkspaceKindsEnvelope Envelope[[]models.WorkspaceKind]
31+
32+
type WorkspaceKindEnvelope Envelope[models.WorkspaceKind]
3233

3334
func (a *App) GetWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
3435
name := ps.ByName("name")
@@ -40,7 +41,7 @@ func (a *App) GetWorkspaceKindHandler(w http.ResponseWriter, r *http.Request, ps
4041

4142
workspaceKind, err := a.repositories.WorkspaceKind.GetWorkspaceKind(r.Context(), name)
4243
if err != nil {
43-
if errors.Is(err, repositories.ErrWorkspaceKindNotFound) {
44+
if errors.Is(err, repository.ErrWorkspaceKindNotFound) {
4445
a.notFoundResponse(w, r)
4546
return
4647
}

0 commit comments

Comments
 (0)