GRE tunnel resource implementation #1091
Conversation
ksamoray
force-pushed
the
gre_tunnel
branch
2 times, most recently
from
3fb65c8 to
4e33da5
Compare
|
/test-all |
| Type: schema.TypeString, | ||
| Description: "Destination IPv4 address", | ||
| Required: true, | ||
| ValidateFunc: validateSingleIP(), |
There was a problem hiding this comment.
If this is IPv4 only, there is validation.IsIPv4Address helper. Not sure the description is accurate though
| "locale_service_path": { | ||
| Type: schema.TypeString, | ||
| Description: "Policy path of associated Gateway Locale Service on NSX", | ||
| Required: true, |
| urpf_mode = "STRICT" | ||
|
|
||
|
|
||
| tag { |
There was a problem hiding this comment.
since this is a resource for dependency, I think we should provide minimal working config (tags etc. not needed)
| testResourceName := "nsxt_policy_tier0_gateway_gre_tunnel.test" | ||
| name := getAccTestResourceName() | ||
|
|
||
| resource.Test(t, resource.TestCase{ |
There was a problem hiding this comment.
good call for not making this one Parallel :)
|
/test-all |
|
|
||
| # nsxt_policy_tier0_gateway_gre_tunnel | ||
|
|
||
| This resource provides a method for the management of a Tier-0 gateway GRE tunnel. Note that edge cluster and in interface must be configured on Tier-0 Gateway in order to configure GRE tunnels on it. |
There was a problem hiding this comment.
User-level comment: What's the "in" interface? It's not a term I'm familiar with for Tier-0 gateways
There was a problem hiding this comment.
Should be "an interface" :)
| } | ||
|
|
||
| resource "nsxt_policy_tier0_gateway" "test" { | ||
| display_name = "terraform-acctest-resource-74743" |
There was a problem hiding this comment.
pedant nit: For documentation I would use simple names. I think you might have extracted those from an acceptance test run?
There was a problem hiding this comment.
Exactly... Will replace the name.
|
|
||
| * `display_name` - (Required) Display name of the resource. | ||
| * `description` - (Optional) Description of the resource. | ||
| * `tag` - (Optional) A list of scope + tag pairs to associate with this resource. |
There was a problem hiding this comment.
It's always tag in all our resources.
And as a general rule - we use singular naming for complex objects as they appear in HCL as a sequence of individual objects, e.g here or here with rule.
For list of simple types (e.g string) we use plural naming, e.g here with ip_addresses.
|
|
||
| obj, err := tier0GatewayGRETunnelFromSchema(d) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create GRE Tunnel: %v", err) |
There was a problem hiding this comment.
nit abou the error message: we have not yet tried the creation here, so maybe the error should state "unable to create the GRE tunnel" or something like that
|
/test-all |
| } | ||
| data "nsxt_policy_gateway_locale_service" "test" { | ||
| gateway_path = nsxt_policy_tier0_gateway.test.path | ||
| depends_on = [data.nsxt_policy_realization_info.realization_info] |
There was a problem hiding this comment.
Does the config fail without this? Policy intent should not depend on realization state..
There was a problem hiding this comment.
IDK - tests pass without it. But would I be able to retrieve locale id while related stuff isn't realized?
There was a problem hiding this comment.
I don't see why not.. everything is on policy level
|
/test-all |
| enable_pim = "true" | ||
| } | ||
|
|
||
| data "nsxt_policy_realization_info" "realization_info" { |
There was a problem hiding this comment.
This is not needed anymore
There was a problem hiding this comment.
Indeed, it's useless.
Signed-off-by: Kobi Samoray <[email protected]>
|
/test-all |
Fixes: #1027