-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
adds scaling properties to appengine standard version #3373
Changes from all commits
5192d81
ea22d22
7a7c139
8bc74f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,8 +57,9 @@ overrides: !ruby/object:Overrides::ResourceOverrides | |
ignore_read: true | ||
threadsafe: !ruby/object:Overrides::Terraform::PropertyOverride | ||
ignore_read: true | ||
# instanceClass defaults to a value based on the scaling method | ||
instanceClass: !ruby/object:Overrides::Terraform::PropertyOverride | ||
ignore_read: true | ||
default_from_api: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keeping the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm, I'd be curious as to why Generally default_from_api does make sense in this case, as it marks the field as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure what is happening with
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also figured that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that actually makes sense, and The step that the test is failing on is an import test, which imports the resource from the existing resource in GCP. Because There was no way for this to work during import before |
||
examples: | ||
- !ruby/object:Provider::Terraform::Examples | ||
name: "app_engine_standard_app_version" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,20 @@ resource "google_app_engine_standard_app_version" "<%= ctx[:primary_resource_id] | |
port = "8080" | ||
} | ||
|
||
automatic_scaling { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you throw as many of these fields as possible in here so they are tested? Preferably we would make a new handwritten test file similar to this one: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/tests/resource_app_engine_application_test.go Where we could add some of these fields, then perform an update to either change the scaling mode and update some of these values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added all the scaling fields to this test.
Never mind; I got the integration test working locally by temporarily setting the This new test started as a copy of
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome, thanks for all the work! This guide has info on setting the org_id and billing account as environment variables: https://github.com/terraform-providers/terraform-provider-google/blob/master/.github/CONTRIBUTING.md#tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had already tried to set the env vars
I also tried setting GOOGLE_ORG to the numeric id of our organisation, but that throws the same error. Anyhow; it does work on your CI servers so it must be something I'm doing wrong (or missing from CONTRIBUTING.md) but it doesn't seem relevant to this PR. |
||
max_concurrent_requests = 10 | ||
min_idle_instances = 1 | ||
max_idle_instances = 3 | ||
min_pending_latency = "1s" | ||
max_pending_latency = "5s" | ||
standard_scheduler_settings { | ||
target_cpu_utilization = 0.5 | ||
target_throughput_utilization = 0.75 | ||
min_instances = 2 | ||
max_instances = 10 | ||
} | ||
} | ||
|
||
delete_service_on_destroy = true | ||
} | ||
|
||
|
@@ -39,6 +53,10 @@ resource "google_app_engine_standard_app_version" "myapp_v2" { | |
port = "8080" | ||
} | ||
|
||
basic_scaling { | ||
max_instances = 5 | ||
} | ||
|
||
noop_on_destroy = true | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,200 @@ | ||
package google | ||
|
||
import ( | ||
"github.com/hashicorp/terraform-plugin-sdk/helper/resource" | ||
"testing" | ||
) | ||
|
||
func TestAccAppEngineStandardAppVersion_update(t *testing.T) { | ||
t.Parallel() | ||
|
||
context := map[string]interface{}{ | ||
"org_id": getTestOrgFromEnv(t), | ||
"billing_account": getTestBillingAccountFromEnv(t), | ||
"random_suffix": randString(t, 10), | ||
} | ||
|
||
vcrTest(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccCheckAppEngineStandardAppVersionDestroyProducer(t), | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: testAccAppEngineStandardAppVersion_python(context), | ||
}, | ||
{ | ||
ResourceName: "google_app_engine_standard_app_version.foo", | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
ImportStateVerifyIgnore: []string{"env_variables", "deployment", "entrypoint", "service", "noop_on_destroy"}, | ||
}, | ||
{ | ||
Config: testAccAppEngineStandardAppVersion_pythonUpdate(context), | ||
}, | ||
{ | ||
ResourceName: "google_app_engine_standard_app_version.foo", | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
ImportStateVerifyIgnore: []string{"env_variables", "deployment", "entrypoint", "service", "noop_on_destroy"}, | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccAppEngineStandardAppVersion_python(context map[string]interface{}) string { | ||
return Nprintf(` | ||
resource "google_project" "my_project" { | ||
name = "tf-test-appeng-std%{random_suffix}" | ||
project_id = "tf-test-appeng-std%{random_suffix}" | ||
org_id = "%{org_id}" | ||
billing_account = "%{billing_account}" | ||
} | ||
|
||
resource "google_app_engine_application" "app" { | ||
project = google_project.my_project.project_id | ||
location_id = "us-central" | ||
} | ||
|
||
resource "google_project_service" "project" { | ||
project = google_project.my_project.project_id | ||
service = "appengine.googleapis.com" | ||
|
||
disable_dependent_services = false | ||
} | ||
|
||
resource "google_app_engine_standard_app_version" "foo" { | ||
project = google_project_service.project.project | ||
version_id = "v1" | ||
service = "default" | ||
runtime = "python38" | ||
|
||
entrypoint { | ||
shell = "gunicorn -b :$PORT main:app" | ||
} | ||
|
||
deployment { | ||
files { | ||
name = "main.py" | ||
source_url = "https://storage.googleapis.com/${google_storage_bucket.bucket.name}/${google_storage_bucket_object.main.name}" | ||
} | ||
|
||
files { | ||
name = "requirements.txt" | ||
source_url = "https://storage.googleapis.com/${google_storage_bucket.bucket.name}/${google_storage_bucket_object.requirements.name}" | ||
} | ||
} | ||
|
||
env_variables = { | ||
port = "8000" | ||
} | ||
|
||
instance_class = "F2" | ||
|
||
automatic_scaling { | ||
max_concurrent_requests = 10 | ||
min_idle_instances = 1 | ||
max_idle_instances = 3 | ||
min_pending_latency = "1s" | ||
max_pending_latency = "5s" | ||
standard_scheduler_settings { | ||
target_cpu_utilization = 0.5 | ||
target_throughput_utilization = 0.75 | ||
min_instances = 2 | ||
max_instances = 10 | ||
} | ||
} | ||
|
||
noop_on_destroy = true | ||
} | ||
|
||
resource "google_storage_bucket" "bucket" { | ||
project = google_project.my_project.project_id | ||
name = "tf-test-%{random_suffix}-standard-ae-bucket" | ||
} | ||
|
||
resource "google_storage_bucket_object" "requirements" { | ||
name = "requirements.txt" | ||
bucket = google_storage_bucket.bucket.name | ||
source = "./test-fixtures/appengine/hello-world-flask/requirements.txt" | ||
} | ||
|
||
resource "google_storage_bucket_object" "main" { | ||
name = "main.py" | ||
bucket = google_storage_bucket.bucket.name | ||
source = "./test-fixtures/appengine/hello-world-flask/main.py" | ||
}`, context) | ||
} | ||
|
||
func testAccAppEngineStandardAppVersion_pythonUpdate(context map[string]interface{}) string { | ||
return Nprintf(` | ||
resource "google_project" "my_project" { | ||
name = "tf-test-appeng-std%{random_suffix}" | ||
project_id = "tf-test-appeng-std%{random_suffix}" | ||
org_id = "%{org_id}" | ||
billing_account = "%{billing_account}" | ||
} | ||
|
||
resource "google_app_engine_application" "app" { | ||
project = google_project.my_project.project_id | ||
location_id = "us-central" | ||
} | ||
|
||
resource "google_project_service" "project" { | ||
project = google_project.my_project.project_id | ||
service = "appengine.googleapis.com" | ||
|
||
disable_dependent_services = false | ||
} | ||
|
||
resource "google_app_engine_standard_app_version" "foo" { | ||
project = google_project_service.project.project | ||
version_id = "v1" | ||
service = "default" | ||
runtime = "python38" | ||
|
||
entrypoint { | ||
shell = "gunicorn -b :$PORT main:app" | ||
} | ||
|
||
deployment { | ||
files { | ||
name = "main.py" | ||
source_url = "https://storage.googleapis.com/${google_storage_bucket.bucket.name}/${google_storage_bucket_object.main.name}" | ||
} | ||
|
||
files { | ||
name = "requirements.txt" | ||
source_url = "https://storage.googleapis.com/${google_storage_bucket.bucket.name}/${google_storage_bucket_object.requirements.name}" | ||
} | ||
} | ||
|
||
env_variables = { | ||
port = "8000" | ||
} | ||
|
||
instance_class = "B2" | ||
|
||
basic_scaling { | ||
max_instances = 5 | ||
} | ||
|
||
noop_on_destroy = true | ||
} | ||
|
||
resource "google_storage_bucket" "bucket" { | ||
project = google_project.my_project.project_id | ||
name = "tf-test-%{random_suffix}-standard-ae-bucket" | ||
} | ||
|
||
resource "google_storage_bucket_object" "requirements" { | ||
name = "requirements.txt" | ||
bucket = google_storage_bucket.bucket.name | ||
source = "./test-fixtures/appengine/hello-world-flask/requirements.txt" | ||
} | ||
|
||
resource "google_storage_bucket_object" "main" { | ||
name = "main.py" | ||
bucket = google_storage_bucket.bucket.name | ||
source = "./test-fixtures/appengine/hello-world-flask/main.py" | ||
}`, context) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried this myself, looks like leaving it blank returns an API error stating it is required. Thanks for the update