Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bff): Use envtest for kubernetes testing instead of hardcoded mock #490

Merged
merged 3 commits into from
Nov 1, 2024
Merged

feat(bff): Use envtest for kubernetes testing instead of hardcoded mock #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
270e309
feat(bff): Use envtest for kubernetes testing instead of hardcoded mock
ederign Oct 18, 2024
cf89f36
feat(bff): using ginkgo and gomega as our test suite
ederign Oct 18, 2024
d03d2a3
PR review from Griffin
ederign Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions clients/ui/bff/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ IMG ?= model-registry-bff:latest
PORT ?= 4000
MOCK_K8S_CLIENT ?= false
MOCK_MR_CLIENT ?= false
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
ENVTEST_K8S_VERSION = 1.29.0

.PHONY: all
all: build
Expand Down Expand Up @@ -32,15 +34,17 @@ vet: .
go vet ./...

.PHONY: test
test:
test: fmt vet envtest
ENVTEST_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \
go test ./...

.PHONY: build
build: fmt vet test
go build -o bin/bff cmd/main.go

.PHONY: run
run: fmt vet
run: fmt vet envtest
ENVTEST_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \
go run ./cmd/main.go --port=$(PORT) --mock-k8s-client=$(MOCK_K8S_CLIENT) --mock-mr-client=$(MOCK_MR_CLIENT)

.PHONY: docker-build
Expand All @@ -54,8 +58,18 @@ LOCALBIN ?= $(shell pwd)/bin
$(LOCALBIN):
mkdir -p $(LOCALBIN)

## Tool Binaries
ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION)
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)

## Tool Versions
GOLANGCI_LINT_VERSION ?= v1.57.2
ENVTEST_VERSION ?= release-0.17

.PHONY: envtest
envtest: $(ENVTEST) ## Download setup-envtest locally if necessary.
$(ENVTEST): $(LOCALBIN)
$(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest,$(ENVTEST_VERSION))
Comment on lines +70 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the run command try to do this if the mock-k8s-client option is passed to the makefile?

If you try to run make run MOCK_K8S_CLIENT=true MOCK_MR_CLIENT=true before the binary is installed you get:

time=2024-10-21T15:23:49.306-04:00 level=ERROR msg="failed to start test environment" error="unable to start control plane itself: failed to start the controlplane. retried 5 times: fork/exec /home/gsulliva/Programming/model-registry/clients/ui/bff/bin/k8s/1.29.0-linux-amd64/etcd: no such file or directory"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Fixed!


.PHONY: golangci-lint
golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
Expand All @@ -75,4 +89,4 @@ echo "Downloading $${package}" ;\
GOBIN=$(LOCALBIN) go install $${package} ;\
mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\
}
endef
endef
8 changes: 8 additions & 0 deletions clients/ui/bff/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ require (
github.com/brianvoe/gofakeit/v7 v7.0.4
github.com/julienschmidt/httprouter v1.3.0
github.com/kubeflow/model-registry v0.2.9
github.com/onsi/ginkgo/v2 v2.19.0
github.com/onsi/gomega v1.33.1
github.com/stretchr/testify v1.9.0
k8s.io/api v0.31.1
k8s.io/apimachinery v0.31.1
Expand All @@ -22,15 +24,18 @@ require (
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/fxamacker/cbor/v2 v2.7.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/zapr v1.3.0 // indirect
github.com/go-openapi/jsonpointer v0.19.6 // indirect
github.com/go-openapi/jsonreference v0.20.2 // indirect
github.com/go-openapi/swag v0.22.4 // indirect
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/imdario/mergo v0.3.6 // indirect
github.com/josharian/intern v1.0.0 // indirect
Expand All @@ -48,13 +53,16 @@ require (
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/x448/float16 v0.8.4 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.26.0 // indirect
golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect
golang.org/x/net v0.28.0 // indirect
golang.org/x/oauth2 v0.22.0 // indirect
golang.org/x/sys v0.24.0 // indirect
golang.org/x/term v0.23.0 // indirect
golang.org/x/text v0.17.0 // indirect
golang.org/x/time v0.5.0 // indirect
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
google.golang.org/protobuf v1.35.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
Expand Down
3 changes: 2 additions & 1 deletion clients/ui/bff/internal/api/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ func NewApp(cfg config.EnvConfig, logger *slog.Logger) (*App, error) {
var err error
if cfg.MockK8Client {
//mock all k8s calls
k8sClient, err = mocks.NewKubernetesClient(logger)
ctx, cancel := context.WithCancel(context.Background())
k8sClient, err = mocks.NewKubernetesClient(logger, ctx, cancel)
} else {
k8sClient, err = integrations.NewKubernetesClient(logger)
}
Expand Down
82 changes: 42 additions & 40 deletions clients/ui/bff/internal/api/model_registry_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,52 @@ package api

import (
"encoding/json"
"github.com/kubeflow/model-registry/ui/bff/internal/mocks"
"github.com/kubeflow/model-registry/ui/bff/internal/models"
"github.com/kubeflow/model-registry/ui/bff/internal/repositories"
"github.com/stretchr/testify/assert"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"io"
"net/http"
"net/http/httptest"
"testing"
)

func TestModelRegistryHandler(t *testing.T) {
mockK8sClient, _ := mocks.NewKubernetesClient(nil)
mockMRClient, _ := mocks.NewModelRegistryClient(nil)

testApp := App{
kubernetesClient: mockK8sClient,
repositories: repositories.NewRepositories(mockMRClient),
}

req, err := http.NewRequest(http.MethodGet, ModelRegistryListPath, nil)
assert.NoError(t, err)

rr := httptest.NewRecorder()

testApp.ModelRegistryHandler(rr, req, nil)
rs := rr.Result()

defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
assert.NoError(t, err)
var actual ModelRegistryListEnvelope
err = json.Unmarshal(body, &actual)
assert.NoError(t, err)

assert.Equal(t, http.StatusOK, rr.Code)

var expected = ModelRegistryListEnvelope{
Data: []models.ModelRegistryModel{
{Name: "model-registry", Description: "Model registry description", DisplayName: "Model Registry"},
{Name: "model-registry-dora", Description: "Model registry dora description", DisplayName: "Model Registry Dora"},
{Name: "model-registry-bella", Description: "Model registry bella description", DisplayName: "Model Registry Bella"},
},
}

assert.Equal(t, expected, actual)

}
var _ = Describe("TestModelRegistryHandler", func() {
Context("fetching model registries", Ordered, func() {
It("should retrieve the model registries successfully", func() {

By("creating the test app")
testApp := App{
kubernetesClient: k8sClient,
repositories: repositories.NewRepositories(mockMRClient),
logger: logger,
}

By("creating the http test infrastructure")
req, err := http.NewRequest(http.MethodGet, ModelRegistryListPath, nil)
Expect(err).NotTo(HaveOccurred())
rr := httptest.NewRecorder()

By("creating the http request for the handler")
testApp.ModelRegistryHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the model registries")
var actual ModelRegistryListEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("should match the expected model registries")
var expected = []models.ModelRegistryModel{
{Name: "model-registry", Description: "Model Registry Description", DisplayName: "Model Registry"},
{Name: "model-registry-bella", Description: "Model Registry Bella description", DisplayName: "Model Registry Bella"},
{Name: "model-registry-dora", Description: "Model Registry Dora description", DisplayName: "Model Registry Dora"},
}
Expect(actual.Data).To(ConsistOf(expected))
})

})
})
158 changes: 84 additions & 74 deletions clients/ui/bff/internal/api/model_versions_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,80 +3,90 @@ package api
import (
"github.com/kubeflow/model-registry/pkg/openapi"
"github.com/kubeflow/model-registry/ui/bff/internal/mocks"
"github.com/stretchr/testify/assert"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"net/http"
"testing"
)

func TestGetModelVersionHandler(t *testing.T) {
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}

actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodGet, "/api/v1/model_registry/model-registry/model_versions/1", nil)
assert.NoError(t, err)

assert.Equal(t, http.StatusOK, rs.StatusCode)
assert.Equal(t, expected.Data.Name, actual.Data.Name)
}

func TestCreateModelVersionHandler(t *testing.T) {
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}

body := ModelVersionEnvelope{Data: openapi.NewModelVersion("Model One", "1")}

actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodPost, "/api/v1/model_registry/model-registry/model_versions", body)
assert.NoError(t, err)

assert.Equal(t, http.StatusCreated, rs.StatusCode)
assert.Equal(t, expected.Data.Name, actual.Data.Name)
assert.Equal(t, rs.Header.Get("Location"), "/api/v1/model_registry/model-registry/model_versions/1")
}

func TestUpdateModelVersionHandler(t *testing.T) {
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}

reqData := openapi.ModelVersionUpdate{
Description: openapi.PtrString("New description"),
}
body := ModelVersionUpdateEnvelope{Data: &reqData}

actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodPatch, "/api/v1/model_registry/model-registry/model_versions/1", body)
assert.NoError(t, err)

assert.Equal(t, http.StatusOK, rs.StatusCode)
assert.Equal(t, expected.Data.Name, actual.Data.Name)
}

func TestGetAllModelArtifactsByModelVersionHandler(t *testing.T) {
data := mocks.GetModelArtifactListMock()
expected := ModelArtifactListEnvelope{Data: &data}

actual, rs, err := setupApiTest[ModelArtifactListEnvelope](http.MethodGet, "/api/v1/model_registry/model-registry/model_versions/1/artifacts", nil)
assert.NoError(t, err)

assert.Equal(t, http.StatusOK, rs.StatusCode)
assert.Equal(t, expected.Data.Size, actual.Data.Size)
assert.Equal(t, expected.Data.PageSize, actual.Data.PageSize)
assert.Equal(t, expected.Data.NextPageToken, actual.Data.NextPageToken)
assert.Equal(t, len(expected.Data.Items), len(actual.Data.Items))
}

func TestCreateModelArtifactByModelVersionHandler(t *testing.T) {
data := mocks.GetModelArtifactMocks()[0]
expected := ModelArtifactEnvelope{Data: &data}

artifact := openapi.ModelArtifact{
Name: openapi.PtrString("Artifact One"),
ArtifactType: "ARTIFACT_TYPE_ONE",
}
body := ModelArtifactEnvelope{Data: &artifact}

actual, rs, err := setupApiTest[ModelArtifactEnvelope](http.MethodPost, "/api/v1/model_registry/model-registry/model_versions/1/artifacts", body)
assert.NoError(t, err)

assert.Equal(t, http.StatusCreated, rs.StatusCode)
assert.Equal(t, expected.Data.GetArtifactType(), actual.Data.GetArtifactType())
assert.Equal(t, rs.Header.Get("Location"), "/api/v1/model_registry/model-registry/model_artifacts/1")
}
var _ = Describe("TestGetModelVersionHandler", func() {
Context("testing Model Version Handler", Ordered, func() {

It("should retrieve a model version", func() {
By("fetching a model version")
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}
actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodGet, "/api/v1/model_registry/model-registry/model_versions/1", nil, k8sClient)
Expect(err).NotTo(HaveOccurred())
By("should match the expected model version")
Expect(rs.StatusCode).To(Equal(http.StatusOK))
Expect(actual.Data.Name).To(Equal(expected.Data.Name))
})

It("should create a model version", func() {
By("creating a model version")
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}
body := ModelVersionEnvelope{Data: openapi.NewModelVersion("Model One", "1")}
actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodPost, "/api/v1/model_registry/model-registry/model_versions", body, k8sClient)
Expect(err).NotTo(HaveOccurred())

By("should match the expected model version created")
Expect(rs.StatusCode).To(Equal(http.StatusCreated))
Expect(actual.Data.Name).To(Equal(expected.Data.Name))
Expect(rs.Header.Get("Location")).To(Equal("/api/v1/model_registry/model-registry/model_versions/1"))
})

It("should updated a model version", func() {
By("updating a model version")
data := mocks.GetModelVersionMocks()[0]
expected := ModelVersionEnvelope{Data: &data}

reqData := openapi.ModelVersionUpdate{
Description: openapi.PtrString("New description"),
}
body := ModelVersionUpdateEnvelope{Data: &reqData}

actual, rs, err := setupApiTest[ModelVersionEnvelope](http.MethodPatch, "/api/v1/model_registry/model-registry/model_versions/1", body, k8sClient)
Expect(err).NotTo(HaveOccurred())

By("should match the expected model version updated")
Expect(rs.StatusCode).To(Equal(http.StatusOK))
Expect(actual.Data.Name).To(Equal(expected.Data.Name))
})

It("get all model artifacts by a model version", func() {
By("getting a model artifacts by model version")
data := mocks.GetModelArtifactListMock()
expected := ModelArtifactListEnvelope{Data: &data}
actual, rs, err := setupApiTest[ModelArtifactListEnvelope](http.MethodGet, "/api/v1/model_registry/model-registry/model_versions/1/artifacts", nil, k8sClient)
Expect(err).NotTo(HaveOccurred())

By("should get all expected model version artifacts")
Expect(rs.StatusCode).To(Equal(http.StatusOK))
Expect(actual.Data.Size).To(Equal(expected.Data.Size))
Expect(actual.Data.PageSize).To(Equal(expected.Data.PageSize))
Expect(actual.Data.NextPageToken).To(Equal(expected.Data.NextPageToken))
Expect(len(actual.Data.Items)).To(Equal(len(expected.Data.Items)))
})

It("create Model Artifact By Model Version", func() {
By("creating a model version")
data := mocks.GetModelArtifactMocks()[0]
expected := ModelArtifactEnvelope{Data: &data}

artifact := openapi.ModelArtifact{
Name: openapi.PtrString("Artifact One"),
ArtifactType: "ARTIFACT_TYPE_ONE",
}
body := ModelArtifactEnvelope{Data: &artifact}
actual, rs, err := setupApiTest[ModelArtifactEnvelope](http.MethodPost, "/api/v1/model_registry/model-registry/model_versions/1/artifacts", body, k8sClient)
Expect(err).NotTo(HaveOccurred())

By("should get all expected model artifacts")
Expect(rs.StatusCode).To(Equal(http.StatusCreated))
Expect(actual.Data.GetArtifactType()).To(Equal(expected.Data.GetArtifactType()))
Expect(rs.Header.Get("Location")).To(Equal("/api/v1/model_registry/model-registry/model_artifacts/1"))

})
})
})
Loading