apme/v5-deeper-marker-find #8

Merged
anaeem merged 20 commits from apme/v5-deeper-marker-find into main 2026-05-24 16:18:30 +00:00
Owner
No description provided.
Two v5 bugs surfaced when validating against fresh WebSphere VMs:

1. roles/discover/tasks/gather/filesystem.yml — the marker-file find used
   -maxdepth 4 from each scan_dir root. WebSphere Liberty's ws-server.jar
   sits at /opt/ibm/wlp/bin/tools (depth 5 from /opt) and traditional WAS's
   WAS.product sits at /opt/IBM/WebSphere/AppServer/properties/version
   (depth 6). Both were silently dropped, leaving the websphere /
   websphere_liberty detectors with only the systemd-service method
   matching — 1 method → low confidence → no fact file emitted. Bumped to
   maxdepth 8.

2. playbooks/utils/apply-v5-to-aap.yml — two fail_msg strings used the
   Jinja expression {{ project_name!r if false else project_name }} which
   relies on Python's !r repr filter. Modern Ansible (2.20+) lazy-parses
   templates and chokes on !r before the 'if false' short-circuit kicks in.
   Replaced with plain quoted expansion.

   Also added a localhost_inventory_name variable (default 'Localhost')
   so installs with a differently-named utility inventory (e.g. 'Demo
   Inventory') don't need to create a new inventory just to satisfy
   apply-v5-to-aap.yml's hardcoded GET.
The Phase-3 filesystem gather missed both WebSphere marker files because
they live deeper than 4 levels under /opt:
  /opt/ibm/wlp/bin/tools/ws-server.jar      (depth 5)
  /opt/IBM/WebSphere/AppServer/properties/version/WAS.product (depth 6)

Without these in discovered_facts.marker_files, the websphere and
websphere_liberty detectors fell back to matching only on systemd
service name — a single method → 'low' confidence → no fact file
emitted, so deep_inspect and the per-app pipeline never ran for any
WebSphere host.

Verified empirically against fresh KubeVirt VMs in test-fleet:
  websphere-liberty (172.16.2.4)      — Open Liberty 24.0.0.6
  websphere-traditional (172.16.2.10) — WAS shim (server1 + 3 EARs)
Found while validating WebSphere detection. Across the gather phase
(processes.yml, filesystem.yml, java_envs.yml, assemble.yml) the
construct `(X.stdout_lines | string)` was used to coerce a possibly-
None register output to something the next pipe could chew on. In
modern Jinja, `list | string` returns the Python repr — a string like
`"['line1', 'line2']"` — and the downstream `[1:]` slices the
string, `| list` iterates *characters*, `map('regex_replace', ...)`
runs once per character, and `for line in ...` iterates characters
too.

Net effect: discovered_marker_files, discovered_processes,
discovered_process_commands, discovered_ports (in the parse loop),
discovered_java_environments (parse loop), and environment (in
assemble.yml) all degraded to silent garbage on every host. The
detector then matched almost nothing via the process / filesystem-
marker / package paths, leaving every host at 'low' confidence unless
the systemd-service or shallow-directory check happened to fire on
its own.

Concretely surfaced on websphere-liberty and websphere-traditional
VMs in test-fleet: marker_files was a list of literal chars like
['[', "'", '/', 'o', 'p', 't', ...], so detector regex_search for
'/opt/ibm/wlp' against single chars produced zero hits.

Fix is mechanical — drop the '| string' coercion; `X.stdout_lines |
default([])` already handles the missing-register case correctly
(returns [] not None).
Same '(X.stdout_lines | string)' bug found in 8 more sites across
build_analysis, java/{log_config,keystores,cron,network},
war/introspect, and extract_single. With detection now reaching
medium/high confidence on WebSphere VMs (post earlier fixes), the
deep-inspect tasks finally executed and surfaced this latent bug —
'dict object has no attribute stdout_lines' raised when iterating a
mangled character-array as a list.

Also tightened the conditional in extract_single.yml lines 128-129:
the previous '(X.stdout_lines | string) is defined' check evaluated
the dotted expression before the defined-check, which races. Replaced
with a plain 'X is defined' guard plus '| default([])' on stdout_lines.
java_common.yml line 48 references playbooks/java/secrets.yml but
the file was never created in the v5 readability split (commit
f702d3a2). Deep-inspect failed with 'Could not find or access' for
every host that reached deep-inspect.

Ported the secrets-detection block from the pre-split java_common.yml
(found in .ansible/collections cache from a prior v4 run). Same
contract: emits _di_secrets_found as list of '<filename>:<attribute>:<path>'
markers without ever reading the secret value itself.

Also forces the file past .gitignore — repo-root .gitignore pattern
'secrets' was matching this filename.
playbooks/utils/fan_out.yml asserts on discovered_apps[] being
defined as a workflow artifact, but the discover role only wrote
discovered_apps.json to disk on localhost — it never called set_stats
to publish the same payload into the AAP workflow's stats stream.

Result: the e2e workflow always died at JT-E (fan_out) with
'fan-out.yml requires: discovered_apps[] (from JT-A workflow
artifact), Got: discovered_apps_len=0'. Surfaced once the earlier
gather/detect bugs were fixed and scan_fleet started emitting real
fact files.

Added a set_stats call right after the discovered_apps.json write,
sharing the same Jinja that builds the file. aggregate:false +
per_host:false keeps the artifact a clean list[dict] for fan_out's
loop.
New Overview-tier page targeted at slide-deck use: a single mermaid
architecture diagram (source/target panes with AAP, MTA, Git between
them) plus short prose. Names only the tools we actually use today --
no RHDH, KAI, Dev Spaces, playbook names, or JT IDs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fan_out.yml line 69 hard-required app.mta_app_id (set only by
publish_to_mta) and app.host_fqdn (flat, never present in the v5
discover schema — discover writes app.host.fqdn nested). With the
e2e workflow's current shape (scan_fleet → approval → pair_apps(noop)
→ fan_out), publish_to_mta is not in the upstream chain, so
discovered_apps comes straight from discover and lacks both fields.

Default mta_app_id to 0 (analyzer-quota lookup tolerates this) and
fall back to the nested app.host.fqdn when the flat host_fqdn isn't
present. Both reads stay safe when discovered_apps was emitted from
the post-publish-to-mta chain that does set those fields.
JT 121 (per_app) has a Survey that AAP /launch/ enforces — when
fan_out POSTed extra_vars without source_repo / source_branch /
source_subpath, the launch was rejected silently (no_log:true on
the uri task masked the 400). Now emits the survey's three keys
alongside the existing repo_url/repo_branch (kept for back-compat
with v4 consumers). Also added top-level 'slug' since pair tasks
key off it.
Adds the parts the previous diagram glossed over: per-app fan-out,
GREEN/YELLOW/RED verdict branching, the two human gates (pair source
repo, PR review+merge), MTA bidirectional feedback into containerize,
and the drift-detect feedback loop closing back to discovery.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AAP runner pods are ephemeral — scan_fleet writes <slug>.facts.json
to its pod's /tmp, then dies. fan_out spawns N per_app jobs that
land on *different* runner pods and can't see the file. The
discover_output_dir extra_var only renames the (still pod-local)
path; it doesn't make /tmp shared.

Workaround: fan_out already has the full fact data in the
discovered_apps workflow artifact (the set_stats publish from
roles/discover/tasks/emit_fact_file.yml). Now passes the whole
app dict inline via the per-launch extra_var app_fact_inline.

per_app.yml grows two new tasks that materialize that inline payload
to {{ discover_output_dir }}/{{ slug }}.facts.json before the stat
check fires. When app_fact_inline isn't set (e.g. a hand-launched
JT-98 run from a developer laptop where discover and per_app share
/tmp), the materialization is skipped and the original 'discover
must run first' guard still applies.
Pairs with the prior fan_out commit. fan_out now passes the full
app dict as the app_fact_inline extra_var; per_app.yml writes it
to {{ discover_output_dir }}/{{ slug }}.facts.json before the stat
check fires. When app_fact_inline isn't set (laptop run with
shared /tmp), the materialization is skipped and the 'discover must
run first' guard still applies.
Container render derived _rm_struct.slug from app.id (e.g. 'websphere_liberty')
while emit_fact_file.yml writes app.slug as '<hostname>-<app_id>'
('websphere-liberty-websphere-liberty', regex-cleaned). Deploy then
looks at publish_bundle_source_dir/<app_slug>/ — the host-prefixed
form — and 404s.

Use app.slug (fall back to app.id) so both render and deploy agree
on the directory name. Same fix on image_ref so the produced image
name is unique-per-host.
Four new sub-pages under the Tomcat walkthrough, one per phase
(Discover, Verdict+Pair, Containerize+Assess, Deploy+Drift). Each
focuses on the data artifacts flowing in and out of that phase so a
single mermaid diagram can be lifted straight into a slide.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
git rev-parse on a missing ref prints the ref string itself on stderr
('origin/bundle/<slug>') and returns 128 — but the previous script
captured stdout (which was empty) into REMOTE_SHA, then the '|| echo ""'
covered the exit. Net result: REMOTE_SHA was empty for missing refs but
SOMETIMES the ref name leaked through, and force-with-lease then choked
with 'cannot parse expected object name'.

Fix:
* git rev-parse --verify -q refs/remotes/origin/<branch> — atomic
  contract: prints the SHA on success, prints nothing & exits non-zero
  on missing ref. -q suppresses stderr.
* Use 'git push -u' on the brand-new-branch path so the local branch
  starts tracking origin/<branch> after the first push (the next run's
  fetch + rev-parse then works on the fast path).

Restored no_log on the calling task.
Previous commit fe38948 had the right commit message but accidentally
only included the no_log restoration in git_pr.yml — the git_push.sh
hunk never made it because the Edit tool's read-cache check rejected
the file modification silently. The .sh on the branch still had the
broken rev-parse path, so every per-app push exited 129 with 'cannot
parse expected object name origin/bundle/<slug>'.

This commit actually delivers the change documented in fe38948:
use 'git rev-parse --verify -q refs/remotes/...' (no fallback echo)
and 'git push -u' on the brand-new-branch path.
llm/context.yml line 35 set _manifest_dir using app.id ('websphere_liberty')
while the K8s render path uses app.slug ('daytrader-liberty-websphere-liberty'
— hostname-prefixed). The LLM-authored Containerfile got written to the
app.id dir; deploy copied from the app.slug dir and dropped only the 5
K8s manifests + VALIDATED.txt — no Containerfile in the bundle.

Mirrored the app.slug fallback pattern from render.yml so both write
to the same per-app directory.
emit_fact_file.yml computed slug (= hostname-appid, regex-cleaned) only
at fact-file write time. Downstream consumers that loop over
detected_applications with loop_var=app (containerize's
llm_containerfile.yml is the main one) didn't see app.slug — it was
synthesized later in read_fact.yml's reconstructed app dict, but the
loop variable shadowed that.

Net effect: the loop body's '{{ app.slug | default(app.id) }}' fell
through to app.id, and the LLM Containerfile landed in
/tmp/discovery-reports/manifests/<app.id>/ (e.g. 'tomcat') instead of
.../manifests/<hostname-appid>/ where deploy reads. Two parallel app
hosts with the same app.id (e.g. tomcat-petclinic and tomcat-roller
both detect 'tomcat') would even race-write the same dir.

Compute slug at detect_single.yml using the same '<inventory_hostname>-<id>'
formula emit_fact_file uses, so every consumer that reads
detected_applications now has slug from the first moment.
fix(v5): read_fact propagates slug into the synthetic detected_applications
Some checks failed
APME static analysis / apme-check (pull_request) Has been cancelled
e3c57ca91f
per_app.yml never runs discover — it reads a pre-emitted fact file and
synthesizes a singleton detected_applications list for the legacy
containerize/llm code path that loops with loop_var=app. That singleton
omitted 'slug', so inside the loop 'app.slug | default(app.id)' fell
through and _manifest_dir resolved to .../manifests/<app.id>/ (e.g.
'tomcat') instead of .../manifests/<hostname-appid>/.

Net effect: LLM-authored Containerfile landed in the wrong directory;
deploy phase copied from the correct (slug-named) directory and didn't
ship a Containerfile in any bundle, even though one was generated per
job.

Bug #13 added slug to detected_applications inside detect_single.yml
(the discover-time path). This commit fixes the per_app re-projection
path that bypasses detect_single entirely.
anaeem merged commit a1f62cf52f into main 2026-05-24 16:18:30 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
anaeem/ansible-collection-discovery!8
No description provided.