zedkube/edgeview: fix stale vmiVNC.run blocking new VNC sessions by naiming-zededa · Pull Request #5875 · lf-edge/eve

naiming-zededa · 2026-04-28T03:58:15Z

Description

Problem: vmiVNC.run was not being cleaned up when an edgeview instance crashed or was killed.

Fixes:

clustertypes.go: Add AppUUID field to VmiVNCConfig to identify file ownership per-app. Add OwnerAlive() which reads /proc//comm and checks the name is edge-view, guarding against PID reuse by unrelated processes.
zedkube/vnc.go: Replace the blind os.Stat block with canClaimVNCFile(). It evicts stale files (dead edgeview PID, reused PID, or idle virtctl port) and blocks only on a genuinely live session — a running edgeview process or a virtctl proxy still listening on the port.
edgeview/network.go: Replace the blind os.Stat block with removeStaleVNCFile(), the edgeview-side equivalent. Also call it at edgeview startup (runOnServer) with evictIdle=false to clear files left from a previous crash without disturbing a live remote-console session. Rewrite isPortListening to read /proc/net/tcp directly instead of spawning ss (avoids disrupting the virtctl WebSocket connection).
vnc-proxy.sh: Move monitor_caller_pid start to the top of handle_vnc before the virtctl retry loop

PR dependencies

How to test and validate this PR

run the edgeview VNC in multiple times, and verify they are not blocked by the stale config file on the device.

Changelog notes

zedkube/edgeview: fix stale vmiVNC.run blocking new VNC sessions

PR Backports

Checklist

I've provided a proper description
I've added the proper documentation
I've tested my PR on amd64 device
I've tested my PR on arm64 device
I've written the test verification instructions
I've set the proper labels to this PR

For backport PRs (remove it if it's not a backport):

I've added a reference link to the original PR
PR's title follows the template

And the last but not least:

I've checked the boxes above, or I've provided a good reason why I didn't
check them.

codecov · 2026-04-28T05:35:03Z

Codecov Report

❌ Patch coverage is 31.81818% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.12%. Comparing base (2281599) to head (6f0c5c2).
⚠️ Report is 633 commits behind head on master.

Files with missing lines	Patch %	Lines
...f-edge/eve/pkg/pillar/utils/netutils/portlisten.go	0.00%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5875      +/-   ##
==========================================
- Coverage   19.52%   17.12%   -2.41%     
==========================================
  Files          19      475     +456     
  Lines        3021    85683   +82662     
==========================================
+ Hits          590    14669   +14079     
- Misses       2310    69496   +67186     
- Partials      121     1518    +1397

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

milan-zededa · 2026-04-30T08:08:39Z

            logmsg "monitor_caller_pid: Caller PID $caller_pid is gone (crashed?), cleaning up"

+            # Re-check CallerPID in the file before acting, in case a new edgeview
+            # process with a recycled PID already wrote a new session file.


Is this really for "recycled PID" if we are checking for changed CallerPID?

updated comments.

milan-zededa · 2026-04-30T08:10:51Z

+│   ┌─────────┐  AppInstanceConfig   ┌──────────┐                       │
+│   │zedagent │─────────────────────►│ zedkube  │                       │
+│   └─────────┘   (RemoteConsole)    └────┬─────┘                       │
+│                                        │ runAppVNC()                  │


Add space to fix the arrow

milan-zededa · 2026-04-30T08:12:57Z

+│   │edgeview  │◄────────────────────│ edgeview │                       │
+│   │ client   │                     │ (server) │                       │
+│   └──────────┘                     └────┬─────┘                       │
+│                                         │ setupEveKVNC()              │


Is edgeview used also for Controller-triggered VNC access? Based on this diagram it seems that zedkube talks to edgeview to setup VNC.

redo the drawing to reflect the schemes properly.

milan-zededa · 2026-04-30T08:15:40Z

+
+Rules:
+
+- `CallerPID > 0` → edgeview owns this session; liveness via `/proc/<pid>/comm`


I believe that in the edgeview-case, the liveness is both PID-check and port-listen check (?)
But the port-listen check is not mentioned for CallerPID > 0

yes, we actually missing the code in checking the vnc port. updated the code and the doc.

milan-zededa · 2026-04-30T08:17:44Z

+    │                        │                │ go runAppVNC()     │                │               │              │
+    │                        │                │ getVMIdomainName() │                │               │              │
+    │                        │                │ (retry up to 6×)   │                │               │              │
+    │                        │                │ canClaimVNCFile()  │                │               │              │


Shoule be here also the "evict stale" step (as shown for the edgeview case below)?

milan-zededa · 2026-04-30T08:20:37Z

+
+Both `removeStaleVNCFile` (edgeview) and `canClaimVNCFile` (zedkube) probe
+whether a virtctl proxy is actually running before declaring a remote-console
+file stale. They do this differently:


Do they really do it differently? They use the same helper function after all

milan-zededa · 2026-04-30T08:22:07Z

+        │
+        ▼ no
+   parsable &
+   VNCPort > 0? ──no──► remove file ──► return true


I feel like there can be a race condition between edgeview and zedkube. These file/PID/port checks are not atomic.
But I guess this is up to the user to avoid opening multiple VNC sessions from these two places in parallel.

i have added the error messages in place in case this happens.

Problem: vmiVNC.run was not being cleaned up when an edgeview instance crashed or was killed. Fixes: - clustertypes.go: Add AppUUID field to VmiVNCConfig to identify file ownership per-app. Add OwnerAlive() which reads /proc/<pid>/comm and checks the name is edge-view, guarding against PID reuse by unrelated processes. - zedkube/vnc.go: Replace the blind os.Stat block with canClaimVNCFile(). It evicts stale files (dead edgeview PID, reused PID, or idle virtctl port) and blocks only on a genuinely live session — a running edgeview process or a virtctl proxy still listening on the port. - edgeview/network.go: Replace the blind os.Stat block with removeStaleVNCFile(), the edgeview-side equivalent. Also call it at edgeview startup (runOnServer) with evictIdle=false to clear files left from a previous crash without disturbing a live remote-console session. Rewrite isPortListening to read /proc/net/tcp directly instead of spawning ss (avoids disrupting the virtctl WebSocket connection). - vnc-proxy.sh: Move monitor_caller_pid start to the top of handle_vnc before the virtctl retry loop Signed-off-by: naiming-zededa <naiming@zededa.com>

naiming-zededa requested review from eriknordmark and zedi-pramodh as code owners April 28, 2026 03:58

github-actions Bot requested review from andrewd-zededa and christoph-zededa April 28, 2026 03:58

naiming-zededa force-pushed the naiming-vnc-cleanup branch 2 times, most recently from b49eec0 to b32e0c1 Compare April 28, 2026 04:13

milan-zededa reviewed Apr 28, 2026

View reviewed changes

Comment thread pkg/edgeview/src/network.go

naiming-zededa force-pushed the naiming-vnc-cleanup branch from b32e0c1 to a2833f4 Compare April 28, 2026 17:19

github-actions Bot requested a review from milan-zededa April 28, 2026 17:19

milan-zededa reviewed Apr 30, 2026

View reviewed changes

naiming-zededa force-pushed the naiming-vnc-cleanup branch from a2833f4 to b257790 Compare April 30, 2026 20:07

github-actions Bot requested a review from milan-zededa April 30, 2026 20:08

naiming-zededa force-pushed the naiming-vnc-cleanup branch from b257790 to 79515e0 Compare April 30, 2026 21:07

naiming-zededa force-pushed the naiming-vnc-cleanup branch from 79515e0 to 6f0c5c2 Compare May 1, 2026 03:58


		Rules:

		- `CallerPID > 0` → edgeview owns this session; liveness via `/proc/<pid>/comm`

Conversation

naiming-zededa commented Apr 28, 2026

Description

PR dependencies

How to test and validate this PR

Changelog notes

PR Backports

Checklist

Uh oh!

codecov Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milan-zededa Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milan-zededa Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Apr 28, 2026 •

edited

Loading

milan-zededa Apr 30, 2026 •

edited

Loading

milan-zededa Apr 30, 2026 •

edited

Loading