-
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
Add ability to set global network endpoint group as backend #3764
Add ability to set global network endpoint group as backend #3764
Conversation
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 137 insertions(+), 24 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134590" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_withBackend|TestAccComputeBackendService_withBackendAndIAP|TestAccComputeBackendService_backendServiceNetworkEndpointExample|TestAccComputeBackendService_withMaxConnections|TestAccComputeBackendService_withMaxConnectionsPerInstance|TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccComputeBackendService_internalLoadBalancing|TestAccComputeInstanceGroup_rename|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccContainerCluster_backend|TestAccFolderIamAuditConfig_multiple|TestAccServiceAccountIamMember_withAndWithoutCondition You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134593" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 121 insertions(+), 38 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134596" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 120 insertions(+), 38 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134597" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceComputeNetworkEndpointGroup|TestAccDataSourceGoogleNetwork|TestAccDataSourceComputeRouter|TestAccDataSourceGoogleSubnetwork|TestAccComputeSubnetworkIamBindingGenerated|TestAccComputeSubnetworkIamMemberGenerated|TestAccComputeSubnetworkIamPolicyGenerated|TestAccCloudFunctionsFunction_vpcConnector|TestAccComposerEnvironment_basic|TestAccComposerEnvironment_update|TestAccComposerEnvironment_private|TestAccComposerEnvironment_withNodeConfig|TestAccComposerEnvironment_withUpdateOnCreate|TestAccComposerEnvironment_withSoftwareConfig|TestAccComputeAddress_addressWithSubnetworkExample|TestAccComputeAddress_internal|TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccComputeFirewall_firewallBasicExample|TestAccComputeFirewall_update|TestAccComputeFirewall_priority|TestAccComputeFirewall_noSource|TestAccComputeFirewall_denied|TestAccComputeFirewall_disabled|TestAccComputeFirewall_serviceAccounts|TestAccComputeFirewall_egress|TestAccComputeFirewall_enableLogging|TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample|TestAccComputeForwardingRule_forwardingRuleInternallbExample|TestAccComputeGlobalAddress_internal|TestAccComputeInstanceGroup_network|TestAccComputeInstanceTemplate_subnet_auto|TestAccComputeInstanceTemplate_subnet_custom|TestAccComputeInstanceTemplate_secondaryAliasIpRange|TestAccComputeInstance_subnet_auto|TestAccComputeInstance_subnet_custom|TestAccComputeInstance_network_ip_custom|TestAccComputeInstance_networkIPAuto|TestAccComputeInstance_multiNic|TestAccComputeInstance_secondaryAliasIpRange|TestAccComputeNetworkEndpointGroup_networkEndpointGroupExample|TestAccComputeNetwork_networkBasicExample|TestAccComputeNetworkPeeringRoutesConfig_networkPeeringRoutesConfigBasicExample|TestAccComputeNetworkPeeringRoutesConfig_networkPeeringRoutesConfigGkeExample|TestAccComputeNetworkPeering_basic|TestAccComputeNetworkPeering_subnetRoutes|TestAccComputeNetwork_explicitAutoSubnet|TestAccComputeNetwork_networkDeleteDefaultRoute|TestAccComputeNetwork_routingModeAndUpdate|TestAccComputeNetwork_customSubnet|TestAccComputeNetwork_default_routing_mode|TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample|TestAccComputeRegionBackendService_withBackendMultiNic|TestAccComputeRoute_routeBasicExample|TestAccComputeRoute_routeIlbExample|TestAccComputeRouter_routerBasicExample|TestAccComputeRouterPeer_advertiseMode|TestAccComputeRouterPeer_basic|TestAccComputeRouterInterface_basic|TestAccComputeRouterNat_basic|TestAccComputeRouterNat_update|TestAccComputeRouterNat_withManualIpAndSubnetConfiguration|TestAccComputeRouterInterface_withTunnel|TestAccComputeRouter_basic|TestAccComputeRouter_noRegion|TestAccComputeRouter_full|TestAccComputeRouter_updateAddRemoveBGP|TestAccComputeRouter_update|TestAccComputeSubnetwork_subnetworkBasicExample|TestAccComputeSubnetwork_subnetworkLoggingConfigExample|TestAccComputeSubnetworkIamPolicy|TestAccComputeSubnetwork_update|TestAccComputeSubnetwork_basic|TestAccComputeSubnetwork_secondaryIpRanges|TestAccComputeSubnetwork_flowLogs|TestAccComputeSubnetwork_flowLogsMigrate|TestAccComputeVpnGateway_targetVpnGatewayBasicExample|TestAccComputeVpnTunnel_vpnTunnelBasicExample|TestAccComputeVpnTunnel_regionFromGateway|TestAccComputeVpnTunnel_router|TestAccComputeVpnTunnel_defaultTrafficSelectors|TestAccContainerCluster_withAuthenticatorGroupsConfig|TestAccContainerCluster_withPrivateClusterConfig|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccContainerCluster_network|TestAccContainerCluster_withIPAllocationPolicy_existingSecondaryRanges|TestAccContainerCluster_withIPAllocationPolicy_specificIPRanges|TestAccContainerCluster_withIPAllocationPolicy_specificSizes|TestAccContainerCluster_errorCleanDanglingCluster|TestAccContainerCluster_withMasterAuthorizedNetworksDisabled|TestAccContainerNodePool_nodeLocations|TestAccContainerNodePool_maxPodsPerNode|TestAccDataprocCluster_withInternalIpOnlyTrue|TestAccDNSManagedZone_dnsManagedZonePrivateExample|TestAccDNSManagedZone_dnsManagedZonePrivatePeeringExample|TestAccDNSManagedZone_privateUpdate|TestAccDNSManagedZone_privateForwardingUpdate|TestAccDNSPolicy_update|TestAccDNSPolicy_dnsPolicyBasicExample|TestAccServiceAccountIamMember_withAndWithoutCondition You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134598" |
75a4573
to
0656adc
Compare
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 5 files changed, 120 insertions(+), 38 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134599" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceComputeNetworkEndpointGroup|TestAccDataSourceGoogleNetwork|TestAccDataSourceComputeRouter|TestAccDataSourceGoogleSubnetwork|TestAccComputeSubnetworkIamBindingGenerated|TestAccComputeSubnetworkIamMemberGenerated|TestAccComputeSubnetworkIamPolicyGenerated|TestAccCloudFunctionsFunction_vpcConnector|TestAccComposerEnvironment_basic|TestAccComposerEnvironment_update|TestAccComposerEnvironment_private|TestAccComposerEnvironment_withNodeConfig|TestAccComposerEnvironment_withUpdateOnCreate|TestAccComposerEnvironment_withSoftwareConfig|TestAccComputeAddress_addressWithSubnetworkExample|TestAccComputeAddress_internal|TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccComputeFirewall_firewallBasicExample|TestAccComputeFirewall_update|TestAccComputeFirewall_priority|TestAccComputeFirewall_noSource|TestAccComputeFirewall_serviceAccounts|TestAccComputeFirewall_denied|TestAccComputeFirewall_egress|TestAccComputeFirewall_disabled|TestAccComputeFirewall_enableLogging|TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample|TestAccComputeForwardingRule_forwardingRuleInternallbExample|TestAccComputeGlobalAddress_internal|TestAccComputeInstanceGroup_network|TestAccComputeInstanceTemplate_subnet_auto|TestAccComputeInstanceTemplate_subnet_custom|TestAccComputeInstanceTemplate_secondaryAliasIpRange|TestAccComputeInstance_subnet_auto|TestAccComputeInstance_subnet_custom|TestAccComputeInstance_networkIPAuto|TestAccComputeInstance_network_ip_custom|TestAccComputeInstance_multiNic|TestAccComputeInstance_secondaryAliasIpRange|TestAccComputeNetworkEndpointGroup_networkEndpointGroupExample|TestAccComputeNetwork_networkBasicExample|TestAccComputeNetworkPeeringRoutesConfig_networkPeeringRoutesConfigBasicExample|TestAccComputeNetworkPeeringRoutesConfig_networkPeeringRoutesConfigGkeExample|TestAccComputeNetworkPeering_basic|TestAccComputeNetworkPeering_subnetRoutes|TestAccComputeNetwork_explicitAutoSubnet|TestAccComputeNetwork_customSubnet|TestAccComputeNetwork_networkDeleteDefaultRoute|TestAccComputeNetwork_routingModeAndUpdate|TestAccComputeNetwork_default_routing_mode|TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample|TestAccComputeRegionBackendService_withBackendMultiNic|TestAccComputeRoute_routeBasicExample|TestAccComputeRoute_routeIlbExample|TestAccComputeRouterPeer_basic|TestAccComputeRouter_routerBasicExample|TestAccComputeRouterPeer_advertiseMode|TestAccComputeRouterNat_basic|TestAccComputeRouterInterface_withTunnel|TestAccComputeRouterInterface_basic|TestAccComputeRouterNat_update|TestAccComputeRouterNat_withManualIpAndSubnetConfiguration|TestAccComputeRouter_basic|TestAccComputeRouter_update|TestAccComputeRouter_noRegion|TestAccComputeRouter_full|TestAccComputeRouter_updateAddRemoveBGP|TestAccComputeSubnetwork_subnetworkLoggingConfigExample|TestAccComputeSubnetwork_subnetworkBasicExample|TestAccComputeSubnetwork_basic|TestAccComputeSubnetworkIamPolicy|TestAccComputeSubnetwork_secondaryIpRanges|TestAccComputeSubnetwork_update|TestAccComputeSubnetwork_flowLogs|TestAccComputeSubnetwork_flowLogsMigrate|TestAccComputeVpnGateway_targetVpnGatewayBasicExample|TestAccComputeVpnTunnel_vpnTunnelBasicExample|TestAccComputeVpnTunnel_regionFromGateway|TestAccComputeVpnTunnel_router|TestAccComputeVpnTunnel_defaultTrafficSelectors|TestAccContainerCluster_withAuthenticatorGroupsConfig|TestAccContainerCluster_withPrivateClusterConfig|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccContainerCluster_network|TestAccContainerCluster_withIPAllocationPolicy_existingSecondaryRanges|TestAccContainerCluster_withIPAllocationPolicy_specificIPRanges|TestAccContainerCluster_withIPAllocationPolicy_specificSizes|TestAccContainerCluster_errorCleanDanglingCluster|TestAccContainerCluster_withMasterAuthorizedNetworksDisabled|TestAccContainerNodePool_maxPodsPerNode|TestAccContainerNodePool_nodeLocations|TestAccDataprocCluster_withInternalIpOnlyTrue|TestAccDNSManagedZone_dnsManagedZonePrivateExample|TestAccDNSManagedZone_dnsManagedZonePrivatePeeringExample|TestAccDNSManagedZone_privateUpdate|TestAccDNSManagedZone_privateForwardingUpdate|TestAccDNSPolicy_dnsPolicyBasicExample|TestAccDNSPolicy_update|TestAccFolderIamAuditConfig_multiple You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134600" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 106 insertions(+), 24 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134602" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccFolderIamAuditConfig_multiple|TestAccServiceAccountIamBinding_withAndWithoutCondition You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134605" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 106 insertions(+), 24 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134820" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 4 files changed, 167 insertions(+), 24 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134821" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccEndpointsService_basic|TestAccFolderIamAuditConfig_multiple|TestAccServiceAccountIamBinding_withAndWithoutCondition|TestAccServiceAccountIamMember_withAndWithoutCondition You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134822" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 113 insertions(+), 24 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134827" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccServiceAccountIamMember_withAndWithoutCondition|TestAccServiceAccountIamBinding_withAndWithoutCondition|TestAccMonitoringMetricDescriptor_monitoringMetricDescriptorBasicExample|TestAccMonitoringMetricDescriptor_monitoringMetricDescriptorAlertExample You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=134828" |
} | ||
|
||
resource "google_compute_global_network_endpoint" "proxy" { | ||
global_network_endpoint_group = google_compute_global_network_endpoint_group.external_proxy.name |
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.
custom_request_headers = ["host: ${google_compute_global_network_endpoint.proxy.fqdn}"] | ||
|
||
backend { | ||
group = google_compute_global_network_endpoint_group.external_proxy.self_link |
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.
also this?
@@ -779,8 +779,6 @@ objects: | |||
either maxRate or maxRatePerEndpoint must be set. | |||
- !ruby/object:Api::Type::Double | |||
name: 'maxUtilization' | |||
send_empty_value: true | |||
default_value: 0.8 |
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.
The docs here still says the default is 0.8. Also, is it possible that removing this default causes diffs (since a user's state file might say 0.8 is the value but their config says it's nothing)?
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.
Due to an interesting behavior (bug?) in backend service, max_utilization
never is used in the hash function for backend: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/templates/terraform/constants/backend_service.go.erb
So modifying this field never causes a diff. I'm worried that adding this field to the hash function would be a breaking change
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.
Also, because this is moving to Optional+Computed it shouldn't cause a diff if their config says nothing (assuming the hash function was working correctly)
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.
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.
Ah, right it doesn't come back from the API in most cases so just optional works.
Keeping send_empty_value
makes terraform send a value in every circumstance, meaning that we can't not specify this field for NEG backends. I could strip it from requests for NEG backends with a virtual field, but then it still appears in state and create plans.
For some reason I'm seeing zero values be sent even with send_empty_value: false
. Not entirely sure why that is happening
@@ -779,8 +779,6 @@ objects: | |||
either maxRate or maxRatePerEndpoint must be set. | |||
- !ruby/object:Api::Type::Double | |||
name: 'maxUtilization' | |||
send_empty_value: true |
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.
Removing this means it can no longer be updated to 0 (or set explicitly to 0, if there's a server-side default that's nonzero). is that what we want?
b6a3a88
to
608e786
Compare
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 115 insertions(+), 26 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=135456" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_backendServiceNetworkEndpointExample|TestAccFolderIamAuditConfig_multiple|TestAccMonitoringMetricDescriptor_monitoringMetricDescriptorAlertExample You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=135458" |
Fixes: hashicorp/terraform-provider-google#6155
Add ability to set global network endpoint group as backend for backend service. Make health_checks optional if NEG is used. Virtual field added to enable this
Release Note Template for Downstream PRs (will be copied)