Skip to content

Commit

Permalink
refactor(controller): remove most dependencies from controller (#405)
Browse files Browse the repository at this point in the history
Because

- reduce dependency cycle between `controller-model` and `model-backend`

This commit

- remove resource updating from `model-backend` to `controller-model` to
avoid api calls cycle
- update public deploy/undeploy methods to change the desire state only,
and let `controller-model` handle the actual deployment request
- update integration-test regarding to the deploy/undeploy public
methods
  • Loading branch information
heiruwu committed Aug 18, 2023
1 parent fc4ef15 commit b7b36a6
Show file tree
Hide file tree
Showing 33 changed files with 551 additions and 440 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0
github.com/iancoleman/strcase v0.2.0
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230801085304-c9e30fb0f220
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230818082151-fe5c9357e125
github.com/instill-ai/usage-client v0.2.4-alpha
github.com/instill-ai/x v0.3.0-alpha
github.com/knadh/koanf v1.4.4
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1305,8 +1305,8 @@ github.com/imdario/mergo v0.3.10/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH
github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230801085304-c9e30fb0f220 h1:moXVYUrVc8N4TFNVykVCiW5Ens2EzI09GhyuNV6F3u4=
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230801085304-c9e30fb0f220/go.mod h1:qsq5ecnA1xi2rLnVQFo/9xksA7I7wQu8c7rqM5xbIrQ=
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230818082151-fe5c9357e125 h1:T8ey9xcT9gSkd675d1XdOgC5KEFB/TxW/dMbVUHqMEk=
github.com/instill-ai/protogen-go v0.3.3-alpha.0.20230818082151-fe5c9357e125/go.mod h1:qsq5ecnA1xi2rLnVQFo/9xksA7I7wQu8c7rqM5xbIrQ=
github.com/instill-ai/usage-client v0.2.4-alpha h1:mYXd62eZsmGKBlzwMcdEgTBgn8zlbagYUHro6+p50c8=
github.com/instill-ai/usage-client v0.2.4-alpha/go.mod h1:BMxgyr02sqH6SeITXSV4M1ewwvfklzXIc5yzIqaN0c8=
github.com/instill-ai/x v0.3.0-alpha h1:z9fedROOG2dVHhswBfVwU/hzHuq8/JKSUON7inF+FH8=
Expand Down
3 changes: 2 additions & 1 deletion integration-test/grpc.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ export default () => {
queryModelPrivate.ListModels()
queryModelPrivate.LookUpModel()
deployModelPrivate.CheckModel()
deployModelPrivate.DeployUndeployModel()
// private deploy will be triggered by public deploy
// deployModelPrivate.DeployUndeployModel()
}

// Create model API
Expand Down
4 changes: 1 addition & 3 deletions integration-test/grpc_create_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ export function CreateModel() {
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeployModel', req, {}), {
'DeployModel status': (r) => r && r.status === grpc.StatusOK,
'DeployModel operation name': (r) => r && r.message.operation.name !== undefined,
'DeployModel operation metadata': (r) => r && r.message.operation.metadata === null,
'DeployModel operation done': (r) => r && r.message.operation.done === false,
'DeployModel model name': (r) => r && r.message.modelId === model_id
});

// Check the model state being updated in 120 secs (in integration test, model is dummy model without download time but in real use case, time will be longer)
Expand Down
4 changes: 1 addition & 3 deletions integration-test/grpc_deploy_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ export function DeployUndeployModel() {
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeployModel', req, {}), {
'DeployModel status': (r) => r && r.status === grpc.StatusOK,
'DeployModel operation name': (r) => r && r.message.operation.name !== undefined,
'DeployModel operation metadata': (r) => r && r.message.operation.metadata === null,
'DeployModel operation done': (r) => r && r.message.operation.done === false,
'DeployModel model name': (r) => r && r.message.modelId === model_id
});

// Check the model state being updated in 120 secs (in integration test, model is dummy model without download time but in real use case, time will be longer)
Expand Down
12 changes: 12 additions & 0 deletions integration-test/grpc_deploy_model_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,18 @@ export function CheckModel() {
}, {}), {
'CheckModelAdmin uuid length is invalid': (r) => r && r.status === grpc.StatusInvalidArgument,
});
currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = publicClient.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(publicClient.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down
8 changes: 2 additions & 6 deletions integration-test/grpc_infer_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ export function InferModel() {
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeployModel', req, {}), {
'DeployModel status': (r) => r && r.status === grpc.StatusOK,
'DeployModel operation name': (r) => r && r.message.operation.name !== undefined,
'DeployModel operation metadata': (r) => r && r.message.operation.metadata === null,
'DeployModel operation done': (r) => r && r.message.operation.done === false,
'DeployModel model name': (r) => r && r.message.modelId === model_id
});

// Check the model state being updated in 120 secs (in integration test, model is dummy model without download time but in real use case, time will be longer)
Expand Down Expand Up @@ -181,9 +179,7 @@ export function InferModel() {
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeployModel', req, {}), {
'DeployModel status': (r) => r && r.status === grpc.StatusOK,
'DeployModel operation name': (r) => r && r.message.operation.name !== undefined,
'DeployModel operation metadata': (r) => r && r.message.operation.metadata === null,
'DeployModel operation done': (r) => r && r.message.operation.done === false,
'DeployModel model name': (r) => r && r.message.modelId === model_id
});

// Check the model state being updated in 120 secs (in integration test, model is dummy model without download time but in real use case, time will be longer)
Expand Down
13 changes: 12 additions & 1 deletion integration-test/grpc_publish_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,18 @@ export function PublishUnPublishModel() {
}), {
"UnpublishModel response not found status": (r) => r.status === grpc.StatusNotFound,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = client.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down
38 changes: 36 additions & 2 deletions integration-test/grpc_query_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,18 @@ export function GetModel() {
}, {}), {
'GetModel non-existed model status not found': (r) => r && r.status === grpc.StatusNotFound,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = client.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down Expand Up @@ -149,7 +160,18 @@ export function ListModels() {
"ListModels response models[0].create_time": (r) => r.message.models[0].createTime !== undefined,
"ListModels response models[0].update_time": (r) => r.message.models[0].updateTime !== undefined,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = client.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down Expand Up @@ -221,6 +243,18 @@ export function LookupModel() {
}, {}), {
'LookUpModel non-existed model status not found': (r) => r && r.status === grpc.StatusInvalidArgument,
});
currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = client.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down
25 changes: 24 additions & 1 deletion integration-test/grpc_query_model_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,18 @@ export function ListModels() {
"ListModelsAdmin response models[0].create_time": (r) => r.message.models[0].createTime !== undefined,
"ListModelsAdmin response models[0].update_time": (r) => r.message.models[0].updateTime !== undefined,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = publicClient.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(publicClient.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down Expand Up @@ -167,6 +178,18 @@ export function LookUpModel() {
}, {}), {
'LookUpModelAdmin non-existed model status not found': (r) => r && r.status === grpc.StatusInvalidArgument,
});
currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = publicClient.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(publicClient.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down
13 changes: 12 additions & 1 deletion integration-test/grpc_update_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,18 @@ export function UpdateModel() {
"UpdateModel response model.create_time": (r) => r.message.model.createTime !== undefined,
"UpdateModel response model.update_time": (r) => r.message.model.updateTime !== undefined,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = client.invoke('model.model.v1alpha.ModelPublicService/WatchModel', {
name: `models/${model_id}`
}, {})
if (res.message.state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}
check(client.invoke('model.model.v1alpha.ModelPublicService/DeleteModel', {
name: "models/" + model_id
}), {
Expand Down
10 changes: 6 additions & 4 deletions integration-test/proto/model/model/v1alpha/model.proto
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,9 @@ message DeployModelRequest {

// DeployModelResponse represents a response for a deployed model
message DeployModelResponse {
// Deploy operation message
google.longrunning.Operation operation = 1;
// Deployed model's id
// Format: models/{model}
string model_id = 1;
}

// UndeployModelRequest represents a request to undeploy a model to offline
Expand All @@ -357,8 +358,9 @@ message UndeployModelRequest {

// UndeployModelResponse represents a response for a undeployed model
message UndeployModelResponse {
// Undeploy operation message
google.longrunning.Operation operation = 1;
// Undeployed model's id
// Format: models/{model}
string model_id = 1;
}

// GetModelCardRequest represents a request to query a model's README card
Expand Down
3 changes: 2 additions & 1 deletion integration-test/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ export default function (data) {
if (!constant.apiGatewayMode) {
queryModelPrivate.ListModelsAdmin()
queryModelPrivate.LookupModelAdmin()
deployModelPrivate.DeployUndeployModel()
// private deploy will be trigger by public deploy
// deployModelPrivate.DeployUndeployModel()
}

// Infer Model API
Expand Down
63 changes: 63 additions & 0 deletions integration-test/rest_create_model.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,31 @@ export function CreateModelFromLocal() {
r.status === 409,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res_cls = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_cls}/watch`, {
headers: genHeader(`application/json`),
})
let res_det = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_det}/watch`, {
headers: genHeader(`application/json`),
})
let res_keypoint = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_keypoint}/watch`, {
headers: genHeader(`application/json`),
})
let res_unspecified = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_unspecified}/watch`, {
headers: genHeader(`application/json`),
})
if (res_cls.json().state !== "STATE_UNSPECIFIED" &&
res_det.json().state !== "STATE_UNSPECIFIED" &&
res_keypoint.json().state !== "STATE_UNSPECIFIED" &&
res_unspecified.json().state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}

// clean up
check(http.request("DELETE", `${constant.apiPublicHost}/v1alpha/models/${model_id_cls}`, null, {
headers: genHeader(`application/json`),
Expand Down Expand Up @@ -281,6 +306,31 @@ export function CreateModelFromLocal() {
r.status === 400,
});

let currentTime = new Date().getTime();
let timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res_cls = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_cls}/watch`, {
headers: genHeader(`application/json`),
})
let res_det = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_det}/watch`, {
headers: genHeader(`application/json`),
})
let res_keypoint = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_keypoint}/watch`, {
headers: genHeader(`application/json`),
})
let res_unspecified = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id_unspecified}/watch`, {
headers: genHeader(`application/json`),
})
if (res_cls.json().state !== "STATE_UNSPECIFIED" &&
res_det.json().state !== "STATE_UNSPECIFIED" &&
res_keypoint.json().state !== "STATE_UNSPECIFIED" &&
res_unspecified.json().state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}

// clean up
check(http.request("DELETE", `${constant.apiPublicHost}/v1alpha/models/${model_id_cls}`, null, {
headers: genHeader(`application/json`),
Expand Down Expand Up @@ -388,6 +438,19 @@ export function CreateModelFromGitHub() {
r.status === 400,
});

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id}/watch`, {
headers: genHeader(`application/json`),
})
if (res.json().state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}

// clean up
check(http.request("DELETE", `${constant.apiPublicHost}/v1alpha/models/${model_id}`, null, {
headers: genHeader(`application/json`),
Expand Down
26 changes: 26 additions & 0 deletions integration-test/rest_create_model_with_jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ export function CreateModelFromLocal() {
currentTime = new Date().getTime();
}

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id}/watch`, {
headers: genHeader(`application/json`),
})
if (res.json().state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}

// clean up
check(http.request("DELETE", `${constant.apiPublicHost}/v1alpha/models/${model_id}`, null, {
headers: genHeaderwithJwtSub(`application/json`, uuidv4()),
Expand Down Expand Up @@ -147,6 +160,19 @@ export function CreateModelFromGitHub() {
currentTime = new Date().getTime();
}

currentTime = new Date().getTime();
timeoutTime = new Date().getTime() + 120000;
while (timeoutTime > currentTime) {
let res = http.get(`${constant.apiPublicHost}/v1alpha/models/${model_id}/watch`, {
headers: genHeader(`application/json`),
})
if (res.json().state !== "STATE_UNSPECIFIED") {
break
}
sleep(1)
currentTime = new Date().getTime();
}

// clean up
check(http.request("DELETE", `${constant.apiPublicHost}/v1alpha/models/${model_id}`, null, {
headers: genHeaderwithJwtSub(`application/json`, uuidv4()),
Expand Down
Loading

0 comments on commit b7b36a6

Please sign in to comment.