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.

Sign up for GitHub

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

[compute_instance] - Allow updating of network and subnetwork properties #4011

Merged
merged 12 commits into from
Sep 25, 2020
Merged

[compute_instance] - Allow updating of network and subnetwork properties #4011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b8c225e
[compute_instance] - Allow updating of network and subnetwork properties
ScottSuarez Sep 23, 2020
aafc4f1
fix spelling error
ScottSuarez Sep 23, 2020
464c82e
resolve comments
ScottSuarez Sep 24, 2020
f13e905
switch to use id over self link
ScottSuarez Sep 24, 2020
8099c39
remove unused var
ScottSuarez Sep 24, 2020
4987c92
resolve comments
ScottSuarez Sep 24, 2020
a1641a1
take into account fingerprint refresh before update call.
ScottSuarez Sep 24, 2020
314005e
spelling fix
ScottSuarez Sep 24, 2020
d21f704
Update third_party/terraform/resources/resource_compute_instance.go.erb
ScottSuarez Sep 25, 2020
ebf565d
Update third_party/terraform/resources/resource_compute_instance.go.erb
ScottSuarez Sep 25, 2020
a04588c
Add comment explaining subnetwork/network relationship. Changed test …
ScottSuarez Sep 25, 2020
1fbb1ef
Merge branch 'scsuarez/update-network-interface' of https://github.co…
ScottSuarez Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions third_party/terraform/resources/resource_compute_instance.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,27 +1345,34 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err

networkName := d.Get(prefix + ".name").(string)
subnetwork := networkInterface.Subnetwork
updateDurringStop := d.HasChange(prefix+".subnetwork") || d.HasChange(prefix+".network") || d.HasChange(prefix+".subnetwork_project")
updateDuringStop := d.HasChange(prefix+".subnetwork") || d.HasChange(prefix+".network") || d.HasChange(prefix+".subnetwork_project")

// Sanity check
if networkName != instNetworkInterface.Name {
return fmt.Errorf("Instance networkInterface had unexpected name: %s", instNetworkInterface.Name)
}

// On creation the network is inferred if only subnetwork is given.
// Unforunately for us there is no way to determine if the user is
// explicitly asigning network or we are reusing the one that was infered
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
// from state. So here we check if subnetwork changed and network did not.
// In the scenario we assume network was inferred and attempt to figure out
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
// the new corresponding network.

if d.HasChange(prefix + ".subnetwork") {
if !d.HasChange(prefix + ".network") {
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
subnetProjectField := prefix + ".subnetwork_project"
sf, err := ParseSubnetworkFieldValueWithProjectField(subnetwork, subnetProjectField, d, config)
if err != nil {
return fmt.Errorf("Error, cannot determine self_link for subnetwork %q: %s", subnetwork, err)
return fmt.Errorf("Cannot determine self_link for subnetwork %q: %s", subnetwork, err)
}
resp, err := config.clientCompute.Subnetworks.Get(sf.Project, sf.Region, sf.Name).Do()
if err != nil {
return errwrap.Wrapf("Error getting subnetwork value: {{err}}", err)
}
nf, err := ParseNetworkFieldValue(resp.Network, d, config)
if err != nil {
return fmt.Errorf("Error, cannot determine self_link for network %q: %s", resp.Network, err)
return fmt.Errorf("Cannot determine self_link for network %q: %s", resp.Network, err)
}
networkInterface.Network = nf.RelativeLink()
}
Expand Down Expand Up @@ -1425,7 +1432,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
instNetworkInterface = instance.NetworkInterfaces[i]
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
}

if !updateDurringStop {
if !updateDuringStop {
if d.HasChange(prefix + ".alias_ip_range") {
// Alias IP ranges cannot be updated; they must be removed and then added.
// unless you are changing subnetwork/network
Expand All @@ -1452,11 +1459,14 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
}
}

networkInterface.Fingerprint = instNetworkInterface.Fingerprint
// Setting NetworkIP to empty and AccessConfigs to nil.
// This will opt them out from being modified in the patch call.
// They cannot be changed by UpdateNetworkInterface
networkInterface.NetworkIP = ""
networkInterface.AccessConfigs = nil
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
networkInterface.Fingerprint = instNetworkInterface.Fingerprint
updateCall := config.clientComputeBeta.Instances.UpdateNetworkInterface(project, zone, instance.Name, networkName, networkInterface).Do
if !updateDurringStop && d.HasChange(prefix+".alias_ip_range") {
if !updateDuringStop && d.HasChange(prefix+".alias_ip_range") {
op, err := updateCall()
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return errwrap.Wrapf("Error updating network interface: {{err}}", err)
Expand Down
27 changes: 4 additions & 23 deletions third_party/terraform/tests/resource_compute_instance_test.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1882,19 +1882,6 @@ func TestAccComputeInstance_subnetworkChange(t *testing.T) {
var instance compute.Instance
instanceName := fmt.Sprintf("tf-test-%s", randString(t, 10))
suffix := fmt.Sprintf("%s", randString(t, 10))
subnet := ""

checkAndUpdateSubnet := func(instance *compute.Instance) resource.TestCheckFunc {
oldSubnetwork := subnet
return func(s *terraform.State) error {
subnet = instance.NetworkInterfaces[0].Subnetwork
if oldSubnetwork != subnet {
return nil
}

return fmt.Errorf("subnetwork equivalent to previous... expected change; %s : %s", oldSubnetwork, subnet)
}
}

vcrTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -1903,20 +1890,10 @@ func TestAccComputeInstance_subnetworkChange(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccComputeInstance_subnetworkChange(suffix, instanceName),
danawillow marked this conversation as resolved.
Show resolved Hide resolved
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceHasAliasIpRange(&instance, "inst-test-tertiary", "10.1.0.0/20"),
checkAndUpdateSubnet(&instance),
),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}),
{
Config: testAccComputeInstance_subnetworkChangeUpdate(suffix, instanceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceHasAliasIpRange(&instance, "inst-test-secondary2", "173.16.0.0/24"),
checkAndUpdateSubnet(&instance),
),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}),
},
Expand Down Expand Up @@ -4807,10 +4784,12 @@ func testAccComputeInstance_subnetworkChange(suffix, instance string) string {

resource "google_compute_network" "inst-test-network" {
ScottSuarez marked this conversation as resolved.
Show resolved Hide resolved
name = "tf-test-network-%s"
auto_create_subnetworks = false
}

resource "google_compute_network" "inst-test-network2" {
name = "tf-test-network2-%s"
auto_create_subnetworks = false
}

resource "google_compute_subnetwork" "inst-test-subnetwork" {
Expand Down Expand Up @@ -4883,10 +4862,12 @@ func testAccComputeInstance_subnetworkChangeUpdate(suffix, instance string) stri

resource "google_compute_network" "inst-test-network" {
name = "tf-test-network-%s"
auto_create_subnetworks = false
}

resource "google_compute_network" "inst-test-network2" {
name = "tf-test-network2-%s"
auto_create_subnetworks = false
}

resource "google_compute_subnetwork" "inst-test-subnetwork" {
Expand Down