Skip to content

Allow blank bucket entries #875

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

Merged
merged 2 commits into from
Mar 17, 2020
Merged

Allow blank bucket entries #875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df32bf0
Allow blank bucket entries
vishalbollu Mar 17, 2020
5bf87f5
Update lib_cluster_config.go
vishalbollu Mar 17, 2020
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
7 changes: 4 additions & 3 deletions cli/cmd/lib_cluster_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ func getClusterUpdateConfig(cachedClusterConfig clusterconfig.Config, awsCreds A
return nil, err
}

if userClusterConfig.Bucket == nil {
userClusterConfig.Bucket = cachedClusterConfig.Bucket
if userClusterConfig.Bucket != "" && userClusterConfig.Bucket != cachedClusterConfig.Bucket {
return nil, clusterconfig.ErrorConfigCannotBeChangedOnUpdate(clusterconfig.BucketKey, cachedClusterConfig.Bucket)
}
userClusterConfig.Bucket = cachedClusterConfig.Bucket

if userClusterConfig.InstanceType != nil && *userClusterConfig.InstanceType != *cachedClusterConfig.InstanceType {
return nil, clusterconfig.ErrorConfigCannotBeChangedOnUpdate(clusterconfig.InstanceTypeKey, *cachedClusterConfig.InstanceType)
Expand Down Expand Up @@ -355,7 +356,7 @@ func confirmInstallClusterConfig(clusterConfig *clusterconfig.Config, awsCreds A

fmt.Printf("your cluster will cost %s per hour%s\n\n", priceStr, suffix)

fmt.Printf("cortex will also create an s3 bucket (%s) and a cloudwatch log group (%s)\n\n", *clusterConfig.Bucket, clusterConfig.LogGroup)
fmt.Printf("cortex will also create an s3 bucket (%s) and a cloudwatch log group (%s)\n\n", clusterConfig.Bucket, clusterConfig.LogGroup)

if isSpot && clusterConfig.SpotConfig.OnDemandBackup != nil && !*clusterConfig.SpotConfig.OnDemandBackup {
if *clusterConfig.SpotConfig.OnDemandBaseCapacity == 0 && *clusterConfig.SpotConfig.OnDemandPercentageAboveBaseCapacity == 0 {
Expand Down
2 changes: 1 addition & 1 deletion docs/cluster-management/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ cluster_name: cortex
region: us-west-2

# S3 bucket (default: <cluster_name>-<RANDOM_ID>)
bucket: cortex-<RANDOM_ID>
bucket: # cortex-<RANDOM_ID>

# List of availability zones for your region (default: 3 random availability zones from the specified region)
availability_zones: # e.g. [us-west-2a, us-west-2b, us-west-2c]
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/endpoints/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ func Deploy(w http.ResponseWriter, r *http.Request) {
return
}

isProjectUploaded, err := config.AWS.IsS3File(*config.Cluster.Bucket, projectKey)
isProjectUploaded, err := config.AWS.IsS3File(config.Cluster.Bucket, projectKey)
if err != nil {
respondError(w, r, err)
return
}
if !isProjectUploaded {
if err = config.AWS.UploadBytesToS3(projectBytes, *config.Cluster.Bucket, projectKey); err != nil {
if err = config.AWS.UploadBytesToS3(projectBytes, config.Cluster.Bucket, projectKey); err != nil {
respondError(w, r, err)
return
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/operator/operator/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func UpdateAPI(apiConfig *userconfig.API, projectID string, force bool) (*spec.A
api := getAPISpec(apiConfig, projectID, deploymentID)

if prevDeployment == nil {
if err := config.AWS.UploadMsgpackToS3(api, *config.Cluster.Bucket, api.Key); err != nil {
if err := config.AWS.UploadMsgpackToS3(api, config.Cluster.Bucket, api.Key); err != nil {
return nil, "", errors.Wrap(err, "upload api spec")
}
if err := applyK8sResources(api, prevDeployment, prevService, prevVirtualService); err != nil {
Expand All @@ -73,7 +73,7 @@ func UpdateAPI(apiConfig *userconfig.API, projectID string, force bool) (*spec.A
if isUpdating && !force {
return nil, "", ErrorAPIUpdating(api.Name)
}
if err := config.AWS.UploadMsgpackToS3(api, *config.Cluster.Bucket, api.Key); err != nil {
if err := config.AWS.UploadMsgpackToS3(api, config.Cluster.Bucket, api.Key); err != nil {
return nil, "", errors.Wrap(err, "upload api spec")
}
if err := applyK8sResources(api, prevDeployment, prevService, prevVirtualService); err != nil {
Expand Down Expand Up @@ -122,7 +122,7 @@ func RefreshAPI(apiName string, force bool) (string, error) {

api = getAPISpec(api.API, api.ProjectID, k8s.RandomName())

if err := config.AWS.UploadMsgpackToS3(api, *config.Cluster.Bucket, api.Key); err != nil {
if err := config.AWS.UploadMsgpackToS3(api, config.Cluster.Bucket, api.Key); err != nil {
return "", errors.Wrap(err, "upload api spec")
}

Expand Down Expand Up @@ -309,7 +309,7 @@ func deleteK8sResources(apiName string) error {

func deleteS3Resources(apiName string) error {
prefix := filepath.Join("apis", apiName)
return config.AWS.DeleteDir(*config.Cluster.Bucket, prefix, true)
return config.AWS.DeleteDir(config.Cluster.Bucket, prefix, true)
}

// returns true if min_replicas are not ready and no updated replicas have errored
Expand Down Expand Up @@ -391,7 +391,7 @@ func DownloadAPISpec(apiName string, apiID string) (*spec.API, error) {
s3Key := specKey(apiName, apiID)
var api spec.API

if err := config.AWS.ReadMsgpackFromS3(&api, *config.Cluster.Bucket, s3Key); err != nil {
if err := config.AWS.ReadMsgpackFromS3(&api, config.Cluster.Bucket, s3Key); err != nil {
return nil, err
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/operator/operator/k8s_specs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func tfDownloadArgs(api *spec.API) string {
LastLog: fmt.Sprintf(_downloaderLastLog, "tensorflow"),
DownloadArgs: []downloadContainerArg{
{
From: aws.S3Path(*config.Cluster.Bucket, api.ProjectKey),
From: aws.S3Path(config.Cluster.Bucket, api.ProjectKey),
To: path.Join(_emptyDirMountPath, "project"),
Unzip: true,
ItemName: "the project code",
Expand Down Expand Up @@ -349,7 +349,7 @@ func pythonDownloadArgs(api *spec.API) string {
LastLog: fmt.Sprintf(_downloaderLastLog, "python"),
DownloadArgs: []downloadContainerArg{
{
From: aws.S3Path(*config.Cluster.Bucket, api.ProjectKey),
From: aws.S3Path(config.Cluster.Bucket, api.ProjectKey),
To: path.Join(_emptyDirMountPath, "project"),
Unzip: true,
ItemName: "the project code",
Expand Down Expand Up @@ -463,7 +463,7 @@ func onnxDownloadArgs(api *spec.API) string {
LastLog: fmt.Sprintf(_downloaderLastLog, "onnx"),
DownloadArgs: []downloadContainerArg{
{
From: aws.S3Path(*config.Cluster.Bucket, api.ProjectKey),
From: aws.S3Path(config.Cluster.Bucket, api.ProjectKey),
To: path.Join(_emptyDirMountPath, "project"),
Unzip: true,
ItemName: "the project code",
Expand Down Expand Up @@ -574,7 +574,7 @@ func getEnvVars(api *spec.API) []kcore.EnvVar {
},
kcore.EnvVar{
Name: "CORTEX_API_SPEC",
Value: aws.S3Path(*config.Cluster.Bucket, api.Key),
Value: aws.S3Path(config.Cluster.Bucket, api.Key),
},
kcore.EnvVar{
Name: "CORTEX_CACHE_DIR",
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/operator/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func getNetworkStatsDef(api *spec.API, period int64) []*cloudwatch.MetricDataQue

func getClassesMetricDef(api *spec.API, period int64) ([]*cloudwatch.MetricDataQuery, error) {
prefix := filepath.Join(api.MetadataRoot, api.ID, "classes") + "/"
classes, err := config.AWS.ListPrefix(*config.Cluster.Bucket, prefix, int64(consts.MaxClassesPerTrackerRequest))
classes, err := config.AWS.ListPrefix(config.Cluster.Bucket, prefix, int64(consts.MaxClassesPerTrackerRequest))
if err != nil {
return nil, err
}
Expand Down
23 changes: 13 additions & 10 deletions pkg/types/clusterconfig/clusterconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type Config struct {
ClusterName string `json:"cluster_name" yaml:"cluster_name"`
Region *string `json:"region" yaml:"region"`
AvailabilityZones []string `json:"availability_zones" yaml:"availability_zones"`
Bucket *string `json:"bucket" yaml:"bucket"`
Bucket string `json:"bucket" yaml:"bucket"`
LogGroup string `json:"log_group" yaml:"log_group"`
Telemetry bool `json:"telemetry" yaml:"telemetry"`
ImagePythonServe string `json:"image_python_serve" yaml:"image_python_serve"`
Expand Down Expand Up @@ -214,8 +214,11 @@ var UserValidation = &cr.StructValidation{
},
},
{
StructField: "Bucket",
StringPtrValidation: &cr.StringPtrValidation{},
StructField: "Bucket",
StringValidation: &cr.StringValidation{
AllowEmpty: true,
TreatNullAsEmpty: true,
},
},
{
StructField: "LogGroup",
Expand Down Expand Up @@ -460,9 +463,9 @@ func (cc *Config) Validate(awsClient *aws.Client) error {
return ErrorMinInstancesGreaterThanMax(*cc.MinInstances, *cc.MaxInstances)
}

bucketRegion, _ := aws.GetBucketRegion(*cc.Bucket)
bucketRegion, _ := aws.GetBucketRegion(cc.Bucket)
if bucketRegion != "" && bucketRegion != *cc.Region { // if the bucket didn't exist, we will create it in the correct region, so there is no error
return ErrorS3RegionDiffersFromCluster(*cc.Bucket, bucketRegion, *cc.Region)
return ErrorS3RegionDiffersFromCluster(cc.Bucket, bucketRegion, *cc.Region)
}

if _, ok := aws.InstanceMetadatas[*cc.Region][*cc.InstanceType]; !ok {
Expand Down Expand Up @@ -726,7 +729,6 @@ func RegionPrompt(clusterConfig *Config) error {

func InstallPrompt(clusterConfig *Config, awsClient *aws.Client) error {
defaults := applyPromptDefaults(*clusterConfig)

accountID, _, err := awsClient.GetCachedAccountID()
if err != nil {
return err
Expand All @@ -742,15 +744,16 @@ func InstallPrompt(clusterConfig *Config, awsClient *aws.Client) error {
}

remainingPrompts := &cr.PromptValidation{
SkipNonNilFields: true,
SkipNonNilFields: true,
SkipNonEmptyFields: true,
PromptItemValidations: []*cr.PromptItemValidation{
{
StructField: "Bucket",
PromptOpts: &prompt.Options{
Prompt: BucketUserKey,
},
StringPtrValidation: &cr.StringPtrValidation{
Default: &defaultBucket,
StringValidation: &cr.StringValidation{
Default: defaultBucket,
MinLength: 3,
MaxLength: 63,
},
Expand Down Expand Up @@ -948,7 +951,7 @@ func (cc *Config) UserTable() table.KeyValuePairs {
if len(cc.AvailabilityZones) > 0 {
items.Add(AvailabilityZonesUserKey, cc.AvailabilityZones)
}
items.Add(BucketUserKey, *cc.Bucket)
items.Add(BucketUserKey, cc.Bucket)
items.Add(InstanceTypeUserKey, *cc.InstanceType)
items.Add(MinInstancesUserKey, *cc.MinInstances)
items.Add(MaxInstancesUserKey, *cc.MaxInstances)
Expand Down