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

Make google_container_node_pool resources importable. #284

Merged
merged 4 commits into from
Aug 7, 2017
Merged

Make google_container_node_pool resources importable. #284

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3def84d
Make google_container_node_pool resources importable.
abogs Aug 3, 2017
933efee
Merge branch 'master' of https://github.com/terraform-providers/terra…
abogs Aug 4, 2017
84aa43f
Merge https://github.com/terraform-providers/terraform-provider-google
drzero42 Aug 5, 2017
fa58dee
Add website documentation for importing google_container_node_pool.
drzero42 Aug 5, 2017
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
32 changes: 32 additions & 0 deletions google/import_container_node_pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package google

import (
"fmt"
"testing"

"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
)

func TestAccGoogleContainerNodePool_import(t *testing.T) {
resourceName := "google_container_node_pool.np"
cluster := fmt.Sprintf("tf-nodepool-test-%s", acctest.RandString(10))
np := fmt.Sprintf("tf-nodepool-test-%s", acctest.RandString(10))
conf := testAccContainerNodePool_basic(cluster, np)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckContainerNodePoolDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: conf,
},

resource.TestStep{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
18 changes: 18 additions & 0 deletions google/resource_container_node_pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package google
import (
"fmt"
"log"
"strings"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
Expand All @@ -18,6 +19,10 @@ func resourceContainerNodePool() *schema.Resource {
Delete: resourceContainerNodePoolDelete,
Exists: resourceContainerNodePoolExists,

Importer: &schema.ResourceImporter{
State: resourceContainerNodePoolStateImporter,
},

Schema: map[string]*schema.Schema{
"project": &schema.Schema{
Type: schema.TypeString,
Expand Down Expand Up @@ -360,3 +365,16 @@ func resourceContainerNodePoolExists(d *schema.ResourceData, meta interface{}) (
}
return true, nil
}

func resourceContainerNodePoolStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
parts := strings.Split(d.Id(), "/")
if len(parts) != 3 {
return nil, fmt.Errorf("Invalid container cluster specifier. Expecting {zone}/{cluster}/{name}")
}

d.Set("zone", parts[0])
d.Set("cluster", parts[1])
d.Set("name", parts[2])

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum...

The Create method sets the id of the resource to the name of the container node pool.
The import will leave the id as zone/cluster/name.

This is not a big deal since we don't use the resource id after creation right now but if we decide to use it in the future, it is nice to have them consistent.

However, only setting the id to name like it's done in Create is wrong since it is valid for two node pool across two clusters to have the same name. Currently, we prevent this use case.

I created a separate issue (#298) to fix the Create method and set the id to zone/cluster/name. This will require a use of schema migration and it is better to do this in a separate PR. Migration will make all the ids consistent.

Nothing to do for you here :) I will take care of the other issue.

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