fix(services): keep error classes through the service enable path

[johnramsden]

Description

EnableService in ceph/services_placement.go runs five phases for every service: PopulateParams, HospitalityCheck, ServiceInit, PostPlacementCheck and DbUpdate. It wrapped each phase's error with fmt.Errorf("...: %v", err), which keeps the text but drops the error value, so errors.Is could never classify the failure. The handler cmdEnableServicePut in api/services.go then returned mcTypes.SyncResponse(false, err), which answers HTTP 200 with an error body regardless of what actually failed.

That is a real bug affecting every service enabled through this path today, not just RGW: mon, mgr, mds, nfs, rgw, rbd-mirror and cephfs-mirror all silently returned 200 on a failed enable. The phase errors are now wrapped with %w so the error value survives, and cmdEnableServicePut and cmdRGWServiceDelete answer through the new serviceErrorResponse in api/services.go instead of the 200-returning SyncResponse. A body that cannot be decoded is now a 400 instead of a 500.

The rest of the change is scaffolding for the RGW placement work later in this series: ceph/placement.go gains two sentinel errors, ErrRgwFrontendInvalid for a request the caller must fix and ErrPlacementOperationFailed for a failure on the member, and serviceErrorResponse maps them to 400 and 500. Nothing returns either sentinel yet; they land now so the classification mechanism can be reviewed on its own, separately from the RGW code that will first use it.

Two smaller changes: PopulateParams now runs before updateConfigFunc renders ceph.conf, so a request that cannot be parsed changes nothing on disk. ServicePlacementHandler no longer logs payload.Payload at debug level, which for RGW carries the TLS private key.

Stack: PR 2 of 13. Merge in order. Each PR is one logical change on top of the one before it.

1. feat(db): add a table for each member's last applied RGW frontend #881 feat(db): add a table for each member's last applied RGW frontend
2. fix(services): keep error classes through the service enable path #882 fix(services): keep error classes through the service enable path (this PR)
3. feat(api): describe an RGW placement as an object with explicit TLS intent #883 feat(api): describe an RGW placement as an object with explicit TLS intent
4. feat(rgw): make the member-side RGW apply safe to repeat #884 feat(rgw): make the member-side RGW apply safe to repeat
5. feat(rgw): wait until the gateway really serves the requested frontend #885 feat(rgw): wait until the gateway really serves the requested frontend
6. feat(rgw): reuse the member's own pair when TLS is asked for without one #886 feat(rgw): reuse the member's own pair when TLS is asked for without one
7. fix(rgw): make RGW disable converge when repeated or interrupted #887 fix(rgw): make RGW disable converge when repeated or interrupted
8. feat(placement): accept the RGW object in the policy and keep its secrets out #888 feat(placement): accept the RGW object in the policy and keep its secrets out
9. feat(placement): reconcile RGW across the cluster #889 feat(placement): reconcile RGW across the cluster
10. test(robot): add the single-VM RGW placement suite #890 test(robot): add the single-VM RGW placement suite
11. test(robot): add the three-VM RGW placement suite #891 test(robot): add the three-VM RGW placement suite
12. docs: keep the RGW placement contract in HACKING.md for now #892 docs: keep the RGW placement contract in HACKING.md for now
13. fix(rgw): do not trust a pending-apply marker the operator completed by hand #893 fix(rgw): do not trust a pending-apply marker the operator completed by hand

Type of change

• Bug fix (non-breaking change which fixes an issue)

How has this been tested?

• Unit tests pass at this stack's cumulative tip

Contributor checklist

Please check that you have:

• self-reviewed the code in this PR
• added code comments, particularly in less straightforward areas
• checked and added or updated relevant documentation
• checked and added or updated relevant release notes
• added tests to verify effectiveness of this change

[sabaini]

hm, the added test does not actually call cmdEnableServicePut() though, right?
possibly expand test coverage to handle this?

[johnramsden]

See 11f7c1a

[sabaini]

lgtm!