Skip to content
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

Add fields to InterconnectAttachment to allow for PARTNER interconnects #1323

Merged

Conversation

cblecker
Copy link
Contributor

This is my first magic-modules PR -- Please let me know if there are additional steps I should take.
ref: hashicorp/terraform-provider-google#2942
cc: @ndmckinley


[all]

[terraform]

[terraform-beta]

[ansible]

[inspec]

@nat-henderson nat-henderson self-requested a review January 30, 2019 00:48
@nat-henderson nat-henderson self-assigned this Jan 30, 2019
@nat-henderson
Copy link
Contributor

Looks great. Let me go ahead and get the downstreams generated and we'll take a look!

@modular-magician
Copy link
Collaborator

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-beta#394
depends: hashicorp/terraform-provider-google#2959
depends: modular-magician/ansible#180

@cblecker
Copy link
Contributor Author

@ndmckinley I actually found another output status field that's needed. Can I just update this PR?

@cblecker cblecker force-pushed the partner-interconnect branch from c231d05 to c4a293f Compare January 30, 2019 02:03
@cblecker
Copy link
Contributor Author

I reviewed the API spec for the enum fields type and spec, and updated with the full list of options.

@nat-henderson
Copy link
Contributor

Yep, go ahead and update the PR! I'll just have to do a quick review of each successive commit - just a quick glance.

@nat-henderson
Copy link
Contributor

Ah, you see that failure under checks there? Here's the error message.

bundler: failed to load command: compiler (compiler)
Psych::SyntaxError: (<unknown>): did not find expected '-' indicator while parsing a block collection at line 4238 column 7
  /root/.rbenv/versions/2.6.0/lib/ruby/2.6.0/psych.rb:456:in `parse'
  /root/.rbenv/versions/2.6.0/lib/ruby/2.6.0/psych.rb:456:in `parse_stream'
  /root/.rbenv/versions/2.6.0/lib/ruby/2.6.0/psych.rb:390:in `parse'
  /root/.rbenv/versions/2.6.0/lib/ruby/2.6.0/psych.rb:349:in `safe_load'
  /tmp/build/fb9bd761/magic-modules-branched/google/yaml_validator.rb:25:in `parse'
  /tmp/build/fb9bd761/magic-modules-branched/api/compiler.rb:35:in `run'
  compiler:117:in `block in <top (required)>'
  compiler:101:in `each'
  compiler:101:in `<top (required)>'

You should be able to reproduce locally - do you get the same result?

@cblecker cblecker force-pushed the partner-interconnect branch from c4a293f to a2b1d44 Compare January 30, 2019 02:51
@cblecker
Copy link
Contributor Author

@ndmckinley Oops! yeah, indentation issue. Fixed and tested locally!

@nat-henderson
Copy link
Contributor

On its way through the pipeline! Watch the status indicators below - should show up under concourse-ci/* - if you see any failures, the "details" link should show you the Concourse CI logs. There's a chance you'll have access issues, but if that happens, let me know.

@modular-magician
Copy link
Collaborator

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

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

@cblecker
Copy link
Contributor Author

Everything looks green from here :)

@nat-henderson
Copy link
Contributor

These changes look great to me! I don't have access to an interconnect - could you post the debug logs of creating one of each type of interconnect attachment? If that looks right, I'll approve and this will be out in the next release.

@cblecker
Copy link
Contributor Author

@ndmckinley I unfortunately don't have a dedicated interconnect either, so I can't set up that type. I have access to a partner interconnect, but can only do the first half of setup for it to go into a pending state, as the second half of setup requires third-party vendor action.

@nat-henderson
Copy link
Contributor

Bummer! Let me go find someone who's got one.

@nat-henderson
Copy link
Contributor

Okay, I do have access to someone who's got access to a dedicated interconnect - if you can test the first part of the partner interconnect, by our powers combined (:globe_with_meridians:), we can merge this change.

@cblecker
Copy link
Contributor Author

Okay, so I created these two objects:

resource "google_compute_router" "router1" {
  name    = "rooter-1"
  network = "default"

  bgp {
    asn            = 16550
    advertise_mode = "CUSTOM"

    advertised_ip_ranges {
      range = "10.0.0.0/8"
    }
  }
}

resource "google_compute_interconnect_attachment" "vlan1" {
  name                     = "vlan1"
  type                     = "PARTNER"
  router                   = "${google_compute_router.router1.name}"
  edge_availability_domain = "AVAILABILITY_DOMAIN_1"
}

I checked out and compiled hashicorp/terraform-provider-google@75909d7, then used it to terraform apply those two objects.

Debug log is here: https://gist.github.com/cblecker/f7bfe8030d47035a401564f0514d57eb

I've modified the log slightly to substitute the GCP project name, and my google account name from the log.

@nat-henderson
Copy link
Contributor

Looks great! I'll wait for mine to get back to me and we'll see what we can do. Thank you!

@rambleraptor
Copy link
Contributor

(The Ansible downstream looks good to me!)

@nat-henderson
Copy link
Contributor

Okay, almost there! You just need to set type to Optional and Computed, by adding the default_from_api: true flag to terraform.yaml. Then I can merge it.

@cblecker cblecker force-pushed the partner-interconnect branch from a2b1d44 to c0f4975 Compare February 1, 2019 23:53
@cblecker
Copy link
Contributor Author

cblecker commented Feb 1, 2019

@ndmckinley Done!

@modular-magician
Copy link
Collaborator

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

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

cblecker and others added 2 commits February 2, 2019 00:21
@modular-magician modular-magician merged commit 0ed1edd into GoogleCloudPlatform:master Feb 2, 2019
@nat-henderson
Copy link
Contributor

Great! Well done, thanks for your contribution.

@cblecker cblecker deleted the partner-interconnect branch February 2, 2019 01:47
@cblecker
Copy link
Contributor Author

cblecker commented Feb 2, 2019

Thanks for the help in getting this in!

One last question: Do you happen to know when the next expected cut of the terraform google provider will be?

@nat-henderson
Copy link
Contributor

We're working that out now, but if it's more than 2 weeks from now, I'd be surprised. :)

@cblecker
Copy link
Contributor Author

cblecker commented Feb 2, 2019

Sounds great! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants