Skip to content

Commit

Permalink
Remove default values for runConfig fields in publisher. The Control …
Browse files Browse the repository at this point in the history
…Plane will set defaults for any unset fields.

PiperOrigin-RevId: 669336914
Change-Id: I403a9c5fa943460682edc25dff804264b64cc46b
  • Loading branch information
abhis3 authored and copybara-github committed Aug 30, 2024
1 parent f2a2913 commit ba1567a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 56 deletions.
42 changes: 5 additions & 37 deletions pkg/firebase/publisher/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,6 @@ type buildSchema struct {
Env []apphostingschema.EnvironmentVariable `yaml:"env,omitempty"`
}

var (
defaultCPU int32 = 1 // From https://cloud.google.com/run/docs/configuring/services/cpu.
defaultMemory int32 = 512 // From https://cloud.google.com/run/docs/configuring/services/memory-limits.
defaultConcurrency int32 = 80 // From https://cloud.google.com/run/docs/about-concurrency.
defaultMaxInstances int32 = 100 // From https://cloud.google.com/run/docs/configuring/max-instances.
defaultMinInstances int32 = 0 // From https://cloud.google.com/run/docs/configuring/min-instances.
)

func readBundleSchemaFromFile(filePath string) (outputBundleSchema, error) {
bundleBuffer, err := os.ReadFile(filePath)
if os.IsNotExist(err) {
Expand All @@ -69,7 +61,7 @@ func writeToFile(buildSchema buildSchema, outputFilePath string) error {
if err != nil {
return fmt.Errorf("converting struct to YAML: %w", err)
}
log.Printf("Final build schema:\n%v\n", string(fileData))
log.Printf("Final build schema:\n%v\n. Note that any unset runConfig fields will be set to reasonable default values.", string(fileData))

err = os.MkdirAll(filepath.Dir(outputFilePath), os.ModeDir)
if err != nil {
Expand All @@ -91,34 +83,10 @@ func writeToFile(buildSchema buildSchema, outputFilePath string) error {
}

func toBuildSchema(schema apphostingschema.AppHostingSchema, bundleSchema outputBundleSchema) buildSchema {
dCPU := float32(defaultCPU)
buildSchema := buildSchema{
RunConfig: &apphostingschema.RunConfig{
CPU: &dCPU,
MemoryMiB: &defaultMemory,
Concurrency: &defaultConcurrency,
MaxInstances: &defaultMaxInstances,
MinInstances: &defaultMinInstances,
},
}
// Copy RunConfig fields from apphosting.yaml.
b := schema.RunConfig
if b.CPU != nil {
cpu := float32(*b.CPU)
buildSchema.RunConfig.CPU = &cpu
}
if b.MemoryMiB != nil {
buildSchema.RunConfig.MemoryMiB = b.MemoryMiB
}
if b.Concurrency != nil {
buildSchema.RunConfig.Concurrency = b.Concurrency
}
if b.MaxInstances != nil {
buildSchema.RunConfig.MaxInstances = b.MaxInstances
}
if b.MinInstances != nil {
buildSchema.RunConfig.MinInstances = b.MinInstances
}
buildSchema := buildSchema{}

// Copy RunConfig fields from apphosting.yaml, Control Plane will set defaults for any unset fields.
buildSchema.RunConfig = &schema.RunConfig

// Copy Env fields from apphosting.yaml
if len(schema.Env) > 0 {
Expand Down
23 changes: 4 additions & 19 deletions pkg/firebase/publisher/publisher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ func TestPublish(t *testing.T) {
desc: "Handle nonexistent apphosting.yaml",
appHostingYAMLFilePath: "nonexistent",
wantBuildSchema: buildSchema{
RunConfig: &apphostingschema.RunConfig{
CPU: proto.Float32(1),
MemoryMiB: proto.Int32(512),
Concurrency: proto.Int32(80),
MaxInstances: proto.Int32(100),
MinInstances: proto.Int32(0),
},
RunConfig: &apphostingschema.RunConfig{},
}},
}

Expand Down Expand Up @@ -122,13 +116,7 @@ func TestToBuildSchemaRunConfig(t *testing.T) {
desc: "Empty AppHostingSchema",
inputAppHostingSchema: apphostingschema.AppHostingSchema{},
expectedSchema: buildSchema{
RunConfig: &apphostingschema.RunConfig{
CPU: proto.Float32(float32(defaultCPU)),
MemoryMiB: &defaultMemory,
Concurrency: &defaultConcurrency,
MaxInstances: &defaultMaxInstances,
MinInstances: proto.Int32(0),
},
RunConfig: &apphostingschema.RunConfig{},
},
},
{
Expand Down Expand Up @@ -174,11 +162,8 @@ func TestToBuildSchemaRunConfig(t *testing.T) {
},
expectedSchema: buildSchema{
RunConfig: &apphostingschema.RunConfig{
CPU: proto.Float32(1000),
MemoryMiB: &defaultMemory,
Concurrency: proto.Int32(2),
MaxInstances: &defaultMaxInstances,
MinInstances: proto.Int32(0),
CPU: proto.Float32(1000),
Concurrency: proto.Int32(2),
},
Env: []apphostingschema.EnvironmentVariable{
apphostingschema.EnvironmentVariable{Variable: "API_URL", Value: "api.service.com", Availability: []string{"BUILD", "RUNTIME"}},
Expand Down

0 comments on commit ba1567a

Please sign in to comment.