Skip to content

New Data Source: alicloud_sae_application_instances - #10153

Open
Explorer1092 wants to merge 1 commit into
aliyun:masterfrom
Explorer1092:datasource-sae-application-instances
Open

New Data Source: alicloud_sae_application_instances#10153
Explorer1092 wants to merge 1 commit into
aliyun:masterfrom
Explorer1092:datasource-sae-application-instances

Conversation

@Explorer1092

Copy link
Copy Markdown

Implements #10152.

What

Adds a new data source alicloud_sae_application_instances that lists the instances of an SAE application, exposing per-instance ENI IP (instance_container_ip), container/health status, image_url, package_version and vswitch_id.

Implementation:

  • group_id unset → all groups are enumerated via GET /pop/v1/sam/app/describeApplicationGroups; group_id set → only that group.
  • Instances are read via GET /pop/v1/sam/app/describeApplicationInstances with pagination (PageSize/CurrentPage) and the standard retry wrapper.
  • Optional client-side ids filtering, output_file, and ids export, following the conventions of alicloud_sae_applications.
  • Documentation page + sidebar entry (website/alicloud.erb) + acceptance test included.

Why

SAE reassigns ENI IPs on every rolling deployment. Downstream IP-based resources (e.g. a self-managed alicloud_alb_server_group fronting an SAE app, needed when listener timeouts beyond the SAE-managed CLB defaults are required) currently have no way to track the new IPs declaratively. Because the alicloud_sae_application update with deploy = true already waits for the ChangeOrder to complete, depends_on lets the data source read the post-deploy IPs within a single apply:

data "alicloud_sae_application_instances" "app" {
  application_id = alicloud_sae_application.app.id
  depends_on     = [alicloud_sae_application.app]
}

resource "alicloud_alb_server_group" "app" {
  server_group_type = "Ip"
  dynamic "servers" {
    for_each = toset(data.alicloud_sae_application_instances.app.instances.*.instance_container_ip)
    content {
      server_id   = servers.value
      server_ip   = servers.value
      server_type = "Ip"
      port        = 4000
    }
  }
}

Testing

  • go build ./alicloud/ and go vet ./alicloud/ pass (the two fmt.Sprintln vet notes in alicloud/common.go are pre-existing upstream).
  • Acceptance test TestAccAlicloudSAEApplicationInstancesDataSource added following the existing SAE data source test pattern (exist: application_id; fake: ids filter). It requires an Alibaba Cloud account to run, so it has not been executed here — happy to adjust if maintainers hit issues running it.

Lists the instances of an SAE application via DescribeApplicationGroups
+ DescribeApplicationInstances, exposing per-instance ENI IP
(instance_container_ip), container/health status, image_url,
package_version and vswitch_id. Supports optional group_id scoping and
client-side ids filtering.
Provides a list of SAE application instances to the user.
---

# alicloud\_sae\_application\_instances

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no need to add the \ any more. can be removed

@xuzhang3

xuzhang3 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the thorough writeup — the use case (ENI IPs churning on every rolling deploy, self-managed ALB server group needing them) is clear, and the issue-then-PR flow is appreciated. Build and vet are clean on my side too; the two fmt.Sprintln notes in common.go are indeed pre-existing.

A few things to fix before this can go in.

1. document-check CI will fail on the doc page

Running the repo's own checker against the new page:

==> Checking docs content of website/docs/d/sae_application_instances.html.markdown ...
[WARNING] line 10: please remove the \
[Error] line 14: Expected: -> **NOTE:** Available since v1.289.0. Got: -> **NOTE:** Available in v1.289.0+.
exit status 1

So: drop the escaping in the # alicloud_sae_application_instances heading (as I noted inline — the surrounding older pages still have it, but the checker no longer accepts it), and use the Available since v1.289.0. wording. v1.289.0 is the right target version.

2. Hard error when a group has no instances

resp, err := jsonpath.Get("$.Data.Instances", response)
if err != nil {
    return WrapErrorf(err, FailedGetAttributeMsg, action, "$.Data.Instances", response)
}

When a group has zero instances, SAE returns Data with only CurrentPage/TotalSize/PageSize and no Instances key, and jsonpath fails with unknown key Instances; a "Data": null body fails with unsupported value type <nil> for select. Either way terraform plan errors out instead of returning an empty list. That is a reachable state for a stopped application, an application scaled to 0 replicas, or — with group_id unset — any one of several groups whose deploy has not produced pods yet, which then fails the whole read.

The current convention in newer data sources is to discard the error and let the type assertion absorb the nil:

resp, _ := jsonpath.Get("$.Data.Instances", response)
result, _ := resp.([]interface{})

result, _ := resp.([]interface{}) below already handles nil correctly, so this is a one-line change. Same treatment for $.Data in the groups loop.

3. Doc examples must be self-contained

Both Example Usage blocks hard-code a literal application_id (and a literal instance ID in the ids example). Two problems:

  • The docs-example CI job actually runs terraform against the example blocks, and a data source pointing at an application ID that does not exist in the test account will fail there.
  • Those values look like they came from a live account. Please replace them with a self-contained example that creates the namespace + application and references alicloud_sae_application.default.id, the way website/docs/d/sae_applications.html.markdown does. If you want to keep a literal for illustration, mask the tail with ****.

4. Naming convention

New files use the AliCloud spelling — dataSourceAliCloudSaeApplicationInstances / dataSourceAliCloudSaeApplicationInstancesRead. The Alicloud form here matches the older neighbouring SAE files, but new code should use the current one.

5. Acceptance test

  • image_url is pinned to registry-vpc.cn-hangzhou.aliyuncs.com/... while the namespace is built from defaultRegionToTest and checkoutSupportedRegions admits every SAE region — outside cn-hangzhou the VPC image pull fails. resource_alicloud_sae_application_test.go templates it: fmt.Sprintf("registry-vpc.%s.aliyuncs.com/...", defaultRegionToTest). (sae_applications_test.go, which this was modelled on, has the same latent problem.)
  • The exist check pins ids.# = 2 and asserts CHECKSET on instance_container_ip. Create waits for the change order to reach {2, 8, 11, 12}, and 8/11/12 are non-success terminals, so pods may not have IPs yet on an unlucky run. Worth confirming when the test runs.

No worries about not having run it — I will run TestAccAlicloudSAEApplicationInstancesDataSource and post the result here.

Optional, but easier now than later

A data source schema is awkward to extend semantically once released, so consider adding while you are here:

  • A status filter on instance_container_status. Your own use case wants only Running pods in the ALB server group — right now Pending/Terminating IPs get fed in too, which is the failure mode you are trying to avoid.
  • eip, instance_container_restarts, and unhealthy_message are already in the response and are the fields people reach for next.
  • DescribeApplicationInstances accepts an InstanceId query param, so ids could be filtered server-side. Client-side is consistent with existing data sources, so this is genuinely optional.
  • Data.TotalSize is returned for instances, so pagination could terminate on it rather than on len(result) < PageSizeLarge — the latter spins forever if the service ever stops honouring CurrentPage. Low risk, just sturdier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants