From d83cc3c5441a594dbbe268f0ed6a95bace527f20 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Tue, 23 Jul 2019 15:55:46 -0700 Subject: [PATCH] add diff suppress to dataproc iamge version --- .../resource_dataproc_cluster.go.erb | 62 +++++++++++++- .../resource_dataproc_cluster_test.go.erb | 81 ++++++++++++++++++- 2 files changed, 136 insertions(+), 7 deletions(-) diff --git a/third_party/terraform/resources/resource_dataproc_cluster.go.erb b/third_party/terraform/resources/resource_dataproc_cluster.go.erb index dd0b0fcf1eaa..40fef648dac4 100644 --- a/third_party/terraform/resources/resource_dataproc_cluster.go.erb +++ b/third_party/terraform/resources/resource_dataproc_cluster.go.erb @@ -16,6 +16,8 @@ import ( "google.golang.org/api/dataproc/v1beta2" ) +var resolveDataprocImageVersion = regexp.MustCompile(`(?P[^\s.-]+)\.(?P[^\s.-]+)(?:\.(?P[^\s.-]+))?(?:\-(?P[^\s.-]+))?`) + func resourceDataprocCluster() *schema.Resource { return &schema.Resource{ Create: resourceDataprocClusterCreate, @@ -262,10 +264,11 @@ func resourceDataprocCluster() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "image_version": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + DiffSuppressFunc: dataprocImageVersionDiffSuppress, }, "override_properties": { @@ -1025,3 +1028,54 @@ func configOptions(d *schema.ResourceData, option string) (map[string]interface{ } return nil, false } + +func dataprocImageVersionDiffSuppress(_, old, new string, _ *schema.ResourceData) bool { + oldV, err := parseDataprocImageVersion(old) + if err != nil { + return false + } + newV, err := parseDataprocImageVersion(new) + if err != nil { + return false + } + + log.Printf("[DEBUG] diff suppress dataproc %q: %+v", old, oldV) + log.Printf("[DEBUG] diff suppress dataproc %q: %+v", new, newV) + + if newV.major != oldV.major { + return false + } + if newV.minor != oldV.minor { + return false + } + // Only compare subminor version if set in config version. + if newV.subminor != "" && newV.subminor != oldV.subminor { + return false + } + // Only compare os if it is set in config version. + if newV.osName != "" && newV.osName != oldV.osName { + return false + } + return true +} + +type dataprocImageVersion struct { + major string + minor string + subminor string + osName string +} + +func parseDataprocImageVersion(version string) (*dataprocImageVersion, error) { + matches := resolveDataprocImageVersion.FindStringSubmatch(version) + if len(matches) != 5 { + return nil, fmt.Errorf("invalid image version %q", version) + } + + return &dataprocImageVersion{ + major: matches[1], + minor: matches[2], + subminor: matches[3], + osName: matches[4], + }, nil +} diff --git a/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb b/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb index 78f37fd78adb..942509d2ceb8 100644 --- a/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb +++ b/third_party/terraform/tests/resource_dataproc_cluster_test.go.erb @@ -17,7 +17,7 @@ import ( "google.golang.org/api/googleapi" ) -func TestExtractInitTimeout(t *testing.T) { +func TestDataprocExtractInitTimeout(t *testing.T) { t.Parallel() actual, err := extractInitTimeout("500s") @@ -30,7 +30,7 @@ func TestExtractInitTimeout(t *testing.T) { } } -func TestExtractInitTimeout_nonSeconds(t *testing.T) { +func TestDataprocExtractInitTimeout_nonSeconds(t *testing.T) { t.Parallel() actual, err := extractInitTimeout("5m") @@ -43,7 +43,7 @@ func TestExtractInitTimeout_nonSeconds(t *testing.T) { } } -func TestExtractInitTimeout_empty(t *testing.T) { +func TestDataprocExtractInitTimeout_empty(t *testing.T) { t.Parallel() _, err := extractInitTimeout("") @@ -54,6 +54,81 @@ func TestExtractInitTimeout_empty(t *testing.T) { t.Fatalf("Expected an error with message '%s', but got %v", expected, err.Error()) } +func TestDataprocParseImageVersion(t *testing.T) { + t.Parallel() + + testCases := map[string]dataprocImageVersion{ + "1.2": {"1", "2", "", ""}, + "1.2.3": {"1", "2", "3", ""}, + "1.2.3rc": {"1", "2", "3rc", ""}, + "1.2-debian9": {"1", "2", "", "debian9"}, + "1.2.3-debian9": {"1", "2", "3", "debian9"}, + "1.2.3rc-debian9": {"1", "2", "3rc", "debian9"}, + } + + for v, expected := range testCases { + actual, err := parseDataprocImageVersion(v) + if actual.major != expected.major { + t.Errorf("parsing version %q returned error: %v", v, err) + } + if err != nil { + t.Errorf("parsing version %q returned error: %v", v, err) + } + if actual.minor != expected.minor { + t.Errorf("parsing version %q returned error: %v", v, err) + } + if actual.subminor != expected.subminor { + t.Errorf("parsing version %q returned error: %v", v, err) + } + if actual.osName != expected.osName { + t.Errorf("parsing version %q returned error: %v", v, err) + } + } + + errorTestCases := []string{ + "", + "1", + "notaversion", + "1-debian", + } + for _, v := range errorTestCases { + if _, err := parseDataprocImageVersion(v); err == nil { + t.Errorf("expected parsing invalid version %q to return error", v) + } + } +} + +func TestDataprocDiffSuppress(t *testing.T) { + t.Parallel() + + doSuppress := [][]string{ + {"1.3.10-debian9", "1.3"}, + {"1.3.10-debian9", "1.3-debian9"}, + {"1.3.10", "1.3"}, + {"1.3-debian9", "1.3"}, + } + + noSuppress := [][]string{ + {"1.3.10-debian9", "1.3.10-ubuntu"}, + {"1.3.10-debian9", "1.3.9-debian9"}, + {"1.3.10-debian9", "1.3-ubuntu"}, + {"1.3.10-debian9", "1.3.9"}, + {"1.3.10-debian9", "1.4"}, + {"1.3.10-debian9", "2.3"}, + } + + for _, tup := range doSuppress { + if !dataprocImageVersionDiffSuppress("", tup[0], tup[1], nil) { + t.Errorf("expected (old: %q, new: %q) to be suppressed", tup[0], tup[1]) + } + } + for _, tup := range noSuppress { + if dataprocImageVersionDiffSuppress("", tup[0], tup[1], nil) { + t.Errorf("expected (old: %q, new: %q) to not be suppressed", tup[0], tup[1]) + } + } +} + func TestAccDataprocCluster_missingZoneGlobalRegion1(t *testing.T) { t.Parallel()