search

hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.33k stars 1.73k forks source link

google_compute_router_peer peer_asn and advertise_mode forcing replacement #7977

Closed tnguyen14 closed 3 years ago

tnguyen14 commented 3 years ago

Community Note

Terraform Version

0.12.29

Affected Resource(s)

Terraform Configuration Files

# Copy-paste your Terraform configurations here.
#
# For large Terraform configs, please use a service like Dropbox and share a link to the ZIP file.
# For security, you can also encrypt the files using our GPG public key:
#    https://www.hashicorp.com/security
#
# If reproducing the bug involves modifying the config file (e.g., apply a config,
# change a value, apply the config again, see the bug), then please include both:
# * the version of the config before the change, and
# * the version of the config after the change.

Debug Output

Panic Output

Expected Behavior

We have BGP peers that are of type "MANAGED_BY_ATTACHMENT". We use terraform import to get them managed by Terraform.

On the GCP console, the "Advertisement Mode" for the peers are shown as "Custom", however, when imported, the terraform state has the attribute advertise_mode as DEFAULT.

When we run terraform plan, the output shows that the advertise_mode needs to be changed from DEFAULT to CUSTOM (as the terraform configuration file expects), and that the peer_asn needs to be updated as well (also as the terraform configuration file dictates).

The changes on these 2 attributes indicate that they would force a replacement of the BGP peer resource.

I don't think that changing these attributes should force a replacement. In the GCP console, I could go in and update these attributes inline without having to delete those resources.

Actual Behavior

Steps to Reproduce

  1. terraform apply

Important Factoids

References

slevenick commented 3 years ago

Hey @tnguyen14 I would be curious why advertise_mode is not being read correctly when you import it.

Are you able to include debug logs for your import & plan afterwards?

Otherwise, adding support for updating peer_asn and advertise_mode would be a feature request and I can add it to the list to be triaged

tnguyen14 commented 3 years ago

@slevenick thanks for the response. I would say go with the feature request for now. I will open a separate ticket for the import issue with more debug logs.

tnguyen14 commented 3 years ago

@slevenick I created https://github.com/hashicorp/terraform-provider-google/issues/8009 for the import issue

rileykarson commented 3 years ago

advertisedGroups mentions:

Note that this field can only be populated if advertiseMode is CUSTOM and overrides the list defined for the router (in the "bgp" message). These groups are advertised in addition to any specified prefixes. Leave this field blank to advertise no custom groups.

advertiseMode's documentation is a little sparse, it only indicates:

User-specified flag to indicate which mode to use for advertisement.

What I suspect is that the same is true for this field as well, and that the bgp settings override those in the peer. That makes a past assumption of ours- that no setting returned means "DEFAULT"- is false. Note that default doesn't mean "default for the router" here, it's the default advertisement mode

The good news: You can ignore this value. Terraform deciding it has a value of "DEFAULT" is entirely clientside, and the field is immutable (eg. changing the value recreates the resource) and as-implemented Terraform is unable to send a bad update. It's completely benign to ignore- I would suggest setting lifecycle.ignore_changes on the field.

We'll still want to investigate and fix this behaviour, we'll want to confirm what we do with the service team.

ScottSuarez commented 3 years ago

Note that default doesn't mean "default for the router" here, it's the default advertisement mode

Default seems to imply inherited from cloud router. 

Each BGP session for a Cloud Router also has a default advertisement. By default, Cloud Router propagates its route advertisements to all of its BGP sessions. If you configure custom route advertisements on a Cloud Router, its BGP sessions inherit those custom advertisements.

The current behavior looks okay to me. We only need to look into seeing if a bpgpeer's AdvertiseMode and AdvertisedGroup is changeable.

From a dev on the Virtual router team. That is, if bgp.advertiseMode is "custom" and bgppeer.advertiseMode is "default", then the custom advertisements from bgp will be applied to the bgppeer.

rileykarson commented 3 years ago

Thanks for looking into this!

tnguyen14 commented 3 years ago

The current behavior looks okay to me.

@ScottSuarez are you saying that it's okay to force replacement if peer_asn and advertise_mode change?

ScottSuarez commented 3 years ago

The current behavior looks okay to me.

@ScottSuarez are you saying that it's okay to force replacement if peer_asn and advertise_mode change?

No I was referring to behavior of DEFAULT being set when bpg.advertisedMode is CUSTOM.

I investigated the updatable fields and I was able to confirm that advertiseMode, advertisedGroups, peerAsn, and peerIpAddress are able to be updated. I've put out a pr to enable you do so. Thanks for your help here @tnguyen14 .

tnguyen14 commented 3 years ago

Does anyone know when the change merged in #8862 will be released?

ScottSuarez commented 3 years ago

April 19th it will be released as apart of 3.65.0

ghost commented 3 years ago

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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!