Skip to content

feat(slot-manager): validate identity pool recovery (AROSLSRE-1896) - #6705

Merged
openshift-merge-bot[bot] merged 4 commits into
Azure:mainfrom
raelga:raelga/arosslsre-1896-slot-identity-pool-docs
Aug 27, 2026
Merged

feat(slot-manager): validate identity pool recovery (AROSLSRE-1896)#6705
openshift-merge-bot[bot] merged 4 commits into
Azure:mainfrom
raelga:raelga/arosslsre-1896-slot-identity-pool-docs

Conversation

@raelga

@raelga Rael Garcia (raelga) commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

AROSLSRE-1896

What

Adds a read-only slot-manager validate-identity-pool command and
validate-identity-pool Make target. The validator expands the canonical slot
catalog, bulk-lists Azure resource groups and user-assigned identities, and
reports missing or unexpected inventory with a non-zero exit code on drift.

Also documents the supported identity-pool reconciliation and validation
workflow in the slot-manager design.

Why

AROSLSRE-1895 found that
DEV shard0 had only 16 of its expected 300 identity-container resource groups.
The recovery target existed, but there was no maintained command for validating
the complete catalog-derived pool after reconciliation.

Testing

  • go test ./cmd/aro-hcp-tests/slot-manager/identity-pool ./cmd/aro-hcp-tests/slot-manager
  • make lint
  • make -C test aro-hcp-tests
  • ./test/aro-hcp-tests slot-manager validate-identity-pool --help
  • Live read-only DEV shard0 validation confirmed all 300 expected resource
    groups and all 3,900 expected managed identities, with zero drift.
  • git diff --check

Special notes for your reviewer

Validation comparisons are case-insensitive because Azure resource names are
case-insensitive and list APIs may return different casing. Pool-level prefixes
are used to detect resources belonging to removed slot indexes.

The apply documentation warns that deployment stacks delete resources and
resource groups that become unmanaged after a catalog change.

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes) - not applicable
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable) - not applicable
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge

If E2E tests are included:

  • E2E tests follow Principles of Good E2E Test Case Design - not applicable
  • If new E2E use case is covered (via a new test or new check/verifier),
    demonstrate that the test is able to detect a defect/error and fail with
    proper error message and logs which communicates nature of the problem. - not applicable

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an operational recovery and validation procedure to the slot-manager design doc for cases where an E2E job fails because its leased managed-identity container resource group is missing.

Changes:

  • Documents how identity pools are derived from test/e2e-config/e2e-slots.yaml and how slot/container naming is expanded.
  • Adds a supported reconciliation flow via make -C test apply-identity-pool, including subscription scoping and deployment-stack behavior.
  • Adds read-only validation commands to confirm the full expected set of resource groups and user-assigned identities exist across the pool.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/cmd/aro-hcp-tests/slot-manager/DESIGN.md Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 10:47
@raelga Rael Garcia (raelga) changed the title docs(slot-manager): document identity pool recovery (AROSLSRE-1896) feat(slot-manager): validate identity pool recovery (AROSLSRE-1896) Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go:70

  • With identities present in the unexpected resource group, the validator should report them as unexpected as well (in addition to the unexpected identity in the expected group).
	if len(result.UnexpectedIdentities) != 1 || result.UnexpectedIdentities[0].Name != "unexpected" {
		t.Fatalf("unexpected identities: %v", result.UnexpectedIdentities)

Comment thread test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate.go Outdated
Comment thread test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go
Copilot AI review requested due to automatic review settings August 27, 2026 11:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread test/cmd/aro-hcp-tests/slot-manager/identity-pool/validate_test.go Outdated
Copilot AI review requested due to automatic review settings August 27, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@raelga

Copy link
Copy Markdown
Collaborator Author

Validated the new read-only target against the restored DEV shard0 pool:

make -C test validate-identity-pool \
  ENVIRONMENT=dev \
  SUBSCRIPTION="ARO HCP E2E Hosted Clusters (EA Subscription)"

Output:

subscription "ARO HCP E2E Hosted Clusters (EA Subscription)":
  resource groups: expected=300 actual=300 missing=0 unexpected=0
  managed identities: expected=3900 actual=3900 missing_in_existing_groups=0 unexpected=0
  result: valid

The command completed successfully (exit status 0) and performed read-only Azure inventory checks.

@mmazur

Copy link
Copy Markdown
Collaborator

/lgtm

@openshift-ci

openshift-ci Bot commented Aug 27, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mmazur, raelga

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@raelga

Rael Garcia (raelga) commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up on recurrence risk: I confirmed the identity pool is currently exposed to the policy-driven cleanup path.

  • All 300 restored shard0 resource groups have persist=true, a newly stamped createdAt, and managedBy=null.
  • tooling/cleanup-sweeper/resourcegroups.policy.yaml has no identity-pool exclusion. Its catch-all persist-true-delete-after-15d rule therefore selects these groups once they are older than 360 hours.
  • The periodic sweeper-rg-ordered job runs every four hours across the DEV customer/infra subscriptions using that policy.
  • The older hourly cleanup resource-groups --expired path is not the match here because it only selects groups tagged e2e.aro-hcp-ci.redhat.com=true with an expired deleteAfter.aro-hcp-ci.redhat.com; the identity-container groups do not have those tags.

Based on the current createdAt range, these groups become eligible around 2026-09-11 10:30–11:25 UTC unless the cleanup policy excludes the managed identity pools first. This PR provides read-only detection and recovery. Prevention is tracked by AROSLSRE-1916 and implemented in PR #6706.

@openshift-merge-bot
openshift-merge-bot Bot merged commit f9b2c34 into Azure:main Aug 27, 2026
15 checks passed
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.

3 participants