Skip to content

Commit 71e3fa0

Browse files
johnramsdensabaini
andcommitted
feat(rgw): make the member-side RGW apply safe to repeat
EnableRGW in ceph/rgw.go was a one-shot. It wrote radosgw.conf, overwrote server.crt and server.key in place, returned as soon as the service start command succeeded, and RgwServicePlacement.HospitalityCheck refused a gateway that was already running. A caller that repeats a request until the cluster converges needs the opposite, and this is the commit the rest of the series exists to enable: EnableRGW is replaced by applyRGWFrontend, and both "microceph enable rgw" and "microceph certificate set rgw" go through it from here on, not later. applyRGWFrontend renders the wanted radosgw.conf in memory and compares only its "rgw frontends" line against the file on disk: matching line, matching pair on disk and a running service means nothing to do. Comparing only that one line, not the whole file, is deliberate: it is what lets an out-of-band "mon host" refresh made by the existing per-minute updateRadosGWMonHost loop survive an apply untouched instead of the two fighting over the file. The certificate and key move into rgw-tls/<hash of both>/, one directory per pair, because a pair rewritten in place cannot promise the certificate and key change together: a crash between two renames could leave a certificate beside the wrong key, which is exactly what the old flat writeSSLFiles could do. The config only ever names a generation once both files are staged, so switching pairs is one line in the config, and rolling back is pointing that line at the previous, still-intact directory. A journal, radosgw.conf.pending, is written before anything changes: a marker still present at the start of the next apply means an earlier attempt did not finish, so the next apply forces a restart rather than trusting whatever the process happens to be serving, and takes the marker's recorded state, not whatever is half-published on disk, as the rollback target. radosgwConfMu is released before the restart and the readiness wait, which can take minutes, so a slow apply never blocks the unrelated per-minute mon-host refresh, which needs the same lock for one line. In ceph/services_placement_rgw.go, the five-phase pipeline is what finally exercises two things this series built earlier and left unused: PopulateParams validates the request with types.RgwPlacement.Normalized, added inert in "feat(api): describe an RGW placement as an object with explicit TLS intent", and returns ErrRgwFrontendInvalid on a bad one, the sentinel added inert in "fix(services): keep error classes through the service enable path". DbUpdate upserts the rgw_frontends row, added inert in "feat(db): add a table for each member's last applied RGW frontend", in the same transaction as the services row, so presence and frontend can never disagree. HospitalityCheck becomes a deliberate no-op: an already-running gateway must be reconciled, not rejected, which is this branch's whole point. PostPlacementCheck and DbUpdate both call restore if the gateway stops or the database write fails. UpdateRGWCertificates now takes the restart flag itself, and cmdCertificatesRGWPut in api/certificates.go maps its errors with serviceErrorResponse. Three things here are deliberately simple, and each is replaced by name in one of the next three commits: checkRGWReady only checks that the service is active, not that it actually serves, replaced in "feat(rgw): wait until the gateway really serves the requested frontend"; ServiceInit refuses ssl=true without a supplied certificate and key instead of reusing the member's own pair, replaced in "feat(rgw): reuse the member's own pair when TLS is asked for without one"; and DisableRGW only removes the new files and keeps its old mock-runner test rather than converging on a repeat or interrupted call, replaced in "fix(rgw): make RGW disable converge when repeated or interrupted". No cluster pass dispatches RGW yet; a gateway is still placed only by a direct enable or certificate request, until "feat(placement): reconcile RGW across the cluster" wires this pipeline into the reconciler. Co-authored-by: Peter Sabaini <peter.sabaini@canonical.com> Assisted-by: claude-code:claude-fable-5-1 Assisted-by: pi:gpt-6-astra Signed-off-by: John Ramsden <john.ramsden@canonical.com>
1 parent 610f8b4 commit 71e3fa0

11 files changed

Lines changed: 1729 additions & 434 deletions

‎microceph/api/certificates.go‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,14 @@ func cmdCertificatesRGWPut(s mcTypes.State, r *http.Request) mcTypes.Response {
2424
return mcTypes.InternalError(err)
2525
}
2626

27-
err = ceph.UpdateRGWCertificates(interfaces.CephState{State: s}, req.SSLCertificate, req.SSLPrivateKey)
27+
// UpdateRGWCertificates handles the --restart flag itself: the restart is
28+
// part of the same serialized lifecycle, so a failure cannot leave a
29+
// published-but-unservable certificate behind. Input failures (bad base64,
30+
// mismatched pair) carry ceph.ErrRgwFrontendInvalid and map to 400;
31+
// operational failures map to 500 (see serviceErrorResponse).
32+
err = ceph.UpdateRGWCertificates(r.Context(), interfaces.CephState{State: s}, req.SSLCertificate, req.SSLPrivateKey, req.Restart)
2833
if err != nil {
29-
return mcTypes.SmartError(err)
30-
}
31-
32-
if req.Restart {
33-
err = ceph.RestartRGW()
34-
if err != nil {
35-
return mcTypes.SmartError(err)
36-
}
34+
return serviceErrorResponse(err)
3735
}
3836

3937
return mcTypes.EmptySyncResponse

‎microceph/api/rgw_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,62 @@
11
package api
22

33
import (
4+
"bytes"
5+
"encoding/json"
46
"errors"
57
"fmt"
68
"net/http"
79
"net/http/httptest"
810
"testing"
911

12+
lxdAPI "github.com/canonical/lxd/shared/api"
13+
mcTypes "github.com/canonical/microcluster/v3/microcluster/types"
1014
"github.com/stretchr/testify/assert"
1115
"github.com/stretchr/testify/require"
1216

17+
"github.com/canonical/microceph/microceph/api/types"
1318
"github.com/canonical/microceph/microceph/ceph"
1419
)
1520

21+
func TestRGWInvalidFrontendSurvivesHTTPTransport(t *testing.T) {
22+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
23+
response := cmdEnableServicePut(nil, r)
24+
err := response.Render(w, r)
25+
if err != nil {
26+
t.Errorf("rendering RGW response: %v", err)
27+
}
28+
}))
29+
t.Cleanup(server.Close)
30+
31+
cases := []struct {
32+
name string
33+
payload string
34+
}{
35+
{"malformed payload", `{`},
36+
{"null payload", `null`},
37+
{"negative port", `{"Port":-1,"SSL":false}`},
38+
{"TLS port overflow", `{"SSL":true,"SSLPort":65536}`},
39+
{"same listener port", `{"Port":443,"SSL":true,"SSLPort":443}`},
40+
{"missing private key", `{"SSL":true,"SSLCertificate":"Y2VydA=="}`},
41+
{"invalid certificate", `{"SSL":true,"SSLCertificate":"!","SSLPrivateKey":"c2VjcmV0"}`},
42+
}
43+
for _, tc := range cases {
44+
t.Run(tc.name, func(t *testing.T) {
45+
body, err := json.Marshal(types.EnableService{Name: "rgw", Wait: true, Payload: tc.payload})
46+
require.NoError(t, err)
47+
response, err := http.Post(server.URL+"/1.0/services/rgw", "application/json", bytes.NewReader(body))
48+
require.NoError(t, err)
49+
assert.Equal(t, http.StatusBadRequest, response.StatusCode)
50+
51+
// Microcluster reconstructs the error from HTTP status, not error_code.
52+
_, err = mcTypes.ParseResponse(response)
53+
require.Error(t, err)
54+
assert.True(t, lxdAPI.StatusErrorCheck(err, http.StatusBadRequest))
55+
assert.NotContains(t, err.Error(), "c2VjcmV0")
56+
})
57+
}
58+
}
59+
1660
// TestServiceErrorResponseMapping verifies the error classifier shared by the
1761
// service enable, RGW disable and RGW certificate handlers.
1862
func TestServiceErrorResponseMapping(t *testing.T) {

‎microceph/ceph/config.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,12 @@ func UpdateConfig(ctx context.Context, s interfaces.StateInterface) error {
404404
monitorAddresses = formatIPv6(monitorAddresses)
405405

406406
// Keep radosgw.conf's `mon host` line in sync with the current monitors.
407-
// radosgw.conf is written once at RGW enable time (EnableRGW) and is never
408-
// re-rendered from scratch, because the RGW frontend port/SSL settings are
409-
// not persisted. We therefore rewrite only the mon host line in place,
410-
// preserving the other settings. A missing file (RGW disabled) is a no-op;
411-
// a failure here must not block the ceph.conf refresh below.
407+
// radosgw.conf is owned by the RGW enable/update paths (applyRGWFrontend),
408+
// which alone know the TLS material the frontend references, so it is not
409+
// re-rendered from scratch here. We therefore rewrite only the mon host
410+
// line in place, preserving the other settings. A missing file (RGW
411+
// disabled) is a no-op; a failure here must not block the ceph.conf
412+
// refresh below.
412413
err = updateRadosGWMonHost(confPath, monitorAddresses)
413414
if err != nil {
414415
logger.Warnf("failed to refresh radosgw.conf mon host: %v", err)

‎microceph/ceph/configwriter.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ceph
22

33
import (
4+
"bytes"
45
"fmt"
56
"os"
67
"path/filepath"
@@ -32,6 +33,24 @@ func (c *Config) validateConfigFile() error {
3233
return nil
3334
}
3435

36+
// RenderConfig renders the configuration template with data into a byte slice
37+
// without touching disk. A caller compares this against the current on-disk
38+
// file to decide whether a rewrite (and service restart) is actually needed,
39+
// the "render from dqlite, compare in place" idiom (AGENTS.md conf-file
40+
// model). The output is byte-identical to WriteConfig for the same data.
41+
func (c *Config) RenderConfig(data map[string]any) ([]byte, error) {
42+
err := c.validateConfigFile()
43+
if err != nil {
44+
return nil, err
45+
}
46+
var buf bytes.Buffer
47+
err = c.configTemplate.Execute(&buf, data)
48+
if err != nil {
49+
return nil, fmt.Errorf("Couldn't render %s: %w", c.configFile, err)
50+
}
51+
return buf.Bytes(), nil
52+
}
53+
3554
// WriteConfig writes the configuration file given a data bag and a filemode.
3655
// It writes atomically: the template is rendered to a uniquely-named temporary
3756
// file (created via os.CreateTemp in the same directory as the destination) and

0 commit comments

Comments
 (0)