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.

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

[TF] GKE Cluster - fix import on some convenience fields #1434

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion build/terraform
2 changes: 1 addition & 1 deletion build/terraform-beta
80 changes: 55 additions & 25 deletions third_party/terraform/resources/resource_container_cluster.go.erb
Original file line number Diff line number Diff line change
Expand Up @@ -671,11 +671,6 @@ func resourceContainerCluster() *schema.Resource {
}
}

func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
// If the user specified a size and the API returned a full cidr block, suppress.
return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new)
}

func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

Expand Down Expand Up @@ -1497,11 +1492,15 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
name := fmt.Sprintf("%s/nodePools/%s", containerClusterFullName(project, location, clusterName), "default-pool")
op, err := config.clientContainerBeta.Projects.Locations.Clusters.NodePools.Delete(name).Do()
if err != nil {
return errwrap.Wrapf("Error deleting default node pool: {{err}}", err)
}
err = containerOperationWait(config, op, project, location, "removing default node pool", timeoutInMinutes)
if err != nil {
return errwrap.Wrapf("Error deleting default node pool: {{err}}", err)
if !isGoogleApiErrorWithCode(err, 404) {
return errwrap.Wrapf("Error deleting default node pool: {{err}}", err)
}
log.Printf("[WARN] Container cluster %q default node pool already removed, no change", d.Id())
} else {
err = containerOperationWait(config, op, project, location, "removing default node pool", timeoutInMinutes)
if err != nil {
return errwrap.Wrapf("Error deleting default node pool: {{err}}", err)
}
}
}

Expand Down Expand Up @@ -2108,29 +2107,55 @@ func flattenPodSecurityPolicyConfig(c *containerBeta.PodSecurityPolicyConfig) []
<% end -%>

func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rip indeed, very sorry

parts := strings.Split(d.Id(), "/")
config := meta.(*Config)

parts := strings.Split(d.Id(), "/")
var project, location, clusterName string
switch len(parts) {
case 2:
if loc := parts[0]; isZone(loc) {
d.Set("zone", loc)
} else {
d.Set("region", loc)
}
d.Set("name", parts[1])
location = parts[0]
clusterName = parts[1]
case 3:
d.Set("project", parts[0])
if loc := parts[1]; isZone(loc) {
d.Set("zone", loc)
} else {
d.Set("region", loc)
}
d.Set("name", parts[2])
project = parts[0]
location = parts[1]
clusterName = parts[2]
default:
return nil, fmt.Errorf("Invalid container cluster specifier. Expecting {zone}/{name} or {project}/{zone}/{name}")
}

d.SetId(parts[len(parts)-1])
if len(project) > 0 {
d.Set("project", project)
} else {
var err error
project, err = getProject(d, config)
if err != nil {
return nil, err
}
}

if isZone(location) {
d.Set("zone", location)
} else {
d.Set("region", location)
}

d.Set("name", clusterName)
d.SetId(clusterName)

// Try to determine remove_default_node_pool from presence of default
// node pool; if still present and user has it set to true, the pool
// will get removed on next apply.
nodePool := fmt.Sprintf("%s/nodePools/%s", containerClusterFullName(project, location, clusterName), "default-pool")
_, err := config.clientContainerBeta.Projects.Locations.Clusters.NodePools.Get(nodePool).Do()
if err != nil && isGoogleApiErrorWithCode(err, 404) {
d.Set("remove_default_node_pool", true)
} else {
d.Set("remove_default_node_pool", false)
if err != nil {
log.Printf("[WARN] Unable to import value for remove_default_node_pool, got error while trying to get default node pool: %s", err)
}
}

return []*schema.ResourceData{d}, nil
}

Expand Down Expand Up @@ -2160,6 +2185,11 @@ func extractNodePoolInformationFromCluster(d *schema.ResourceData, config *Confi
}, nil
}

func cidrOrSizeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
// If the user specified a size and the API returned a full cidr block, suppress.
return strings.HasPrefix(new, "/") && strings.HasSuffix(old, new)
}

// We want to suppress diffs for empty or default client certificate configs, i.e:
// [{ "issue_client_certificate": true}] --> []
// [] -> [{ "issue_client_certificate": true}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"bytes"
"fmt"
"testing"

"regexp"
"strconv"

"github.com/hashicorp/terraform/helper/acctest"
Expand Down Expand Up @@ -276,7 +276,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
},
{
Config: testAccContainerCluster_removeNetworkPolicy(clusterName),
Expand All @@ -290,7 +289,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
},
{
Config: testAccContainerCluster_withNetworkPolicyDisabled(clusterName),
Expand All @@ -304,7 +302,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
},
{
Config: testAccContainerCluster_withNetworkPolicyConfigDisabled(clusterName),
Expand All @@ -318,7 +315,6 @@ func TestAccContainerCluster_withNetworkPolicyEnabled(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
},
{
Config: testAccContainerCluster_withNetworkPolicyConfigDisabled(clusterName),
Expand Down Expand Up @@ -1214,7 +1210,6 @@ func TestAccContainerCluster_withDefaultNodePoolRemoved(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"remove_default_node_pool"},
},
},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,10 @@ $ terraform import google_container_cluster.mycluster my-gcp-project/us-east1-a/

$ terraform import google_container_cluster.mycluster us-east1-a/my-cluster
```

~> **Note:** This resource has several fields that control Terraform-specific behavior and aren't present in the API. If they are set in config and you import a cluster, Terraform may need to perform an update immediately after import. Some of these updates are no-ops, and some may modify your cluster.

For example:

- `min_master_version` will not be set on import and will show a no-op diff if set in config.
- `remove_default_node_pool`: If the default node pool exists at import, this value will be set to false in state (or true if non-existant). If set to true in config but the node pool exists, a follow-up diff/apply will delete the default node pool.