From f23853aa3573007a4edbcf5996b22fb709ecfaf2 Mon Sep 17 00:00:00 2001 From: Ivan Zhang Date: Thu, 13 Jun 2019 13:37:44 -0400 Subject: [PATCH 1/5] add external region --- docs/applications/advanced/external-models.md | 6 +- docs/applications/resources/apis.md | 4 +- examples/external-model/app.yaml | 4 +- pkg/lib/aws/s3.go | 31 +++++++++- pkg/operator/api/userconfig/apis.go | 58 ++++++++++++++----- pkg/operator/api/userconfig/errors.go | 11 +++- pkg/operator/context/apis.go | 7 ++- pkg/workloads/tf_api/api.py | 2 +- 8 files changed, 99 insertions(+), 24 deletions(-) diff --git a/docs/applications/advanced/external-models.md b/docs/applications/advanced/external-models.md index d82abdb466..ea767666d0 100644 --- a/docs/applications/advanced/external-models.md +++ b/docs/applications/advanced/external-models.md @@ -17,12 +17,14 @@ $ zip -r model.zip export/estimator $ aws s3 cp model.zip s3://your-bucket/model.zip ``` -3. Specify `model_path` in an API, e.g. +3. Specify `external_model` in an API, e.g. ```yaml - kind: api name: my-api - model_path: s3://your-bucket/model.zip + external_model: + path: s3://your-bucket/model.zip + region: us-west-2 compute: replicas: 5 gpu: 1 diff --git a/docs/applications/resources/apis.md b/docs/applications/resources/apis.md index 29c8ac0638..a8260b7cb9 100644 --- a/docs/applications/resources/apis.md +++ b/docs/applications/resources/apis.md @@ -8,7 +8,9 @@ Serve models at scale and use them to build smarter applications. - kind: api # (required) name: # API name (required) model_name: # name of a Cortex model (required) - model_path: # path to a zipped model dir (optional) + external_model: + path: # path to a zipped model dir (optional) + region: # region of external model compute: replicas: # number of replicas to launch (default: 1) cpu: # CPU request (default: Null) diff --git a/examples/external-model/app.yaml b/examples/external-model/app.yaml index a9c60cd5ed..bf136ab01c 100644 --- a/examples/external-model/app.yaml +++ b/examples/external-model/app.yaml @@ -3,6 +3,8 @@ - kind: api name: iris - model_path: s3://cortex-examples/iris-model.zip + external_model: + path: s3://cortex2-test-us-east/iris-model.zip + region: us-east-1 compute: replicas: 1 diff --git a/pkg/lib/aws/s3.go b/pkg/lib/aws/s3.go index 4bcd6a7c4f..7f7557476d 100644 --- a/pkg/lib/aws/s3.go +++ b/pkg/lib/aws/s3.go @@ -265,7 +265,19 @@ func SplitS3aPath(s3aPath string) (string, string, error) { if !IsValidS3aPath(s3aPath) { return "", "", ErrorInvalidS3aPath(s3aPath) } - fullPath := s3aPath[6:] + fullPath := s3aPath[len("s3a://"):] + slashIndex := strings.Index(fullPath, "/") + bucket := fullPath[0:slashIndex] + key := fullPath[slashIndex+1:] + + return bucket, key, nil +} + +func SplitS3Path(s3Path string) (string, string, error) { + if !IsValidS3Path(s3Path) { + return "", "", ErrorInvalidS3aPath(s3Path) + } + fullPath := s3Path[len("s3://"):] slashIndex := strings.Index(fullPath, "/") bucket := fullPath[0:slashIndex] key := fullPath[slashIndex+1:] @@ -291,6 +303,23 @@ func IsS3PrefixExternal(bucket string, prefix string, region string) (bool, erro return hasPrefix, nil } +func IsS3FileExternal(bucket string, key string, region string) (bool, error) { + sess := session.Must(session.NewSession(&aws.Config{ + Region: aws.String(region), + })) + + out, err := s3.New(sess).HeadObject(&s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + }) + + if err != nil { + return false, errors.Wrap(err, key) + } + + return out != nil, nil +} + func IsS3aPrefixExternal(s3aPath string, region string) (bool, error) { bucket, prefix, err := SplitS3aPath(s3aPath) if err != nil { diff --git a/pkg/operator/api/userconfig/apis.go b/pkg/operator/api/userconfig/apis.go index fbc960d628..35a34be232 100644 --- a/pkg/operator/api/userconfig/apis.go +++ b/pkg/operator/api/userconfig/apis.go @@ -17,6 +17,7 @@ limitations under the License. package userconfig import ( + "github.com/cortexlabs/cortex/pkg/lib/aws" cr "github.com/cortexlabs/cortex/pkg/lib/configreader" "github.com/cortexlabs/cortex/pkg/lib/errors" "github.com/cortexlabs/cortex/pkg/operator/api/resource" @@ -26,10 +27,10 @@ type APIs []*API type API struct { ResourceFields - Model *string `json:"model" yaml:"model"` - ModelPath *string `json:"model_path" yaml:"model_path"` - Compute *APICompute `json:"compute" yaml:"compute"` - Tags Tags `json:"tags" yaml:"tags"` + Model *string `json:"model" yaml:"model"` + ExternalModel *ExternalModel `json:"external_model" yaml:"external_model"` + Compute *APICompute `json:"compute" yaml:"compute"` + Tags Tags `json:"tags" yaml:"tags"` } var apiValidation = &cr.StructValidation{ @@ -47,18 +48,36 @@ var apiValidation = &cr.StructValidation{ RequireCortexResources: true, }, }, - { - StructField: "ModelPath", - StringPtrValidation: &cr.StringPtrValidation{ - Validator: cr.GetS3PathValidator(), - }, - }, + externalModelFieldValidation, apiComputeFieldValidation, tagsFieldValidation, typeFieldValidation, }, } +type ExternalModel struct { + Path string `json:"path" yaml:"path"` + Region string `json:"region" yaml:"region"` +} + +var externalModelFieldValidation = &cr.StructFieldValidation{ + StructField: "ExternalModel", + StructValidation: &cr.StructValidation{ + StructFieldValidations: []*cr.StructFieldValidation{ + { + StructField: "Path", + StringValidation: &cr.StringValidation{ + Validator: cr.GetS3PathValidator(), + }, + }, + { + StructField: "Region", + StringValidation: &cr.StringValidation{}, + }, + }, + }, +} + func (apis APIs) Validate() error { for _, api := range apis { if err := api.Validate(); err != nil { @@ -80,12 +99,23 @@ func (apis APIs) Validate() error { } func (api *API) Validate() error { - if api.ModelPath == nil && api.Model == nil { - return errors.Wrap(ErrorSpecifyOnlyOneMissing("model_name", "model_path"), Identify(api)) + if api.ExternalModel == nil && api.Model == nil { + return errors.Wrap(ErrorSpecifyOnlyOneMissing("model", "external_model"), Identify(api)) + } + + if api.ExternalModel != nil && api.Model != nil { + return errors.Wrap(ErrorSpecifyOnlyOne("model", "external_model"), Identify(api)) } - if api.ModelPath != nil && api.Model != nil { - return errors.Wrap(ErrorSpecifyOnlyOne("model_name", "model_path"), Identify(api)) + if api.ExternalModel != nil { + bucket, key, err := aws.SplitS3Path(api.ExternalModel.Path) + if err != nil { + return err + } + + if ok, err := aws.IsS3FileExternal(bucket, key, api.ExternalModel.Region); err != nil || !ok { + return ErrorExternalModelNotFound(api.ExternalModel.Path) + } } return nil diff --git a/pkg/operator/api/userconfig/errors.go b/pkg/operator/api/userconfig/errors.go index 19b1416996..87334d0fa0 100644 --- a/pkg/operator/api/userconfig/errors.go +++ b/pkg/operator/api/userconfig/errors.go @@ -75,6 +75,7 @@ const ( ErrEnvSchemaMismatch ErrExtraResourcesWithExternalAPIs ErrImplDoesNotExist + ErrExternalModelNotFound ) var errorKinds = []string{ @@ -124,9 +125,10 @@ var errorKinds = []string{ "err_env_schema_mismatch", "err_extra_resources_with_external_a_p_is", "err_impl_does_not_exist", + "err_external_model_not_found", } -var _ = [1]int{}[int(ErrImplDoesNotExist)-(len(errorKinds)-1)] // Ensure list length matches +var _ = [1]int{}[int(ErrExternalModelNotFound)-(len(errorKinds)-1)] // Ensure list length matches func (t ErrorKind) String() string { return errorKinds[t] @@ -575,3 +577,10 @@ func ErrorImplDoesNotExist(path string) error { message: fmt.Sprintf("%s: implementation file does not exist", path), } } + +func ErrorExternalModelNotFound(path string) error { + return Error{ + Kind: ErrExternalModelNotFound, + message: fmt.Sprintf("%s: file not found or inaccessible", path), + } +} diff --git a/pkg/operator/context/apis.go b/pkg/operator/context/apis.go index c3383d49de..dbc7322bf4 100644 --- a/pkg/operator/context/apis.go +++ b/pkg/operator/context/apis.go @@ -48,10 +48,11 @@ func getAPIs(config *userconfig.Config, buf.WriteString(model.ID) } - if apiConfig.ModelPath != nil { - modelName = *apiConfig.ModelPath + if apiConfig.ExternalModel != nil { + modelName = apiConfig.ExternalModel.Path buf.WriteString(datasetVersion) - buf.WriteString(*apiConfig.ModelPath) + buf.WriteString(apiConfig.ExternalModel.Path) + buf.WriteString(apiConfig.ExternalModel.Region) } id := hash.Bytes(buf.Bytes()) diff --git a/pkg/workloads/tf_api/api.py b/pkg/workloads/tf_api/api.py index 69fe3ff7da..5135758559 100644 --- a/pkg/workloads/tf_api/api.py +++ b/pkg/workloads/tf_api/api.py @@ -402,7 +402,7 @@ def start(args): else: if not os.path.isdir(args.model_dir): - ctx.storage.download_and_unzip_external(api["model_path"], args.model_dir) + ctx.storage.download_and_unzip_external(api["external_model"]["path"], args.model_dir) channel = grpc.insecure_channel("localhost:" + str(args.tf_serve_port)) local_cache["stub"] = prediction_service_pb2_grpc.PredictionServiceStub(channel) From f02092254df01c7f52e2fd36c8808a367b04a27a Mon Sep 17 00:00:00 2001 From: Ivan Zhang Date: Thu, 13 Jun 2019 13:46:47 -0400 Subject: [PATCH 2/5] fix validation --- examples/external-model/app.yaml | 2 +- pkg/operator/api/userconfig/apis.go | 29 +++++++++++++++-------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/examples/external-model/app.yaml b/examples/external-model/app.yaml index bf136ab01c..fab2ba42fb 100644 --- a/examples/external-model/app.yaml +++ b/examples/external-model/app.yaml @@ -4,7 +4,7 @@ - kind: api name: iris external_model: - path: s3://cortex2-test-us-east/iris-model.zip + path: s3://cortex-test-us-east/iris-model.zip region: us-east-1 compute: replicas: 1 diff --git a/pkg/operator/api/userconfig/apis.go b/pkg/operator/api/userconfig/apis.go index 35a34be232..ed8b7ab08d 100644 --- a/pkg/operator/api/userconfig/apis.go +++ b/pkg/operator/api/userconfig/apis.go @@ -48,7 +48,10 @@ var apiValidation = &cr.StructValidation{ RequireCortexResources: true, }, }, - externalModelFieldValidation, + { + StructField: "ExternalModel", + StructValidation: externalModelFieldValidation, + }, apiComputeFieldValidation, tagsFieldValidation, typeFieldValidation, @@ -60,21 +63,19 @@ type ExternalModel struct { Region string `json:"region" yaml:"region"` } -var externalModelFieldValidation = &cr.StructFieldValidation{ - StructField: "ExternalModel", - StructValidation: &cr.StructValidation{ - StructFieldValidations: []*cr.StructFieldValidation{ - { - StructField: "Path", - StringValidation: &cr.StringValidation{ - Validator: cr.GetS3PathValidator(), - }, - }, - { - StructField: "Region", - StringValidation: &cr.StringValidation{}, +var externalModelFieldValidation = &cr.StructValidation{ + DefaultNil: true, + StructFieldValidations: []*cr.StructFieldValidation{ + { + StructField: "Path", + StringValidation: &cr.StringValidation{ + Validator: cr.GetS3PathValidator(), }, }, + { + StructField: "Region", + StringValidation: &cr.StringValidation{}, + }, }, } From b87f03e864915459cebdcb9c85ecaad8ef00637f Mon Sep 17 00:00:00 2001 From: Ivan Zhang Date: Thu, 13 Jun 2019 14:37:06 -0400 Subject: [PATCH 3/5] address comments --- examples/external-model/app.yaml | 4 ++-- pkg/lib/aws/s3.go | 8 ++++++-- pkg/operator/api/userconfig/apis.go | 16 ++++++++++------ pkg/operator/api/userconfig/config_key.go | 5 +++-- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/examples/external-model/app.yaml b/examples/external-model/app.yaml index fab2ba42fb..c3f4f26457 100644 --- a/examples/external-model/app.yaml +++ b/examples/external-model/app.yaml @@ -4,7 +4,7 @@ - kind: api name: iris external_model: - path: s3://cortex-test-us-east/iris-model.zip - region: us-east-1 + path: s3://cortex-examples/ir3is-model.zip + region: us-west-2 compute: replicas: 1 diff --git a/pkg/lib/aws/s3.go b/pkg/lib/aws/s3.go index 7f7557476d..51211c0a02 100644 --- a/pkg/lib/aws/s3.go +++ b/pkg/lib/aws/s3.go @@ -308,16 +308,20 @@ func IsS3FileExternal(bucket string, key string, region string) (bool, error) { Region: aws.String(region), })) - out, err := s3.New(sess).HeadObject(&s3.HeadObjectInput{ + _, err := s3.New(sess).HeadObject(&s3.HeadObjectInput{ Bucket: aws.String(bucket), Key: aws.String(key), }) + if IsNotFoundErr(err) { + return false, nil + } + if err != nil { return false, errors.Wrap(err, key) } - return out != nil, nil + return true, nil } func IsS3aPrefixExternal(s3aPath string, region string) (bool, error) { diff --git a/pkg/operator/api/userconfig/apis.go b/pkg/operator/api/userconfig/apis.go index ed8b7ab08d..87048dd85c 100644 --- a/pkg/operator/api/userconfig/apis.go +++ b/pkg/operator/api/userconfig/apis.go @@ -70,11 +70,15 @@ var externalModelFieldValidation = &cr.StructValidation{ StructField: "Path", StringValidation: &cr.StringValidation{ Validator: cr.GetS3PathValidator(), + Required: true, }, }, { - StructField: "Region", - StringValidation: &cr.StringValidation{}, + StructField: "Region", + StringValidation: &cr.StringValidation{ + Default: aws.DefaultS3Region, + AllowedValues: aws.S3Regions.Slice(), + }, }, }, } @@ -101,21 +105,21 @@ func (apis APIs) Validate() error { func (api *API) Validate() error { if api.ExternalModel == nil && api.Model == nil { - return errors.Wrap(ErrorSpecifyOnlyOneMissing("model", "external_model"), Identify(api)) + return errors.Wrap(ErrorSpecifyOnlyOneMissing(ModelKey, ExternalModelKey), Identify(api)) } if api.ExternalModel != nil && api.Model != nil { - return errors.Wrap(ErrorSpecifyOnlyOne("model", "external_model"), Identify(api)) + return errors.Wrap(ErrorSpecifyOnlyOne(ModelKey, ExternalModelKey), Identify(api)) } if api.ExternalModel != nil { bucket, key, err := aws.SplitS3Path(api.ExternalModel.Path) if err != nil { - return err + return errors.Wrap(err, Identify(api), ExternalModelKey, PathKey) } if ok, err := aws.IsS3FileExternal(bucket, key, api.ExternalModel.Region); err != nil || !ok { - return ErrorExternalModelNotFound(api.ExternalModel.Path) + return errors.Wrap(ErrorExternalModelNotFound(api.ExternalModel.Path), Identify(api), ExternalModelKey, PathKey) } } diff --git a/pkg/operator/api/userconfig/config_key.go b/pkg/operator/api/userconfig/config_key.go index b5a5830e20..f4ea733b48 100644 --- a/pkg/operator/api/userconfig/config_key.go +++ b/pkg/operator/api/userconfig/config_key.go @@ -93,6 +93,7 @@ const ( DatasetComputeKey = "dataset_compute" // API - ModelKey = "model" - ModelNameKey = "model_name" + ModelKey = "model" + ModelNameKey = "model_name" + ExternalModelKey = "external_model" ) From a1fe6cff969e96c2202a5492ba6e55d6fa475284 Mon Sep 17 00:00:00 2001 From: Ivan Zhang Date: Thu, 13 Jun 2019 14:40:10 -0400 Subject: [PATCH 4/5] oops --- examples/external-model/app.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/external-model/app.yaml b/examples/external-model/app.yaml index c3f4f26457..257402bc63 100644 --- a/examples/external-model/app.yaml +++ b/examples/external-model/app.yaml @@ -4,7 +4,7 @@ - kind: api name: iris external_model: - path: s3://cortex-examples/ir3is-model.zip - region: us-west-2 + path: s3://cortex-examples/iris-model.zip + region: us-east-2 compute: replicas: 1 From f5f781317576a0a19eaf44af25e30cf6e58b8274 Mon Sep 17 00:00:00 2001 From: Ivan Zhang Date: Thu, 13 Jun 2019 14:42:37 -0400 Subject: [PATCH 5/5] oops 2 --- examples/external-model/app.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/external-model/app.yaml b/examples/external-model/app.yaml index 257402bc63..918656db7a 100644 --- a/examples/external-model/app.yaml +++ b/examples/external-model/app.yaml @@ -5,6 +5,6 @@ name: iris external_model: path: s3://cortex-examples/iris-model.zip - region: us-east-2 + region: us-west-2 compute: replicas: 1