Allow to configure nsx_id for locale service #1040
Conversation
annakhm
force-pushed
the
locale_service_id
branch
from
24a9913 to
11c31e7
Compare
|
/test-all |
annakhm
force-pushed
the
locale_service_id
branch
from
11c31e7 to
ab7f62b
Compare
|
/test-all |
|
/test-all |
annakhm
force-pushed
the
locale_service_id
branch
from
ab7f62b to
1d3b736
Compare
|
/test-all |
1 similar comment
|
/test-all |
|
As the code validates ha_mode, let's rebase on top of PR #1042 (which is merged already), and run against NSX v4.1.1 and above, just to make sure that it works there as well? |
annakhm
force-pushed
the
locale_service_id
branch
from
1d3b736 to
0d483bf
Compare
|
/test-all |
| } | ||
| } | ||
|
|
||
| return keyMap |
There was a problem hiding this comment.
The implementation looks good, and I'm not questioning anything.
I'm just curious if we really need a map or we could have just returned an array of ids
There was a problem hiding this comment.
Just for lookup speed it is easier to have a map
|
|
||
| keyMap := make(map[string]bool) | ||
| for _, item := range schemaSet.(*schema.Set).List() { | ||
| mapItem := item.(map[string]interface{}) |
There was a problem hiding this comment.
is this case always guaranteed to work? Wondering if there are case where the list element is not a map.
There was a problem hiding this comment.
This function is tailored for TypeSet of nested schema, in which case its always a list of maps. I will clarify this in the comment.
| func getTestAdvancedTopology() string { | ||
| // Non-basic testing topology available | ||
| // For now this is used by tests that have minimum 2 edge nodes per cluster | ||
| return os.Getenv("NSXT_TEST_ADVANCED_TOPOLOGY") |
There was a problem hiding this comment.
I understand the env variable is an easy solution, but in theory could have we verified the number of edges in the cluster with a precheck?
There was a problem hiding this comment.
I thought it would be a good idea to have extra test granularity for basic topologies and complex topologies. But you're right, we could leverage the precheck here as well.
|
/test-all |
annakhm
force-pushed
the
locale_service_id
branch
from
0d483bf to
1692eab
Compare
|
/test-all |
Rather than recreating locale services with every update, allow to change existing locale service by specifying it nsx_id When nsx_id is specified for locale service, update will happen in place, thus keeping all objects dependant on the locale service intact (for example, service interfaces). Note that nsx_id would only be set in state if specified by user, otherwise non-empty plan would always be produced due to SDK design issue that offers limited support for Computed fields in TypeSet nested schema. This workaround should be removed after upgrading to plugin framework. When nsx_id is not speicified, there is no change of behavior. Signed-off-by: Anna Khmelnitsky <[email protected]>
annakhm
force-pushed
the
locale_service_id
branch
from
1692eab to
53ca4e9
Compare
Rather than recreating locale services with every update, allow to change existing locale service by specifying it nsx_id
When nsx_id is specified for locale service, update will happen in place. When not specified, update on locale service will force new gateway object, thus recreating dependant objects such as service interfaces.