New Resource: aws_gamelift_fleet #3327

radeksimko · 2018-02-12T11:20:39Z

Test results

=== RUN   TestAccAWSGameliftFleet_basic
--- PASS: TestAccAWSGameliftFleet_basic (1111.72s)
=== RUN   TestAccAWSGameliftFleet_allFields
--- PASS: TestAccAWSGameliftFleet_allFields (1262.53s)

Related: #2546

bflad

Left some initial comments -- let me know if you have any questions! 🚀

bflad · 2018-02-12T11:35:35Z

aws/resource_aws_gamelift_build_test.go

 // Location found from CloudTrail event after finishing tutorial
 // e.g. https://us-west-2.console.aws.amazon.com/gamelift/home?region=us-west-2#/r/fleets/sample
-func testAccAWSGameliftSampleGameLocation(region string) (*gamelift.S3Location, error) {
+func testAccAWSGameliftSampleGameLocation(region string) (*testAccGameliftGame, error) {


Minor nitpick: Maybe this function should be renamed without Location (since it has more delicious information) and optionally moved to a "top level" file aws/resource_aws_gamelift_test.go since it spans multiple resources?

bflad · 2018-02-12T11:38:29Z

aws/resource_aws_gamelift_fleet.go

+							Type:     schema.TypeString,
+							Required: true,
+							ValidateFunc: validation.StringInSlice([]string{
+								"TCP", "UDP",


Nitpick: You'll be happy to know there are SDK constants for these 😛

gamelift.IpProtocolTcp, gamelift.IpProtocolUdp

bflad · 2018-02-12T11:45:41Z

aws/resource_aws_gamelift_fleet.go

+	}
+
+	log.Printf("[INFO] Creating Gamelift Fleet: %s", input)
+	out, err := conn.CreateFleet(&input)


I'm glad if we haven't run into eventual consistency issues with BuildId 😄

The build must have been successfully uploaded to Amazon GameLift and be in a READY status.

bflad · 2018-02-12T11:49:02Z

aws/resource_aws_gamelift_fleet.go

+			gamelift.FleetStatusNew,
+			gamelift.FleetStatusValidating,
+		},
+		Target:  []string{gamelift.FleetStatusActive},


Nitpick: Should we try to catch gamelift.FleetStatusError as a target or in the refresh function to put a better message than the Terraform unexpected state one?

So I think we provide as much details as we can in such case. I'm intentionally not catching FleetStatusError and let it fall through as error, so I can catch it below and ask for reasons of the state.

We also save some API calls that way since we don't ask for failures with each retry, but only when we know things have failed.

Okay that makes sense, and is pretty awesome. 👍

bflad · 2018-02-12T11:49:55Z

aws/resource_aws_gamelift_fleet.go

+			log.Printf("[WARN] Failed to poll fleet failures: %s", fErr)
+		}
+		if len(events) > 0 {
+			return fmt.Errorf("%s Recent failures:\n%+v", err, events)


This is pretty awesome 👍

bflad · 2018-02-12T12:29:42Z

aws/resource_aws_gamelift_fleet_test.go

+				Config: testAccAWSGameliftFleetBasicConfig(fleetName, launchPath, params, buildName, bucketName, key, roleArn),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSGameliftFleetExists("aws_gamelift_fleet.test", &conf),
+					resource.TestCheckResourceAttrSet("aws_gamelift_fleet.test", "build_id"),


Missing arn attribute check:

resource.TestMatchResourceAttr("aws_gamelift_fleet.test", "arn", regexp.MustCompile(`^arn:[^:]+:gamelift:[^:]+:[^:]+:.+$`))

bflad · 2018-02-12T12:34:08Z

aws/resource_aws_gamelift_fleet_test.go

+
+		attributes := out.FleetAttributes
+
+		if len(attributes) > 0 {


Do we need to check its status for gamelift.FleetStatusDeleting or gamelift.FleetStatusTerminated?

waitForGameliftFleetToBeDeleted should guarantee that we don't as the fleet should be completely gone by this time. If it's not I'd rather treat that as bug.

Ah sorry, I missed this when looking at waitForGameliftFleetToBeDeleted - we're going for zero returned results instead of just gamelift.FleetStatusTerminated status. I thought you'd be getting the one result with the terminated status 👍

bflad · 2018-02-12T12:36:49Z

website/docs/r/gamelift_fleet.html.markdown

+* `name` - (Required) The name of the fleet.
+* `description` - (Optional) Human-readable description of the fleet.
+* `ec2_inbound_permission` - (Optional) Range of IP addresses and port settings that permit inbound traffic to access server processes running on the fleet. See below.
+* `metric_groups` - (Optional) Name of a metric group to add this fleet to. A metric group tracks metrics across all fleets in the group. Defaults to `default`.


List of names to match schema and API, but max of one 👍

bflad · 2018-02-12T12:37:37Z

website/docs/r/gamelift_fleet.html.markdown

+* `ec2_inbound_permission` - (Optional) Range of IP addresses and port settings that permit inbound traffic to access server processes running on the fleet. See below.
+* `metric_groups` - (Optional) Name of a metric group to add this fleet to. A metric group tracks metrics across all fleets in the group. Defaults to `default`.
+* `new_game_session_protection_policy` - (Optional) Game session protection policy to apply to all instances in this fleet. e.g. `FullProtection`. Defaults to `NoProtection`.
+* `resource_creation_limit_policy` - (Optional) Policy that limits the number of game sessions an individual player can create over a span of time for this fleet. sSee below.


Typo 🕶 : sSee

bflad · 2018-02-12T12:39:42Z

aws/resource_aws_gamelift_fleet.go

+							Required:     true,
+							ValidateFunc: validation.IntBetween(1, 60000),
+						},
+						"ip_range": {


I think we can validate this with ValidateFunc: validateCIDRNetworkAddress,

bflad · 2018-02-12T19:19:12Z

Let me know when you'd like me to give another look! 👀

radeksimko · 2018-02-12T21:03:36Z

@bflad PTAL

Hinidu · 2018-02-13T09:02:20Z

Do you plan to implement fleet autoscaling policies? It is essential part of the fleet for us and many other developers.

radeksimko · 2018-02-13T09:33:25Z

@Hinidu supporting autoscaling policy sounds like a good idea, although it should certainly be a separate resource, e.g. aws_gamelift_autoscaling_policy which maps to these API methods:

It's not on my near-term plan though, so I think we'd be happy to accept PRs for such resource.

Hinidu · 2018-02-13T09:41:29Z

@radeksimko got it. I hope I will find some time to make these PRs 🙂

bflad

This looks great! 🚀 🚢 ⛵️

bflad · 2018-02-27T12:41:33Z

This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-07T17:55:15Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

New Resource: aws_gamelift_fleet

ab40320

radeksimko added new-resource Introduces a new resource. service/gamelift Issues and PRs that pertain to the gamelift service. labels Feb 12, 2018

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Feb 12, 2018

radeksimko mentioned this pull request Feb 12, 2018

Amazon GameLift support #2546

Closed

bflad self-requested a review February 12, 2018 11:27

bflad added this to the v1.10.0 milestone Feb 12, 2018

bflad reviewed Feb 12, 2018

View reviewed changes

Reflect review

94e786f

ghost added the size/XXL Managed by automation to categorize the size of a PR. label Feb 12, 2018

bflad approved these changes Feb 13, 2018

View reviewed changes

radeksimko merged commit b1a437e into master Feb 13, 2018

radeksimko deleted the f-gamelift-fleet branch February 13, 2018 13:49

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_gamelift_fleet #3327

New Resource: aws_gamelift_fleet #3327

radeksimko commented Feb 12, 2018 •

edited

Loading

bflad left a comment

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

radeksimko Feb 12, 2018

bflad Feb 13, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

radeksimko Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad commented Feb 12, 2018

radeksimko commented Feb 12, 2018

Hinidu commented Feb 13, 2018

radeksimko commented Feb 13, 2018

Hinidu commented Feb 13, 2018

bflad left a comment

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

New Resource: aws_gamelift_fleet #3327

New Resource: aws_gamelift_fleet #3327

Conversation

radeksimko commented Feb 12, 2018 • edited Loading

Test results

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad commented Feb 12, 2018

radeksimko commented Feb 12, 2018

Hinidu commented Feb 13, 2018

radeksimko commented Feb 13, 2018

Hinidu commented Feb 13, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

radeksimko commented Feb 12, 2018 •

edited

Loading