Skip to content

Commit

Permalink
Support connectivity fields for Composer 3 (GoogleCloudPlatform#9889)
Browse files Browse the repository at this point in the history 
* add composer_network_attachment

* indicate conflicting configs

* commas

* no need for bidirectional conflict definition (generates double errors)

* protect nit PrivateClusterConfig

* for optimizing error messages about conflicts

* add 2 step update for composer_network_attachment

* make composer_network_attachment available in beta only

* add two step update for network and subnetwork

* corrections in 2 phase update for network/subnetwork

* remove composer3 check(CustomizeDiff will solve this), filter api error, add tests (unsetting netwok/subnetwork not working)

* added ForceNewIf fot network/subnetwork, problem with unsetting these fields remains

* add docs for composer_network_attachment

* add test for network attachment

* ignore non empty plan in network attachment test

* add networkAttachment update and conflicting fields tests

* add ComputedIf for network, change isComposer3

* minor corrections

* remove computedIf

* filter equivalent values of network/subnetwork in ForceNewIf

* simplify ResourceConditionFunc, add beta/ga version conditions

* typo

* more general comparison of network references

* use tpgresource.CompareSelfLinkRelativePaths instead of custom function

* modify isComposer3 to avoid merge conflicts later.

* removing this since documentation is handled in other PR and to avoid conflicts while merging.

* replace ExpectNonEmptyPlan with lifecycle.ignore_changes

* add testcase for changing network attachment to network and subnetwork

* add third step to TestAccComposerEnvironmentComposer3_updateWithNetworkAndSubnetwork

* modify tests to use different network for attachment

* remove unused constant

* remove ExpectNonEmptyPlan (already replaced with lifecycle.ignore_changes)
  • Loading branch information
@spapi17 @pengq-google
spapi17 authored and pengq-google committed May 21, 2024
1 parent 6d000e5 commit 8604085
Show file tree
Hide file tree
Showing 2 changed files with 441 additions and 25 deletions.
109 changes: 107 additions & 2 deletions mmv1/third_party/terraform/services/composer/resource_composer_environment.go.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ func ResourceComposerEnvironment() *schema.Resource {
tpgresource.DefaultProviderRegion,
tpgresource.SetLabelsDiff,
<% unless version == "ga" -%>
customdiff.ValidateChange("config.0.software_config.0.image_version", imageVersionChangeValidationFunc),
customdiff.ForceNewIf("config.0.node_config.0.network", forceNewCustomDiff("config.0.node_config.0.network")),
customdiff.ForceNewIf("config.0.node_config.0.subnetwork", forceNewCustomDiff("config.0.node_config.0.subnetwork")),
customdiff.ValidateChange("config.0.software_config.0.image_version", imageVersionChangeValidationFunc),
versionValidationCustomizeDiffFunc,
<% end -%>
),
Expand Down Expand Up @@ -242,17 +244,37 @@ func ResourceComposerEnvironment() *schema.Resource {
Type: schema.TypeString,
Computed: true,
Optional: true,
<% if version == "ga" -%>
ForceNew: true,
<% else -%>
ForceNew: false,
ConflictsWith: []string{"config.0.node_config.0.composer_network_attachment"},
<% end -%>
DiffSuppressFunc: tpgresource.CompareSelfLinkOrResourceName,
Description: `The Compute Engine machine type used for cluster instances, specified as a name or relative resource name. For example: "projects/{project}/zones/{zone}/machineTypes/{machineType}". Must belong to the enclosing environment's project and region/zone. The network must belong to the environment's project. If unspecified, the "default" network ID in the environment's project is used. If a Custom Subnet Network is provided, subnetwork must also be provided.`,
},
"subnetwork": {
Type: schema.TypeString,
Optional: true,
<% if version == "ga" -%>
ForceNew: true,
<% else -%>
ForceNew: false,
Computed: true,
ConflictsWith: []string{"config.0.node_config.0.composer_network_attachment"},
<% end -%>
DiffSuppressFunc: tpgresource.CompareSelfLinkOrResourceName,
Description: `The Compute Engine subnetwork to be used for machine communications, , specified as a self-link, relative resource name (e.g. "projects/{project}/regions/{region}/subnetworks/{subnetwork}"), or by name. If subnetwork is provided, network must also be provided and the subnetwork must belong to the enclosing environment's project and region.`,
Description: `The Compute Engine subnetwork to be used for machine communications, specified as a self-link, relative resource name (e.g. "projects/{project}/regions/{region}/subnetworks/{subnetwork}"), or by name. If subnetwork is provided, network must also be provided and the subnetwork must belong to the enclosing environment's project and region.`,
},
<% unless version == "ga" -%>
"composer_network_attachment": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: false,
Description: `PSC (Private Service Connect) Network entry point. Customers can pre-create the Network Attachment and point Cloud Composer environment to use. It is possible to share network attachment among many environments, provided enough IP addresses are available.`,
},
<% end -%>
"disk_size_gb": {
Type: schema.TypeInt,
Computed: true,
Expand Down Expand Up @@ -1195,6 +1217,68 @@ func resourceComposerEnvironmentUpdate(d *schema.ResourceData, meta interface{})
return err
}

<% unless version == "ga" -%>
noChangeErrorMessage := "Update request does not result in any change to the environment's configuration"
if d.HasChange("config.0.node_config.0.network") || d.HasChange("config.0.node_config.0.subnetwork"){
// step 1: update with empty network and subnetwork
patchObjEmpty := &composer.Environment{
Config: &composer.EnvironmentConfig{
NodeConfig: &composer.NodeConfig{},
},
}
err = resourceComposerEnvironmentPatchField("config.nodeConfig.network,config.nodeConfig.subnetwork", userAgent, patchObjEmpty, d, tfConfig)
if err != nil && !strings.Contains(err.Error(), noChangeErrorMessage){
return err
}

// step 2: update with new network and subnetwork, if new values are not empty
if (config.NodeConfig.Network != "" && config.NodeConfig.Subnetwork != ""){
patchObj := &composer.Environment{
Config: &composer.EnvironmentConfig{
NodeConfig: &composer.NodeConfig{},
},
}
if config != nil && config.NodeConfig != nil {
patchObj.Config.NodeConfig.Network = config.NodeConfig.Network
patchObj.Config.NodeConfig.Subnetwork = config.NodeConfig.Subnetwork
}
err = resourceComposerEnvironmentPatchField("config.nodeConfig.network,config.nodeConfig.subnetwork", userAgent, patchObj, d, tfConfig)
if err != nil {
return err
}
}
}

if d.HasChange("config.0.node_config.0.composer_network_attachment") {
// step 1: update with empty composer_network_attachment
patchObjEmpty := &composer.Environment{
Config: &composer.EnvironmentConfig{
NodeConfig: &composer.NodeConfig{},
},
}
err = resourceComposerEnvironmentPatchField("config.nodeConfig.composerNetworkAttachment", userAgent, patchObjEmpty, d, tfConfig)
if err != nil && !strings.Contains(err.Error(), noChangeErrorMessage){
return err
}

// step 2: update with new composer_network_attachment
if (config.NodeConfig.ComposerNetworkAttachment != ""){
patchObj := &composer.Environment{
Config: &composer.EnvironmentConfig{
NodeConfig: &composer.NodeConfig{},
},
}
if config != nil && config.NodeConfig != nil {
patchObj.Config.NodeConfig.ComposerNetworkAttachment = config.NodeConfig.ComposerNetworkAttachment
}
err = resourceComposerEnvironmentPatchField("config.nodeConfig.composerNetworkAttachment", userAgent, patchObj, d, tfConfig)
if err != nil {
return err
}
}
}
<% end -%>
<% unless version == "ga" -%>
if d.HasChange("config.0.software_config.0.image_version") {
patchObj := &composer.Environment{
Expand Down Expand Up @@ -1853,6 +1937,9 @@ func flattenComposerEnvironmentConfigNodeConfig(nodeCfg *composer.NodeConfig) in
transformed["machine_type"] = nodeCfg.MachineType
transformed["network"] = nodeCfg.Network
transformed["subnetwork"] = nodeCfg.Subnetwork
<% unless version == "ga" -%>
transformed["composer_network_attachment"] = nodeCfg.ComposerNetworkAttachment
<% end -%>
transformed["disk_size_gb"] = nodeCfg.DiskSizeGb
transformed["service_account"] = nodeCfg.ServiceAccount
transformed["oauth_scopes"] = flattenComposerEnvironmentConfigNodeConfigOauthScopes(nodeCfg.OauthScopes)
Expand Down Expand Up @@ -2470,6 +2557,13 @@ func expandComposerEnvironmentConfigNodeConfig(v interface{}, d *schema.Resource
}
transformed.Subnetwork = transformedSubnetwork
}

<% unless version == "ga" -%>
if v, ok := original["composer_network_attachment"]; ok {
transformed.ComposerNetworkAttachment = v.(string)
}
<% end -%>

transformedIPAllocationPolicy, err := expandComposerEnvironmentIPAllocationPolicy(original["ip_allocation_policy"], d, config)
if err != nil {
return nil, err
Expand Down Expand Up @@ -2951,6 +3045,17 @@ func isComposer3(imageVersion string) bool {
return strings.Contains(imageVersion, "composer-3")
}

func forceNewCustomDiff(key string) customdiff.ResourceConditionFunc {
return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool {
old, new := d.GetChange(key)
imageVersion := d.Get("config.0.software_config.0.image_version").(string)
if isComposer3(imageVersion) || tpgresource.CompareSelfLinkRelativePaths("", old.(string), new.(string), nil) {
return false
}
return true
}
}

func imageVersionChangeValidationFunc(ctx context.Context, old, new, meta any) error {
if old.(string) != "" && !isComposer3(old.(string)) && isComposer3(new.(string)) {
return fmt.Errorf("upgrade to composer 3 is not yet supported")
Expand Down
Loading

0 comments on commit 8604085

Please sign in to comment.