Skip to content

feat: implement the TAG action to re-tag untagged instances - #62

Merged
michaelcoburn merged 1 commit into
percona:mainfrom
michaelcoburn:fix/tag-instances
Jun 2, 2026
Merged

feat: implement the TAG action to re-tag untagged instances#62
michaelcoburn merged 1 commit into
percona:mainfrom
michaelcoburn:fix/tag-instances

Conversation

@michaelcoburn

Copy link
Copy Markdown
Collaborator

Closes #53

Problem

tagInstances() (the TAG action) was never finished — it built a name list, then hit // TODO: finish this and exit() before doing anything. The dead code beneath it was also buggy (unset($instanceNames) cleared the entire array on the first match, and it assumed the Name tag was always at index 0). So -a TAG was a no-op.

What it does now

Re-tags instances that failed to get a Name tag at creation (the scenario the original comment describes):

  1. Requires -m and -c so the expected Name set can be rebuilt — mirrors the exact naming in addNewInstance() (Percona-Training-<suffix>-<machine>-T<team>, including the scoreboardT0 special case).
  2. Lists VPC instances (excluding terminated), splitting them into already-named vs. untagged, reading the Name tag robustly.
  3. Computes the still-missing names (expected − taken) and assigns them to the untagged instances, re-applying Name + TrainingEndDate.
  4. Safety guard: if there are more untagged instances than missing names (e.g. multiple machine types in play), it refuses rather than risk mis-tagging, and tells the operator to re-run ADD instead.

Limitation (documented in code)

An untagged instance gives no hint of its intended machine type, so this is only reliable for a single machine type per run. Noted in the function comment.

Verification

php -l passes. ⚠️ Not yet exercised against live AWS — the EC2 describeInstances/createTags path needs a validation run (ideally by reproducing the original "instance failed to tag" case, or by manually untagging an instance and running -a TAG -m <type> -c <n>).

tagInstances() was a stub (built a name list, then hit a TODO + exit()),
and 'TAG' was also missing from the $actions whitelist -- so `-a TAG`
printed help and was never reachable. Now:

- Register 'TAG' in $actions so the action is actually accepted.
- Require -m and -c so the expected Name set can be rebuilt (mirrors
  addNewInstance naming, incl. the scoreboard T0 case).
- List VPC instances (excluding terminated), split into named vs untagged
  (reading the Name tag robustly), assign the still-missing names to the
  untagged instances, and re-apply Name + TrainingEndDate.
- Safety: if untagged count > missing names (e.g. multiple machine types),
  abort rather than risk mis-tagging.

Validated live (us-west-2): re-tag of one untagged instance, the safety
guard abort, and a full two-instance re-tag all behaved correctly.

Closes percona#53

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@michaelcoburn

Copy link
Copy Markdown
Collaborator Author

Live-tested in us-west-2 — now passing.

Testing first surfaced a second bug: -a TAG printed the help text and exited, because TAG was missing from the $actions whitelist in start-instances.php (it was in the switch but never accepted by parseOptions) — so the action was unreachable even before this PR. Fixed here by registering TAG in $actions (and the branch is rebased onto current main, which now has the merged Makefile fixes).

End-to-end validation (2× db1, no Ansible — just the tag path):

Scenario Result
Untag one instance → -a TAG -c 2 ✅ re-tagged …-db1-T1 (Name + TrainingEndDate restored)
Untag both-a TAG -c 1 (untagged 2 > missing 1) aborted: "Found 2 untagged instance(s) but only 1 expected name(s) are missing. Re-run ADD…"
Proper re-tag → -a TAG -c 2 ✅ both instances re-tagged

⚠️ Related finding from this exercise (separate from this PR): make teardown searches by Name tag, so an instance left untagged (e.g. the failure mode this very feature exists to repair) escapes teardown and is silently reported as "0 remaining." I hit exactly this and had to terminate two orphaned instances manually. Filing that separately — flagging here since TAG is the intended remediation tool for untagged instances.

@michaelcoburn
michaelcoburn merged commit 8e3d51b into percona:main Jun 2, 2026
4 checks passed
@michaelcoburn
michaelcoburn deleted the fix/tag-instances branch June 2, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement incomplete 'TAG' functionality in start-instances.php

1 participant