Skip to content

PMM-15266 Bring back the update status endpoint - #5733

Open
ademidoff wants to merge 5 commits into
mainfrom
PMM-15266-bring-back-update-status-endpoint
Open

PMM-15266 Bring back the update status endpoint#5733
ademidoff wants to merge 5 commits into
mainfrom
PMM-15266-bring-back-update-status-endpoint

Conversation

@ademidoff

@ademidoff ademidoff commented Aug 5, 2026

Copy link
Copy Markdown
Member

Ticket number: PMM-15266

Feature build: SUBMODULES-4508

PMM-14969 (#5394) removed StartUpdate and UpdateStatus together with the GUI-triggered upgrade flow. Pre-3.9 clients, however, keep polling /v1/server/updates:getStatus after they trigger an update - that is how they learn the freshly started server has finished initializing. Without it they never leave the "in progress" state when migrating to 3.9.0.

Restore UpdateStatus only. StartUpdate stays removed, so the auth token file it used to write (/srv/pmm-update.json) is gone as well and the token can no longer be verified: it was issued by the instance this one replaced. Requests carrying one are accepted as is.

Completion now comes from the supervisord state of pmm-init, the program that runs the initialization and upgrade tasks. Unlike the maintenance mode marker, supervisord reports a program as running from the moment it is spawned, so there is no window right after a restart in which the endpoint would wrongly report an update as complete.

The nginx maintenance mode bypass listed /v1/updates/Status, a PMM 2 path that has matched nothing in 3.x, so the progress log never reached pmm-managed while an upgrade was running. It now names the actual endpoint.

PMM-14969 (#5394) removed StartUpdate and UpdateStatus together with the
GUI-triggered upgrade flow. Pre-3.9 clients, however, keep polling
/v1/server/updates:getStatus after they trigger an update - that is how
they learn the freshly started server has finished initializing. Without
it they never leave the "in progress" state when migrating to 3.9.0.

Restore UpdateStatus only. StartUpdate stays removed, so the auth token
file it used to write (/srv/pmm-update.json) is gone as well and the
token can no longer be verified: it was issued by the instance this one
replaced. Requests carrying one are accepted as is.

Completion now comes from the supervisord state of pmm-init, the program
that runs the initialization and upgrade tasks. Unlike the maintenance
mode marker, supervisord reports a program as running from the moment it
is spawned, so there is no window right after a restart in which the
endpoint would wrongly report an update as complete.

The nginx maintenance mode bypass listed /v1/updates/Status, a PMM 2 path
that has matched nothing in 3.x, so the progress log never reached
pmm-managed while an upgrade was running. It now names the actual
endpoint.
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.78947% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.44%. Comparing base (31318c7) to head (2f916bf).
⚠️ Report is 92 commits behind head on main.

Files with missing lines Patch % Lines
managed/services/supervisord/supervisord.go 61.76% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5733      +/-   ##
==========================================
+ Coverage   43.59%   45.44%   +1.85%     
==========================================
  Files         415      418       +3     
  Lines       43134    43363     +229     
==========================================
+ Hits        18804    19708     +904     
+ Misses      22454    21713     -741     
- Partials     1876     1942      +66     
Flag Coverage Δ
admin 34.96% <ø> (+0.17%) ⬆️
agent 51.12% <ø> (+2.07%) ⬆️
managed 45.02% <65.78%> (+2.04%) ⬆️
vmproxy 72.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Only "done" matters to pre-3.9 clients, so stop serving the pmm-init
progress log: log_lines and log_offset always marshal as their default
values, and auth_token is accepted without being verified. All three
fields are marked deprecated in the proto.

Dropping the log removes Updater.InitLog along with the
gRPCMessageMaxSize it needed to cap a single response, which restores
updater.go and main.go to their pre-restoration state. The offset a
client carries over refers to the log of the instance it polled before
the update, so replaying from it could only return the previous boot's
output.

Also drop two documentation rows left stale by PMM-14969: the removed
/v1/server/updates:start endpoint and the "Start updates" permission,
which no longer has a UI trigger.

Signed-off-by: Alex Demidoff <a@demidoff.me>
@ademidoff
ademidoff marked this pull request as ready for review August 5, 2026 15:45
@ademidoff
ademidoff requested review from a team as code owners August 5, 2026 15:45
@ademidoff
ademidoff requested review from JiriCtvrtka, matejkubinec and maxkondr and removed request for a team and JiriCtvrtka August 5, 2026 15:45
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Very well. The PR adds UpdateStatus through the API contract, generated client, server handler, supervisord status reporting, unauthenticated routing, legacy cleanup, and integration tests.

Changes

Update status

Layer / File(s) Summary
Define UpdateStatus API contract
api/server/v1/server.proto, api/server/v1/json/..., api/server/v1/json/v1.json, api/swagger/...
Adds the UpdateStatus RPC, compatibility request and response fields, validation code, OpenAPI definitions, and generated Go client support.
Report initialization status
managed/services/server/..., managed/services/supervisord/...
Adds Server.UpdateStatus, maps Done to the pmm-init program state, and captures supervisorctl status output for parsing. Tests cover running and stopped initialization states.
Expose and verify endpoint
managed/services/grafana/auth_server.go, build/ansible/..., documentation/docs/admin/roles/index.md, api-tests/server/updates_test.go
Allows unauthenticated access to the new endpoint, updates maintenance-mode routing, removes obsolete update permissions, cleans legacy token files, and tests tokenless requests.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant UpdateStatus
  participant Supervisord
  Client->>UpdateStatus: POST /v1/server/updates:getStatus
  UpdateStatus->>Supervisord: ProgramRunning("pmm-init")
  Supervisord-->>UpdateStatus: Running state
  UpdateStatus-->>Client: Done status and compatibility fields
Loading
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: restoring the update status endpoint.
Description check ✅ Passed The description includes the ticket, feature build, motivation, implementation details, compatibility behavior, and endpoint changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Review ran into problems

🔥 Problems

Repository analysis: Couldn't refresh percona/pmm clone - clone failed: Clone operation failed: remote: Invalid username or token. Password authentication is not supported for Git operations.
fatal: Authentication failed for 'https://github.com/percona/pmm.git/'


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/server/v1/json/client/server_service/server_service_client.go`:
- Around line 460-475: Update the OpenAPI definition for UpdateStatus so
AuthToken is transmitted only over HTTPS, then regenerate the client so the
UpdateStatus operation no longer permits the http scheme when
UpdateStatusBody.AuthToken is present. Preserve the existing UpdateStatus client
behavior otherwise.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a4193a5-c462-4f80-9d13-a41ea6eb67c3

📥 Commits

Reviewing files that changed from the base of the PR and between 13c3072 and 2f916bf.

⛔ Files ignored due to path filters (4)
  • api/descriptor.bin is excluded by !**/*.bin
  • api/server/v1/server.pb.go is excluded by !**/*.pb.go
  • api/server/v1/server.pb.gw.go is excluded by !**/*.pb.gw.go
  • api/server/v1/server_grpc.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (19)
  • api-tests/server/updates_test.go
  • api/server/v1/json/client/server_service/server_service_client.go
  • api/server/v1/json/client/server_service/update_status_parameters.go
  • api/server/v1/json/client/server_service/update_status_responses.go
  • api/server/v1/json/v1.json
  • api/server/v1/server.pb.validate.go
  • api/server/v1/server.proto
  • api/swagger/swagger-dev.json
  • api/swagger/swagger.json
  • build/ansible/roles/initialization/tasks/main.yml
  • build/ansible/roles/nginx/files/conf.d/pmm.conf
  • documentation/docs/admin/roles/index.md
  • managed/services/grafana/auth_server.go
  • managed/services/server/deps.go
  • managed/services/server/mock_supervisord_service_test.go
  • managed/services/server/server.go
  • managed/services/server/server_test.go
  • managed/services/supervisord/devcontainer_test.go
  • managed/services/supervisord/supervisord.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • percona/pmm-qa (manual)
  • percona/pmm (manual)
💤 Files with no reviewable changes (1)
  • documentation/docs/admin/roles/index.md

Comment thread api/server/v1/json/client/server_service/server_service_client.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants