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

Jump to bottom

Partly rollback changes from #11321 to fix an alias_ip_range bug #12180

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+11 −97
Open

Partly rollback changes from #11321 to fix an alias_ip_range bug #12180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1eb92d9
partly rollback changes from PR https://github.com/GoogleCloudPlatfor…
karolgorc Oct 29, 2024
f25589e
fix subnetworkUpdate test
karolgorc Oct 31, 2024
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
21 changes: 5 additions & 16 deletions mmv1/third_party/terraform/services/compute/compute_instance_helpers.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,28 +53,17 @@ func expandAliasIpRanges(ranges []interface{}) []*compute.AliasIpRange {
return ipRanges
}

func flattenAliasIpRange(d *schema.ResourceData, ranges []*compute.AliasIpRange, i int) []map[string]interface{} {
prefix := fmt.Sprintf("network_interface.%d", i)
func flattenAliasIpRange(ranges []*compute.AliasIpRange) []map[string]interface{} {
rangesSchema := make([]map[string]interface{}, 0, len(ranges))

configData := []map[string]interface{}{}
for _, item := range d.Get(prefix + ".alias_ip_range").([]interface{}) {
configData = append(configData, item.(map[string]interface{}))
}

apiData := make([]map[string]interface{}, 0, len(ranges))
for _, ipRange := range ranges {
apiData = append(apiData, map[string]interface{}{
rangesSchema = append(rangesSchema, map[string]interface{}{
"ip_cidr_range": ipRange.IpCidrRange,
"subnetwork_range_name": ipRange.SubnetworkRangeName,
})
}

//permadiff fix
sorted, err := tpgresource.SortMapsByConfigOrder(configData, apiData, "ip_cidr_range")
if err != nil {
return apiData
}
return sorted
return rangesSchema
}

func expandScheduling(v interface{}) (*compute.Scheduling, error) {
Expand Down Expand Up @@ -414,7 +403,7 @@ func flattenNetworkInterfaces(d *schema.ResourceData, config *transport_tpg.Conf
"subnetwork": tpgresource.ConvertSelfLinkToV1(iface.Subnetwork),
"subnetwork_project": subnet.Project,
"access_config": ac,
"alias_ip_range": flattenAliasIpRange(d, iface.AliasIpRanges, i),
"alias_ip_range": flattenAliasIpRange(iface.AliasIpRanges),
"nic_type": iface.NicType,
"stack_type": iface.StackType,
"ipv6_access_config": flattenIpv6AccessConfigs(iface.Ipv6AccessConfigs),
Expand Down
20 changes: 0 additions & 20 deletions mmv1/third_party/terraform/services/compute/resource_compute_instance.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2270,9 +2270,6 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
Fingerprint: instNetworkInterface.Fingerprint,
ForceSendFields: []string{"AliasIpRanges"},
}
if commonAliasIpRanges := CheckForCommonAliasIp(instNetworkInterface, networkInterface); len(commonAliasIpRanges) > 0 {
ni.AliasIpRanges = commonAliasIpRanges
}
op, err := config.NewComputeClient(userAgent).Instances.UpdateNetworkInterface(project, zone, instance.Name, networkName, ni).Do()
if err != nil {
return errwrap.Wrapf("Error removing alias_ip_range: {{"{{"}}err{{"}}"}}", err)
Expand Down Expand Up @@ -3320,20 +3317,3 @@ func isEmptyServiceAccountBlock(d *schema.ResourceData) bool {
}
return false
}

// Alias ip ranges cannot be removed and created at the same time. This checks if there are any unchanged alias ip ranges
// to be kept in between the PATCH operations on Network Interface
func CheckForCommonAliasIp(old, new *compute.NetworkInterface) []*compute.AliasIpRange {
newAliasIpMap := make(map[string]bool)
for _, ipRange := range new.AliasIpRanges {
newAliasIpMap[ipRange.IpCidrRange] = true
}

resultAliasIpRanges := make([]*compute.AliasIpRange, 0)
for _, val := range old.AliasIpRanges {
if newAliasIpMap[val.IpCidrRange] {
resultAliasIpRanges = append(resultAliasIpRanges, val)
}
}
return resultAliasIpRanges
}
67 changes: 6 additions & 61 deletions mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"

"github.com/stretchr/testify/assert"
"github.com/hashicorp/terraform-provider-google/google/acctest"
"github.com/hashicorp/terraform-provider-google/google/envvar"
tpgcompute "github.com/hashicorp/terraform-provider-google/google/services/compute"
Expand Down Expand Up @@ -100,60 +99,6 @@ func TestMinCpuPlatformDiffSuppress(t *testing.T) {
}
}

func TestCheckForCommonAliasIp(t *testing.T) {
type testCase struct {
old, new []*compute.AliasIpRange
expected []*compute.AliasIpRange
}

testCases := []testCase{
{
old: []*compute.AliasIpRange{
{IpCidrRange: "10.0.0.0/24"},
{IpCidrRange: "10.0.1.0/24"},
},
new: []*compute.AliasIpRange{
{IpCidrRange: "10.0.0.0/24"},
{IpCidrRange: "10.0.2.0/24"},
},
expected: []*compute.AliasIpRange{
{IpCidrRange: "10.0.0.0/24"},
},
},
{
old: []*compute.AliasIpRange{
{IpCidrRange: "172.16.0.0/24"},
{IpCidrRange: "10.0.1.0/24"},
},
new: []*compute.AliasIpRange{
{IpCidrRange: "172.16.0.0/24"},
{IpCidrRange: "10.0.2.0/24"},
},
expected: []*compute.AliasIpRange{
{IpCidrRange: "172.16.0.0/24"},
},
},
{
old: []*compute.AliasIpRange{
{IpCidrRange: "10.0.0.0/24"},
{IpCidrRange: "10.0.1.0/24"},
},
new: []*compute.AliasIpRange{
{IpCidrRange: "192.168.0.0/24"},
{IpCidrRange: "172.17.0.0/24"},
},
expected: []*compute.AliasIpRange{},
},
}

for _, tc := range testCases {
oldInterface := &compute.NetworkInterface{AliasIpRanges: tc.old}
newInterface := &compute.NetworkInterface{AliasIpRanges: tc.new}
result := tpgcompute.CheckForCommonAliasIp(oldInterface, newInterface)
assert.Equal(t, tc.expected, result)
}
}

func computeInstanceImportStep(zone, instanceName string, additionalImportIgnores []string) resource.TestStep {
// metadata is only read into state if set in the config
// importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config,
Expand Down Expand Up @@ -2038,15 +1983,15 @@ func TestAccComputeInstance_secondaryAliasIpRange(t *testing.T) {
testAccCheckComputeInstanceHasAliasIpRange(&instance, "inst-test-secondary", "172.16.0.0/24"),
),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{}),
{
Config: testAccComputeInstance_secondaryAliasIpRangeUpdate(networkName, subnetName, instanceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceHasAliasIpRange(&instance, "", "10.0.1.0/24"),
),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{}),
},
})
}
Expand Down Expand Up @@ -2090,7 +2035,7 @@ func TestAccComputeInstance_aliasIpRangeCommonAddresses(t *testing.T) {
testAccCheckComputeInstanceHasAliasIpRange(&instance, "inst-test-tertiary", "10.1.3.0/24"),
),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{}),
},
})
}
Expand Down Expand Up @@ -2933,15 +2878,15 @@ func TestAccComputeInstance_subnetworkUpdate(t *testing.T) {
{
Config: testAccComputeInstance_subnetworkUpdate(suffix, instanceName),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update", "network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}),
{
Config: testAccComputeInstance_subnetworkUpdateTwo(suffix, instanceName),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update", "network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}),
{
Config: testAccComputeInstance_subnetworkUpdate(suffix, instanceName),
},
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update", "network_interface.0.alias_ip_range.0.ip_cidr_range", "network_interface.0.alias_ip_range.0.subnetwork_range_name", "network_interface.0.alias_ip_range.1.ip_cidr_range", "network_interface.0.alias_ip_range.1.subnetwork_range_name"}),
computeInstanceImportStep("us-east1-d", instanceName, []string{"allow_stopping_for_update"}),
},
})
}
Expand Down
Loading