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

Support connectivity fields for Composer 3 #9889

Merged
merged 41 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
fbb1a7d
add composer_network_attachment
spapi17 Dec 6, 2023
ba9a1f5
indicate conflicting configs
spapi17 Dec 11, 2023
c68d7c7
commas
spapi17 Dec 11, 2023
83a24e3
no need for bidirectional conflict definition (generates double errors)
spapi17 Dec 11, 2023
53ed66b
protect nit PrivateClusterConfig
spapi17 Dec 11, 2023
4ecb5cd
for optimizing error messages about conflicts
spapi17 Dec 11, 2023
34f9c5f
add 2 step update for composer_network_attachment
spapi17 Dec 11, 2023
3713ba3
make composer_network_attachment available in beta only
spapi17 Dec 18, 2023
4898509
add two step update for network and subnetwork
spapi17 Dec 18, 2023
aa9c9d4
Merge branch 'main' into network
spapi17 Dec 18, 2023
33db174
corrections in 2 phase update for network/subnetwork
spapi17 Dec 19, 2023
713e34e
Merge branch 'GoogleCloudPlatform:main' into network
spapi17 Jan 17, 2024
654aec6
remove composer3 check(CustomizeDiff will solve this), filter api err…
spapi17 Jan 18, 2024
e4f61ed
Merge branch 'GoogleCloudPlatform:main' into network
spapi17 Jan 18, 2024
5383cf6
merge main
spapi17 Jan 19, 2024
4239e3a
Merge branch 'main' into network
spapi17 Jan 19, 2024
f719deb
added ForceNewIf fot network/subnetwork, problem with unsetting these…
spapi17 Jan 22, 2024
51bf71a
add docs for composer_network_attachment
spapi17 Jan 25, 2024
ac103fc
add test for network attachment
spapi17 Jan 26, 2024
0ce121e
ignore non empty plan in network attachment test
spapi17 Jan 26, 2024
4d48140
add networkAttachment update and conflicting fields tests
spapi17 Jan 26, 2024
0b3c252
add ComputedIf for network, change isComposer3
spapi17 Jan 29, 2024
29e4dd7
Merge branch 'main' into network
spapi17 Jan 29, 2024
4546db2
minor corrections
spapi17 Jan 29, 2024
b3cbf7b
remove computedIf
spapi17 Feb 4, 2024
1f82190
filter equivalent values of network/subnetwork in ForceNewIf
spapi17 Feb 6, 2024
4a577a8
simplify ResourceConditionFunc, add beta/ga version conditions
spapi17 Feb 7, 2024
4aa9684
typo
spapi17 Feb 7, 2024
62ce208
more general comparison of network references
spapi17 Feb 9, 2024
4d3c0a5
use tpgresource.CompareSelfLinkRelativePaths instead of custom function
spapi17 Feb 19, 2024
b8d0f9e
modify isComposer3 to avoid merge conflicts later.
spapi17 Feb 19, 2024
3c129c2
removing this since documentation is handled in other PR and to avoid…
spapi17 Feb 20, 2024
9021018
replace ExpectNonEmptyPlan with lifecycle.ignore_changes
spapi17 Feb 22, 2024
d293e8f
Merge branch 'main' into network
spapi17 Feb 22, 2024
7ec49b0
add testcase for changing network attachment to network and subnetwork
spapi17 Feb 22, 2024
6185aad
Merge branch 'network' of github.com:spapi17/magic-modules into network
spapi17 Feb 22, 2024
250452e
add third step to TestAccComposerEnvironmentComposer3_updateWithNetwo…
spapi17 Feb 22, 2024
684adb9
modify tests to use different network for attachment
spapi17 Feb 23, 2024
1bf57c7
remove unused constant
spapi17 Feb 26, 2024
321d558
merge main
spapi17 Feb 26, 2024
528f9d8
remove ExpectNonEmptyPlan (already replaced with lifecycle.ignore_cha…
spapi17 Feb 26, 2024
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
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": {
Copy link
Member

Choose a reason for hiding this comment

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

Oh, we'll need to add handwritten documentation for the field: https://googlecloudplatform.github.io/magic-modules/develop/resource/#add-documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a separate PR for documenting Composer 3 argument usage.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! This is covered there, sgtm

Type: schema.TypeString,
Computed: true,
rileykarson marked this conversation as resolved.
Show resolved Hide resolved
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
Loading