Redis support #192

danawillow · 2018-05-12T00:53:31Z

Adds a new Redis resource and enables it in Terraform.

[all]

[terraform]

Redis resource

[puppet]

[puppet-dns]

[puppet-sql]

[puppet-compute]

[chef]

danawillow · 2018-05-12T01:00:18Z

templates/terraform/transport.go

+
+	queries := u.Query()
+	queries.Add("alt", "json")
+	newUrl := u.Scheme + "://" + u.Host + u.Path + "?" + queries.Encode()


note to self: https://github.com/GoogleCloudPlatform/magic-modules/pull/191/files does it better

modular-magician · 2018-05-12T01:06:06Z

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google#1485

rosbo · 2018-05-14T22:35:50Z

products/redis/api.yaml

+    input: true
+<%= indent(compile_file({}, 'templates/regional_async.yaml.erb'), 4) %>
+    parameters:
+      - !ruby/object:Api::Type::String # TODO: resourceref?


For Terraform, it doesn't really make a difference whether or not we have a Location resource defined in api.yaml, since we probably won't support it (perhaps as a data source, though if so that won't be for a while). But maybe the other providers would be better off with it! I wasn't sure, hence TODO.

rosbo · 2018-05-14T22:39:53Z

products/redis/api.yaml

+    self_link: projects/{{project}}/locations/{{region}}/instances/{{name}}
+    description: |
+      A Google Cloud Redis instance.
+    input: true


Add a reference block with the links to API documentation and guide:

references: !ruby/object:Api::Resource::ReferenceLinks guides: 'Reserving a Static External IP Address': 'https://cloud.google.com/compute/docs/ip-addresses/reserve-static-external-ip-address' api: 'https://cloud.google.com/compute/docs/reference/latest/globalAddresses'

rosbo · 2018-05-14T22:40:19Z

products/redis/api.yaml

+          and can change after a failover event.
+        output: true
+      - !ruby/object:Api::Type::String
+        name: displayName


No update method for this one?

It uses patch with updateMasks which I'll want to spend some time testing, so I figure no update support for now and then I'll do it in a follow-up PR.

rosbo · 2018-05-14T22:41:30Z

products/redis/terraform.yaml

+        required: false
+        default: !ruby/object:Provider::Terraform::Default
+          from_api: true
+      reservedIpRange: !ruby/object:Provider::Terraform::PropertyOverride


Do we want to see :BASIC as the default value for tier? And set required to false

Sure, done.

nelsonjr · 2018-05-15T21:58:03Z

products/redis/api.yaml

+        values:
+          - :BASIC
+          - :STANDARD_HA
+        required: true


new line at end?

nelsonjr · 2018-05-15T21:59:18Z

products/redis/api.yaml

+  - !ruby/object:Api::Resource
+    name: 'Instance'
+    exclude: true
+    base_url: |


A few things here:

The base_url is to provide a collection. You probably want self_link_query instead

Keep base_url: projects/{{project}}/locations/{{region}}/instances

Remove self_link

Can you explain what a collection is in this context?

The reason I did it this way is that in the Terraform templates, we use base_url (via collection_url) to create the resource, and then self_link (via self_link_url) to read, update, and delete it. For redis, the query params are in the create call, so we would call POST to projects/{{project}}/locations/{{region}}/instances?instanceId={{name}} to create it. Then once we want to read it though, it's a GET to projects/{{project}}/locations/{{region}}/instances/{{name}}. The way it is now outputs the correct URLs for the Terraform templates- would you change it to use custom code, or is this fine?

danawillow · 2018-05-17T00:26:40Z

Looking into rspec failures.

nat-henderson · 2018-05-17T00:52:07Z

I'm filling in for a robot who works on MagicModules PRs!

The robot tried to run tests, but:

+ go test -v ./google -parallel 16 -run '^Test' -timeout 1m
# github.com/terraform-providers/terraform-provider-google/google
google/transport.go:79:37: undefined: raw
google/transport.go:108:12: undefined: urllib
FAIL	github.com/terraform-providers/terraform-provider-google/google [build failed]

Unfortunately, the robot's not smart enough yet to complain when things don't work. That's on the roadmap!

modular-magician · 2018-05-17T22:57:03Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit f366eb5) have been included in your existing downstream PRs.

modular-magician · 2018-05-17T22:59:39Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit e4e52a8) have been included in your existing downstream PRs.

modular-magician · 2018-05-17T23:16:59Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 24cbb67) have been included in your existing downstream PRs.

modular-magician · 2018-05-17T23:25:35Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit c499376) have been included in your existing downstream PRs.

nat-henderson · 2018-05-18T17:45:47Z

templates/terraform/resource.html.markdown.erb

@@ -91,6 +91,7 @@ In addition to the arguments listed above, the following computed attributes are
 <%= lines(build_nested_property_documentation(prop)) -%>
 <% end -%>

+<% unless object.async.nil? -%>


Shoot, I forgot to do that on mine. Good catch.

modular-magician · 2018-05-21T20:22:25Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit b4102f8) have been included in your existing downstream PRs.

danawillow · 2018-05-21T20:31:14Z

All right, the generated code in Terraform is looking good from my perspective, so this is ready for real reviews.

nat-henderson

This LGTM, but you're going to need @nelsonjr to explain this self_link_query vs self_link vs collection situation. I agree this is right for what the templates do now, but the templates could be wrong, depending on what those things are meant to represent.

modular-magician · 2018-05-23T18:40:20Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit b3de14c) have been included in your existing downstream PRs.

Tracked submodules are build/puppet/compute build/puppet/sql build/terraform.

modular-magician · 2018-05-23T19:56:30Z

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 7fce509) have been included in your existing downstream PRs.

danawillow requested review from nat-henderson, nelsonjr, rosbo and rambleraptor May 12, 2018 00:53

danawillow commented May 12, 2018

View reviewed changes

modular-magician added the downstream-generated label May 12, 2018

rosbo suggested changes May 14, 2018

View reviewed changes

nelsonjr suggested changes May 15, 2018

View reviewed changes

danawillow added 3 commits May 16, 2018 16:26

initial redis support

75a413e

Merge branch 'master' into redis

86b5cc9

cleanup + review comments

40a38d5

danawillow force-pushed the redis branch from b14a2a2 to 40a38d5 Compare May 17, 2018 00:16

danawillow added 3 commits May 17, 2018 10:20

steal import_format idea from nathan

e82637d

acceptance tests + fixes

9e79f7a

add copyright notice

0bbf264

modular-magician added the downstream-generated label May 17, 2018

danawillow added 2 commits May 17, 2018 16:02

update redis example and fix timeouts in docs

050e94a

Merge branch 'master' into redis

f1f117a

danawillow added 5 commits May 17, 2018 16:48

Merge branch 'master' into redis

87f18f6

add redis to generate-terraform

a0f9727

Merge branch 'master' into redis

6d23ef3

use new default stuff

9b9b82c

update resourcemanager/terraform.yaml to use new flattener name

4df896c

nat-henderson reviewed May 18, 2018

View reviewed changes

danawillow force-pushed the redis branch from 0736607 to aeb8920 Compare May 21, 2018 19:05

only output timeouts in docs for async objects

0d3b79e

danawillow force-pushed the redis branch from aeb8920 to 0d3b79e Compare May 21, 2018 20:09

modular-magician added the downstream-generated label May 21, 2018

danawillow changed the title ~~WIP: initial redis support~~ Redis support May 21, 2018

nat-henderson approved these changes May 21, 2018

View reviewed changes

rosbo approved these changes May 23, 2018

View reviewed changes

danawillow added 2 commits May 23, 2018 11:28

Merge branch 'master' into redis

361e6c7

Merge 361e6c7 into 58ae517

b3de14c

modular-magician added the downstream-generated label May 23, 2018

Update tracked submodules -> HEAD on Wed May 23 19:45:36 UTC 2018

03f74ca

Tracked submodules are build/puppet/compute build/puppet/sql build/terraform.

modular-magician merged commit c507db5 into master May 23, 2018

danawillow deleted the redis branch May 23, 2018 19:47

modular-magician added the downstream-generated label May 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redis support #192

Redis support #192

danawillow commented May 12, 2018 •

edited

Loading

danawillow May 12, 2018

modular-magician commented May 12, 2018

rosbo May 14, 2018

danawillow May 17, 2018

rosbo May 14, 2018

danawillow May 17, 2018

rosbo May 14, 2018

danawillow May 17, 2018

rosbo May 14, 2018

danawillow May 17, 2018

nelsonjr May 15, 2018

danawillow May 17, 2018

nelsonjr May 15, 2018

danawillow May 17, 2018

danawillow commented May 17, 2018

nat-henderson commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

nat-henderson May 18, 2018

modular-magician commented May 21, 2018

danawillow commented May 21, 2018

nat-henderson left a comment

modular-magician commented May 23, 2018

modular-magician commented May 23, 2018

Redis support #192

Redis support #192

Conversation

danawillow commented May 12, 2018 • edited Loading

[all]

[terraform]

[puppet]

[puppet-dns]

[puppet-sql]

[puppet-compute]

[chef]

Choose a reason for hiding this comment

modular-magician commented May 12, 2018

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

danawillow commented May 17, 2018

nat-henderson commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

modular-magician commented May 17, 2018

Choose a reason for hiding this comment

modular-magician commented May 21, 2018

danawillow commented May 21, 2018

nat-henderson left a comment

Choose a reason for hiding this comment

modular-magician commented May 23, 2018

modular-magician commented May 23, 2018

danawillow commented May 12, 2018 •

edited

Loading