diff --git a/weed/admin/dash/volume_management.go b/weed/admin/dash/volume_management.go index b07f7409a..da848e1ea 100644 --- a/weed/admin/dash/volume_management.go +++ b/weed/admin/dash/volume_management.go @@ -9,6 +9,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" ) @@ -399,6 +400,57 @@ func (s *AdminServer) GetVolumeDetails(volumeID uint32, server string) (*VolumeD }, nil } +// SetVolumeReadOnly changes the access mode of a single volume replica. +func (s *AdminServer) SetVolumeReadOnly(ctx context.Context, volumeID uint32, server string, readOnly bool) error { + var address pb.ServerAddress + err := s.masterClient.WithClient(ctx, false, func(client master_pb.SeaweedClient) error { + resp, err := client.VolumeList(ctx, &master_pb.VolumeListRequest{VolumeIds: []uint32{volumeID}}) + if err != nil { + return err + } + address, err = volumeReplicaAddress(resp.GetTopologyInfo(), volumeID, server) + return err + }) + if err != nil { + return err + } + + return s.WithVolumeServerClient(address, func(client volume_server_pb.VolumeServerClient) error { + if readOnly { + _, err := client.VolumeMarkReadonly(ctx, &volume_server_pb.VolumeMarkReadonlyRequest{ + VolumeId: volumeID, + Persist: true, + }) + return err + } + _, err := client.VolumeMarkWritable(ctx, &volume_server_pb.VolumeMarkWritableRequest{VolumeId: volumeID}) + return err + }) +} + +// Resolve the selected replica through the master rather than dialing a +// user-supplied address. Node IDs can differ from their network addresses. +func volumeReplicaAddress(topology *master_pb.TopologyInfo, volumeID uint32, server string) (pb.ServerAddress, error) { + for _, dc := range topology.GetDataCenterInfos() { + for _, rack := range dc.GetRackInfos() { + for _, node := range rack.GetDataNodeInfos() { + if node.GetId() != server { + continue + } + for _, disk := range node.GetDiskInfos() { + for _, volume := range disk.GetVolumeInfos() { + // Older masters may ignore the volume ID filter. + if volume.GetId() == volumeID { + return pb.NewServerAddressFromDataNode(node), nil + } + } + } + } + } + } + return "", fmt.Errorf("volume %d not found on server %s", volumeID, server) +} + // VacuumVolume performs a vacuum operation on a specific volume func (s *AdminServer) VacuumVolume(volumeID int, server string) error { // Validate volumeID range before converting to uint32 diff --git a/weed/admin/dash/volume_management_test.go b/weed/admin/dash/volume_management_test.go new file mode 100644 index 000000000..7381dec71 --- /dev/null +++ b/weed/admin/dash/volume_management_test.go @@ -0,0 +1,159 @@ +package dash + +import ( + "context" + "net" + "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/pb" + "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" + "github.com/seaweedfs/seaweedfs/weed/wdclient" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" +) + +type volumeAccessTestMaster struct { + master_pb.UnimplementedSeaweedServer + topology *master_pb.TopologyInfo +} + +type volumeAccessTestServer struct { + volume_server_pb.UnimplementedVolumeServerServer + rpcError error + calls chan any +} + +func (s *volumeAccessTestMaster) KeepConnected(stream master_pb.Seaweed_KeepConnectedServer) error { + if _, err := stream.Recv(); err != nil { + return err + } + if err := stream.Send(&master_pb.KeepConnectedResponse{}); err != nil { + return err + } + <-stream.Context().Done() + return stream.Context().Err() +} + +func (s *volumeAccessTestMaster) VolumeList(context.Context, *master_pb.VolumeListRequest) (*master_pb.VolumeListResponse, error) { + // Return unfiltered topology to exercise compatibility with older masters. + return &master_pb.VolumeListResponse{TopologyInfo: s.topology}, nil +} + +func (s *volumeAccessTestServer) VolumeMarkReadonly(_ context.Context, req *volume_server_pb.VolumeMarkReadonlyRequest) (*volume_server_pb.VolumeMarkReadonlyResponse, error) { + s.calls <- req + return &volume_server_pb.VolumeMarkReadonlyResponse{}, s.rpcError +} + +func (s *volumeAccessTestServer) VolumeMarkWritable(_ context.Context, req *volume_server_pb.VolumeMarkWritableRequest) (*volume_server_pb.VolumeMarkWritableResponse, error) { + s.calls <- req + return &volume_server_pb.VolumeMarkWritableResponse{}, s.rpcError +} + +func volumeAccessTestTopology(nodes ...*master_pb.DataNodeInfo) *master_pb.TopologyInfo { + return &master_pb.TopologyInfo{DataCenterInfos: []*master_pb.DataCenterInfo{{ + RackInfos: []*master_pb.RackInfo{{DataNodeInfos: nodes}}, + }}} +} + +func TestSetVolumeReadOnly(t *testing.T) { + for _, tc := range []struct { + name string + readOnly bool + volumeID uint32 + server string + rpcError error + wantCall bool + }{ + {name: "persist read-only", readOnly: true, volumeID: 7, server: "node-a", wantCall: true}, + {name: "restore read-write", volumeID: 7, server: "node-a", wantCall: true}, + {name: "propagate volume server error", volumeID: 7, server: "node-a", wantCall: true, rpcError: status.Error(codes.PermissionDenied, "server is in maintenance mode")}, + {name: "reject unknown volume", readOnly: true, volumeID: 8, server: "node-a"}, + {name: "reject unknown server", readOnly: true, volumeID: 7, server: "127.0.0.1:9999"}, + } { + t.Run(tc.name, func(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + port := listener.Addr().(*net.TCPAddr).Port + master := &volumeAccessTestMaster{ + topology: volumeAccessTestTopology(&master_pb.DataNodeInfo{ + Id: "node-a", Address: "127.0.0.1:8080", GrpcPort: uint32(port), + DiskInfos: map[string]*master_pb.DiskInfo{"hdd": { + VolumeInfos: []*master_pb.VolumeInformationMessage{{Id: 7}}, + }}, + }), + } + fake := &volumeAccessTestServer{ + rpcError: tc.rpcError, + calls: make(chan any, 1), + } + grpcServer := grpc.NewServer() + master_pb.RegisterSeaweedServer(grpcServer, master) + volume_server_pb.RegisterVolumeServerServer(grpcServer, fake) + go grpcServer.Serve(listener) + t.Cleanup(grpcServer.Stop) + + dialOption := grpc.WithTransportCredentials(insecure.NewCredentials()) + address := pb.NewServerAddress("127.0.0.1", 9333, port) + discovery := pb.NewServiceDiscoveryFromMap(map[string]pb.ServerAddress{"master": address}) + masterClient := wdclient.NewMasterClient(dialOption, "", "admin", "", "", "", *discovery) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + go masterClient.KeepConnectedToMaster(ctx) + admin := &AdminServer{masterClient: masterClient, grpcDialOption: dialOption} + + err = admin.SetVolumeReadOnly(ctx, tc.volumeID, tc.server, tc.readOnly) + if !tc.wantCall { + require.ErrorContains(t, err, "not found on server") + require.Empty(t, fake.calls, "an unknown replica must not receive a mutation") + return + } + if tc.rpcError != nil { + require.Equal(t, status.Code(tc.rpcError), status.Code(err)) + } else { + require.NoError(t, err) + } + select { + case call := <-fake.calls: + if tc.readOnly { + req, ok := call.(*volume_server_pb.VolumeMarkReadonlyRequest) + require.True(t, ok) + require.Equal(t, tc.volumeID, req.VolumeId) + require.True(t, req.Persist, "operator changes must survive a restart") + require.False(t, req.CanDelete) + } else { + req, ok := call.(*volume_server_pb.VolumeMarkWritableRequest) + require.True(t, ok) + require.Equal(t, tc.volumeID, req.VolumeId) + } + default: + t.Fatal("volume server did not receive an access mode change") + } + }) + } +} + +func TestVolumeReplicaAddress(t *testing.T) { + topology := volumeAccessTestTopology( + &master_pb.DataNodeInfo{Id: "legacy:8080", DiskInfos: map[string]*master_pb.DiskInfo{ + "hdd": {VolumeInfos: []*master_pb.VolumeInformationMessage{{Id: 7}}}, + }}, + &master_pb.DataNodeInfo{Id: "node-b", Address: "host-b:8081", GrpcPort: 18082, DiskInfos: map[string]*master_pb.DiskInfo{ + "ssd": {VolumeInfos: []*master_pb.VolumeInformationMessage{{Id: 7}}, EcShardInfos: []*master_pb.VolumeEcShardInformationMessage{{Id: 8}}}, + }}, + ) + address, err := volumeReplicaAddress(topology, 7, "legacy:8080") + require.NoError(t, err) + require.Equal(t, "legacy:18080", address.ToGrpcAddress()) + address, err = volumeReplicaAddress(topology, 7, "node-b") + require.NoError(t, err) + require.Equal(t, "host-b:18082", address.ToGrpcAddress()) + _, err = volumeReplicaAddress(topology, 8, "node-b") + require.Error(t, err, "EC shards must not be treated as regular volumes") + _, err = volumeReplicaAddress(nil, 7, "node-b") + require.Error(t, err) +} diff --git a/weed/admin/handlers/admin_handlers.go b/weed/admin/handlers/admin_handlers.go index 67131b4b7..007d1e14b 100644 --- a/weed/admin/handlers/admin_handlers.go +++ b/weed/admin/handlers/admin_handlers.go @@ -265,6 +265,7 @@ func (h *AdminHandlers) registerAPIRoutes(api *mux.Router, enforceWrite bool) { volumeApi := api.PathPrefix("/volumes").Subrouter() volumeApi.HandleFunc("/export", h.clusterHandlers.ExportClusterVolumes).Methods(http.MethodGet) volumeApi.Handle("/{id}/{server}/vacuum", wrapWrite(h.clusterHandlers.VacuumVolume)).Methods(http.MethodPost) + volumeApi.Handle("/{id}/{server}/read-only", wrapWrite(h.clusterHandlers.SetVolumeReadOnly)).Methods(http.MethodPost) pluginApi := api.PathPrefix("/plugin").Subrouter() pluginApi.HandleFunc("/status", h.adminServer.GetPluginStatusAPI).Methods(http.MethodGet) diff --git a/weed/admin/handlers/cluster_handlers.go b/weed/admin/handlers/cluster_handlers.go index 5a453a17c..5551d3355 100644 --- a/weed/admin/handlers/cluster_handlers.go +++ b/weed/admin/handlers/cluster_handlers.go @@ -1,8 +1,10 @@ package handlers import ( + "context" "encoding/json" "fmt" + "io" "math" "mime" "net/http" @@ -416,6 +418,50 @@ func (h *ClusterHandlers) GetVolumeServers(w http.ResponseWriter, r *http.Reques writeJSON(w, http.StatusOK, map[string]interface{}{"volume_servers": topology.VolumeServers}) } +// SetVolumeReadOnly handles access mode changes for a single volume replica. +func (h *ClusterHandlers) SetVolumeReadOnly(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + volumeID, err := strconv.ParseUint(vars["id"], 10, 32) + if err != nil { + writeJSONError(w, http.StatusBadRequest, "Invalid volume ID") + return + } + server := vars["server"] + if server == "" { + writeJSONError(w, http.StatusBadRequest, "Server is required") + return + } + var request struct { + ReadOnly *bool `json:"read_only"` + } + decoder := json.NewDecoder(r.Body) + if err := decoder.Decode(&request); err != nil || request.ReadOnly == nil { + writeJSONError(w, http.StatusBadRequest, "read_only must be a boolean") + return + } + if err := decoder.Decode(&struct{}{}); err != io.EOF { + writeJSONError(w, http.StatusBadRequest, "read_only must be a boolean") + return + } + + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + defer cancel() + if err := h.adminServer.SetVolumeReadOnly(ctx, uint32(volumeID), server, *request.ReadOnly); err != nil { + writeJSONError(w, http.StatusInternalServerError, "Failed to change volume access mode: "+err.Error()) + return + } + mode := "read/write" + if *request.ReadOnly { + mode = "read-only" + } + writeJSON(w, http.StatusOK, map[string]interface{}{ + "message": fmt.Sprintf("Volume %d on %s marked %s", volumeID, server, mode), + "volume_id": volumeID, + "server": server, + "read_only": *request.ReadOnly, + }) +} + // VacuumVolume handles volume vacuum requests via API func (h *ClusterHandlers) VacuumVolume(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) diff --git a/weed/admin/handlers/cluster_handlers_test.go b/weed/admin/handlers/cluster_handlers_test.go new file mode 100644 index 000000000..4dfa8de64 --- /dev/null +++ b/weed/admin/handlers/cluster_handlers_test.go @@ -0,0 +1,62 @@ +package handlers + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/admin/dash" + "github.com/stretchr/testify/require" +) + +func TestSetVolumeReadOnlyInvalidRequests(t *testing.T) { + for _, tc := range []struct { + name, id, server, body string + }{ + {"missing volume", "", "node-a", `{"read_only":true}`}, + {"negative volume", "-1", "node-a", `{"read_only":true}`}, + {"overflow volume", "4294967296", "node-a", `{"read_only":true}`}, + {"invalid volume", "abc", "node-a", `{"read_only":true}`}, + {"missing server", "7", "", `{"read_only":true}`}, + {"missing mode", "7", "node-a", `{}`}, + {"null mode", "7", "node-a", `{"read_only":null}`}, + {"string mode", "7", "node-a", `{"read_only":"false"}`}, + {"malformed JSON", "7", "node-a", `{"read_only":`}, + {"trailing JSON", "7", "node-a", `{"read_only":true}{}`}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(tc.body)) + req = mux.SetURLVars(req, map[string]string{"id": tc.id, "server": tc.server}) + w := httptest.NewRecorder() + // Invalid input must be rejected before accessing the cluster. + (&ClusterHandlers{}).SetVolumeReadOnly(w, req) + require.Equal(t, http.StatusBadRequest, w.Code, w.Body.String()) + }) + } +} + +func TestSetVolumeReadOnlyRoutePermissions(t *testing.T) { + for _, tc := range []struct { + name, role string + enforce bool + wantStatus int + }{ + {"read-only user", dash.RoleReadOnly, true, http.StatusForbidden}, + {"admin", "admin", true, http.StatusBadRequest}, + {"no authentication", "", false, http.StatusBadRequest}, + } { + t.Run(tc.name, func(t *testing.T) { + router := mux.NewRouter() + newRouteTestAdminHandlers().registerAPIRoutes(router.PathPrefix("/api").Subrouter(), tc.enforce) + // Deliberately omit the mode: allowed callers reach validation, while + // read-only callers must be rejected by the route's write guard. + req := httptest.NewRequest(http.MethodPost, "/api/volumes/7/node-a/read-only", strings.NewReader(`{}`)) + req = req.WithContext(dash.WithAuthContext(req.Context(), "", tc.role, "")) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + require.Equal(t, tc.wantStatus, w.Code, w.Body.String()) + }) + } +} diff --git a/weed/admin/view/app/volume_details.templ b/weed/admin/view/app/volume_details.templ index b4ae7c7be..7d61b93c3 100644 --- a/weed/admin/view/app/volume_details.templ +++ b/weed/admin/view/app/volume_details.templ @@ -355,14 +355,29 @@ templ VolumeDetails(data dash.VolumeDetailsData) {
-
+
+ if !dash.IsReadOnlyRole(dash.RoleFromContext(ctx)) { + + + }
+ if !dash.IsReadOnlyRole(dash.RoleFromContext(ctx)) { +
Access mode changes apply only to this replica and persist across restarts. Volume size and disk limits still apply.
+ }
@@ -396,8 +411,38 @@ templ VolumeDetails(data dash.VolumeDetailsData) { performVacuum(volumeId, server, this); }); } + document.querySelectorAll('.volume-access-btn').forEach(button => { + button.addEventListener('click', function() { + setVolumeReadOnly(this.dataset.volumeId, this.dataset.server, this.dataset.readOnly === 'true'); + }); + }); }); + async function setVolumeReadOnly(volumeId, server, readOnly) { + const mode = readOnly ? 'read-only' : 'read/write'; + if (!confirm(`Set volume ${volumeId} on ${server} to ${mode}? This applies only to this replica.`)) { + return; + } + const buttons = document.querySelectorAll('.volume-access-btn'); + buttons.forEach(button => { button.disabled = true; }); + try { + const response = await fetch((window.__BASE_PATH__ || '') + `/api/volumes/${volumeId}/${encodeURIComponent(server)}/read-only`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ read_only: readOnly }) + }); + const data = await response.json(); + if (!response.ok || data.error) { + throw new Error(data.error || 'Failed to change volume access mode'); + } + showMessage(data.message, 'success'); + setTimeout(() => window.location.reload(), 1000); + } catch (error) { + showMessage(error.message, 'error'); + buttons.forEach(button => { button.disabled = false; }); + } + } + function performVacuum(volumeId, server, button) { // Disable button and show loading state const originalText = button.innerHTML; @@ -443,10 +488,13 @@ templ VolumeDetails(data dash.VolumeDetailsData) { toast.style.zIndex = '9999'; toast.style.minWidth = '300px'; - toast.innerHTML = ` - ${message} - - `; + toast.textContent = message; + const closeButton = document.createElement('button'); + closeButton.type = 'button'; + closeButton.className = 'btn-close'; + closeButton.setAttribute('data-bs-dismiss', 'alert'); + closeButton.setAttribute('aria-label', 'Close'); + toast.appendChild(closeButton); document.body.appendChild(toast); diff --git a/weed/admin/view/app/volume_details_templ.go b/weed/admin/view/app/volume_details_templ.go index 676cf77eb..c8bf198b4 100644 --- a/weed/admin/view/app/volume_details_templ.go +++ b/weed/admin/view/app/volume_details_templ.go @@ -1,6 +1,6 @@ // Code generated by templ - DO NOT EDIT. -// templ: version: v0.3.1001 +// templ: version: v0.3.1020 package app //lint:file-ignore SA4006 This context is only used if a nested component is present. @@ -328,11 +328,11 @@ func VolumeDetails(data dash.VolumeDetailsData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var22 string - templ_7745c5c3_Var22, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%.1f", float64(data.Volume.Size-data.Volume.DeletedByteCount)/float64(data.Volume.Size)*100)) + templ_7745c5c3_Var22, templ_7745c5c3_Err = templ.ResolveAttributeValue(fmt.Sprintf("%.1f", float64(data.Volume.Size-data.Volume.DeletedByteCount)/float64(data.Volume.Size)*100)) if templ_7745c5c3_Err != nil { return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/volume_details.templ`, Line: 177, Col: 157} } - _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var22)) + _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var22) if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } @@ -451,11 +451,11 @@ func VolumeDetails(data dash.VolumeDetailsData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var27 string - templ_7745c5c3_Var27, templ_7745c5c3_Err = templ.JoinStringErrs(data.Volume.RemoteStorageKey) + templ_7745c5c3_Var27, templ_7745c5c3_Err = templ.ResolveAttributeValue(data.Volume.RemoteStorageKey) if templ_7745c5c3_Err != nil { return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/volume_details.templ`, Line: 261, Col: 138} } - _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var27)) + _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var27) if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } @@ -680,16 +680,16 @@ func VolumeDetails(data dash.VolumeDetailsData) templ.Component { return templ_7745c5c3_Err } } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 63, "
Actions
Use these actions to perform maintenance operations on the volume.
Last updated: ") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 65, "\">Vacuum ") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - var templ_7745c5c3_Var45 string - templ_7745c5c3_Var45, templ_7745c5c3_Err = templ.JoinStringErrs(data.LastUpdated.Format("2006-01-02 15:04:05")) - if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/volume_details.templ`, Line: 382, Col: 77} + if !dash.IsReadOnlyRole(dash.RoleFromContext(ctx)) { + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 66, " ") + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } } - _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var45)) + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 71, "
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 66, "
") + if !dash.IsReadOnlyRole(dash.RoleFromContext(ctx)) { + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 72, "
Access mode changes apply only to this replica and persist across restarts. Volume size and disk limits still apply.
") + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } + } + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 73, "
Use these actions to perform maintenance operations on the volume.
Last updated: ") + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } + var templ_7745c5c3_Var49 string + templ_7745c5c3_Var49, templ_7745c5c3_Err = templ.JoinStringErrs(data.LastUpdated.Format("2006-01-02 15:04:05")) + if templ_7745c5c3_Err != nil { + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/volume_details.templ`, Line: 397, Col: 77} + } + _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var49)) + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 74, "
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/admin/view/app/volume_details_test.go b/weed/admin/view/app/volume_details_test.go new file mode 100644 index 000000000..141074bf7 --- /dev/null +++ b/weed/admin/view/app/volume_details_test.go @@ -0,0 +1,47 @@ +package app + +import ( + "bytes" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/admin/dash" + "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" + "github.com/stretchr/testify/require" + "golang.org/x/net/html" +) + +func TestVolumeDetailsAccessControls(t *testing.T) { + for _, role := range []string{"", "admin", dash.RoleReadOnly} { + for _, readOnly := range []bool{false, true} { + data := dash.VolumeDetailsData{Volume: dash.VolumeWithTopology{ + VolumeInformationMessage: &master_pb.VolumeInformationMessage{Id: 7, ReadOnly: readOnly}, + Server: "node-a", + }} + var rendered bytes.Buffer + ctx := dash.WithAuthContext(t.Context(), "", role, "") + require.NoError(t, VolumeDetails(data).Render(ctx, &rendered)) + doc, err := html.Parse(&rendered) + require.NoError(t, err) + var modes []string + var walk func(*html.Node) + walk = func(node *html.Node) { + if node.Type == html.ElementNode && node.Data == "button" { + for _, attr := range node.Attr { + if attr.Key == "data-read-only" { + modes = append(modes, attr.Val) + } + } + } + for child := node.FirstChild; child != nil; child = child.NextSibling { + walk(child) + } + } + walk(doc) + if role == dash.RoleReadOnly { + require.Empty(t, modes) + } else { + require.Equal(t, []string{"true", "false"}, modes, "both modes must be available even when the displayed state is stale") + } + } + } +} diff --git a/weed/server/volume_grpc_admin.go b/weed/server/volume_grpc_admin.go index a4b6b2241..d12092af6 100644 --- a/weed/server/volume_grpc_admin.go +++ b/weed/server/volume_grpc_admin.go @@ -27,6 +27,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" ) // checkGrpcAdminAuth verifies the gRPC caller is authorized for destructive @@ -282,9 +283,18 @@ func (vs *VolumeServer) makeVolumeReadonly(ctx context.Context, v *storage.Volum // rare case 1.5: it will be unlucky if heartbeat happened between step 1 and 2. // step 2: mark local volume as readonly + var persistErr error if err := vs.store.MarkVolumeReadonly(v.Id, canDelete, persist); err != nil { - glog.Errorf("mark volume %d readonly: %v", v.Id, err) - return err + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + glog.Errorf("mark volume %d readonly: %v", v.Id, err) + return err + } + // Post-rename durability failure: the .vif already holds the new + // mode. Continue with step 3 so the master reflects the change, + // then propagate the durability warning. + glog.Warningf("mark volume %d readonly: %v", v.Id, err) + persistErr = err } else { glog.V(2).Infof("volume %d marked readonly", v.Id) } @@ -294,7 +304,7 @@ func (vs *VolumeServer) makeVolumeReadonly(ctx context.Context, v *storage.Volum return err } - return nil + return persistErr } func (vs *VolumeServer) makeVolumeWritable(ctx context.Context, v *storage.Volume) error { @@ -302,9 +312,18 @@ func (vs *VolumeServer) makeVolumeWritable(ctx context.Context, v *storage.Volum return err } + var persistErr error if err := vs.store.MarkVolumeWritable(v.Id); err != nil { - glog.Errorf("mark volume %d writable: %v", v.Id, err) - return err + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + glog.Errorf("mark volume %d writable: %v", v.Id, err) + return err + } + // Post-rename durability failure: the .vif already holds the new + // mode. Continue notifying the master so traffic is redirected, + // then propagate the durability warning. + glog.Warningf("mark volume %d writable: %v", v.Id, err) + persistErr = err } else { glog.V(2).Infof("volume %d marked writable", v.Id) } @@ -314,7 +333,7 @@ func (vs *VolumeServer) makeVolumeWritable(ctx context.Context, v *storage.Volum return err } - return nil + return persistErr } func isNotLeaderErr(err error) bool { diff --git a/weed/server/volume_grpc_erasure_coding.go b/weed/server/volume_grpc_erasure_coding.go index 64085cfb6..cd520b287 100644 --- a/weed/server/volume_grpc_erasure_coding.go +++ b/weed/server/volume_grpc_erasure_coding.go @@ -2,6 +2,7 @@ package weed_server import ( "context" + "errors" "fmt" "io" "math" @@ -164,7 +165,14 @@ func (vs *VolumeServer) VolumeEcShardsGenerate(ctx context.Context, req *volume_ req.VolumeId, ecCtx.DataShards, ecCtx.ParityShards, ecCtx.Total()) if err := volume_info.SaveVolumeInfo(baseFileName+".vif", volumeInfo); err != nil { - return nil, fmt.Errorf("SaveVolumeInfo %s: %v", baseFileName, err) + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + return nil, fmt.Errorf("SaveVolumeInfo %s: %v", baseFileName, err) + } + // The .vif is committed but may not be crash-durable. The EC + // config is already on disk, so do not clean up the generated + // shard files; a restart will find them and the matching metadata. + glog.Warningf("SaveVolumeInfo %s saved but not crash-durable: %v", baseFileName, err) } shouldCleanup = false diff --git a/weed/server/volume_grpc_tier_download.go b/weed/server/volume_grpc_tier_download.go index afac16b81..32e65979b 100644 --- a/weed/server/volume_grpc_tier_download.go +++ b/weed/server/volume_grpc_tier_download.go @@ -1,6 +1,7 @@ package weed_server import ( + "errors" "fmt" "os" "path/filepath" @@ -12,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage" "github.com/seaweedfs/seaweedfs/weed/storage/backend" "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" ) // VolumeTierMoveDatFromRemote copy dat file from a remote tier to local volume server @@ -94,7 +96,14 @@ func (vs *VolumeServer) VolumeTierMoveDatFromRemote(req *volume_server_pb.Volume // with a .vif referencing the remote object while that object is deleted. v.GetVolumeInfo().Files = v.GetVolumeInfo().Files[1:] if err := v.SaveVolumeInfo(); err != nil { - return fmt.Errorf("volume %d failed to save remote file info: %v", v.Id, err) + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + return fmt.Errorf("volume %d failed to save remote file info: %v", v.Id, err) + } + // The .vif is committed but may not be crash-durable. Continue + // with the backend switch and remote deletion since the metadata + // already reflects the local-only state. + glog.Warningf("volume %d saved remote file info but not crash-durable: %v", v.Id, err) } // fsync the directory again so the rewritten .vif is durable. diff --git a/weed/storage/needle_map_file_pool_test.go b/weed/storage/needle_map_file_pool_test.go index 9f1f15e35..c313ad467 100644 --- a/weed/storage/needle_map_file_pool_test.go +++ b/weed/storage/needle_map_file_pool_test.go @@ -77,7 +77,9 @@ func TestSortedFileNeedleMap_HoldsNoDescriptors(t *testing.T) { t.Fatalf("write needle %d: %v", i, err) } } - v.PersistReadOnly(true, true) + if err := v.PersistReadOnly(true, true); err != nil { + t.Fatalf("persist read-only: %v", err) + } v.Close() v, err = NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) diff --git a/weed/storage/store.go b/weed/storage/store.go index 19632d0c3..3b7818e11 100644 --- a/weed/storage/store.go +++ b/weed/storage/store.go @@ -838,6 +838,8 @@ func (s *Store) MarkVolumeReadonly(i needle.VolumeId, canDelete bool, persist bo } } v.noWriteLock.Lock() + prevNoWriteOrDelete := v.noWriteOrDelete + prevNoWriteCanDelete := v.noWriteCanDelete v.noWriteOrDelete = !canDelete if canDelete { v.noWriteCanDelete = true @@ -846,7 +848,18 @@ func (s *Store) MarkVolumeReadonly(i needle.VolumeId, canDelete bool, persist bo v.noWriteCanDelete = false } if persist { - v.PersistReadOnly(true, canDelete) + if err := v.PersistReadOnly(true, canDelete); err != nil { + // A pre-commit failure leaves the old .vif intact, so roll + // back the in-memory flags. A NotCrashDurableError means the + // rename already committed; keep flags aligned with the file. + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + v.noWriteOrDelete = prevNoWriteOrDelete + v.noWriteCanDelete = prevNoWriteCanDelete + } + v.noWriteLock.Unlock() + return fmt.Errorf("volume %d persist read-only: %w", i, err) + } } v.noWriteLock.Unlock() return nil @@ -865,18 +878,36 @@ func (s *Store) MarkVolumeWritable(i needle.VolumeId) error { return fmt.Errorf("volume %d reopen idx for write: %v", i, err) } v.noWriteLock.Lock() + prevNoWriteOrDelete := v.noWriteOrDelete + prevNoWriteCanDelete := v.noWriteCanDelete v.noWriteOrDelete = false // Remote-tiered volumes must stay noWriteCanDelete regardless of marks. if !v.HasRemoteFile() { v.noWriteCanDelete = false } - v.PersistReadOnly(false, false) + persistErr := v.PersistReadOnly(false, false) + if persistErr != nil { + var ndErr *volume_info.NotCrashDurableError + if !errors.As(persistErr, &ndErr) { + // Pre-commit failure: the old .vif is intact, so roll back + // the in-memory flags and return early. + v.noWriteOrDelete = prevNoWriteOrDelete + v.noWriteCanDelete = prevNoWriteCanDelete + v.noWriteLock.Unlock() + return fmt.Errorf("volume %d persist writable: %w", i, persistErr) + } + // Post-rename durability failure: the file already holds the new + // mode. Wrap the error but continue with post-commit work so the + // volume is usable in memory even though the rename may not + // survive a crash. + persistErr = fmt.Errorf("volume %d persist writable: %w", i, persistErr) + } v.noWriteLock.Unlock() // Clear the EIO streak and the sticky quarantine flag so the next // CollectHeartbeat can announce the volume again. If the disk is // still bad, the next failed op will re-arm the streak. v.resetIoErrorState() - return nil + return persistErr } func (s *Store) MountVolume(i needle.VolumeId) error { diff --git a/weed/storage/volume.go b/weed/storage/volume.go index 5a3baf823..404d3ba43 100644 --- a/weed/storage/volume.go +++ b/weed/storage/volume.go @@ -1,6 +1,7 @@ package storage import ( + "errors" "fmt" "os" "path" @@ -16,6 +17,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" "github.com/seaweedfs/seaweedfs/weed/glog" ) @@ -575,10 +577,25 @@ func (v *Volume) ReadOnlyReasons() (readOnly, noWriteOrDelete, noWriteCanDelete, return noWriteOrDelete || noWriteCanDelete || diskSpaceLow, noWriteOrDelete, noWriteCanDelete, diskSpaceLow } -func (v *Volume) PersistReadOnly(readOnly bool, canDelete bool) { +func (v *Volume) PersistReadOnly(readOnly bool, canDelete bool) error { v.volumeInfoRWLock.Lock() defer v.volumeInfoRWLock.Unlock() + prevReadOnly := v.volumeInfo.ReadOnly + prevReadOnlyCanDelete := v.volumeInfo.ReadOnlyCanDelete v.volumeInfo.ReadOnly = readOnly v.volumeInfo.ReadOnlyCanDelete = readOnly && canDelete - v.SaveVolumeInfo() + if err := v.SaveVolumeInfo(); err != nil { + // A pre-commit failure (write/sync/close/rename) leaves the old + // .vif intact, so roll back in-memory state to match it. A + // NotCrashDurableError means the rename already committed the + // new mode to disk; rolling back would split in-memory state + // from the durable file, so keep the new state and propagate. + var ndErr *volume_info.NotCrashDurableError + if !errors.As(err, &ndErr) { + v.volumeInfo.ReadOnly = prevReadOnly + v.volumeInfo.ReadOnlyCanDelete = prevReadOnlyCanDelete + } + return fmt.Errorf("persist volume read-only state: %w", err) + } + return nil } diff --git a/weed/storage/volume_info/volume_info.go b/weed/storage/volume_info/volume_info.go index 78c931af6..b10be555d 100644 --- a/weed/storage/volume_info/volume_info.go +++ b/weed/storage/volume_info/volume_info.go @@ -3,6 +3,7 @@ package volume_info import ( "fmt" "os" + "path/filepath" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" @@ -68,6 +69,22 @@ func MaybeLoadVolumeInfo(fileName string) (volumeInfo *volume_server_pb.VolumeIn return } +// NotCrashDurableError indicates that the .vif file was renamed +// successfully but the directory entry may not survive a crash. The +// on-disk file already holds the new metadata, so callers should keep +// in-memory state aligned with the file rather than rolling back, while +// still propagating the durability failure to the user. +type NotCrashDurableError struct { + FileName string + Err error +} + +func (e *NotCrashDurableError) Error() string { + return fmt.Sprintf("volume info %s saved but not crash-durable: %v", e.FileName, e.Err) +} + +func (e *NotCrashDurableError) Unwrap() error { return e.Err } + func SaveVolumeInfo(fileName string, volumeInfo *volume_server_pb.VolumeInfo) error { if exists, _, canWrite, _, _ := util.CheckFile(fileName); exists && !canWrite { @@ -85,8 +102,49 @@ func SaveVolumeInfo(fileName string, volumeInfo *volume_server_pb.VolumeInfo) er return fmt.Errorf("failed to marshal %s: %v", fileName, marshalErr) } - if err := util.WriteFile(fileName, text, 0644); err != nil { - return fmt.Errorf("failed to write %s: %v", fileName, err) + // Write atomically so a write/sync/close failure leaves the existing + // .vif file intact. PersistReadOnly rolls back in-memory state on + // error; the atomic rename guarantees the durable file still matches + // that rolled-back state rather than the requested mode. Use a + // unique temp file so concurrent saves for the same volume do not + // collide on a shared .tmp path. + f, err := os.CreateTemp(filepath.Dir(fileName), filepath.Base(fileName)+".tmp.*") + if err != nil { + return fmt.Errorf("failed to create temp file for %s: %w", fileName, err) + } + tmpName := f.Name() + if _, err := f.Write(text); err != nil { + f.Close() + os.Remove(tmpName) + return fmt.Errorf("failed to write %s: %w", fileName, err) + } + if err := f.Chmod(0644); err != nil { + f.Close() + os.Remove(tmpName) + return fmt.Errorf("failed to chmod %s: %w", fileName, err) + } + if err := f.Sync(); err != nil { + f.Close() + os.Remove(tmpName) + return fmt.Errorf("failed to sync %s: %w", fileName, err) + } + if err := f.Close(); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to close %s: %w", fileName, err) + } + if err := os.Rename(tmpName, fileName); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to rename %s: %w", fileName, err) + } + // The rename has committed the new metadata to the on-disk file. + // A directory fsync failure only risks losing the rename across a + // crash; the file content is already correct, so callers must not + // roll back in-memory state. Return NotCrashDurableError so they + // can distinguish this from a pre-commit failure and keep state + // aligned with the renamed file while still reporting the issue. + if err := util.FsyncDir(filepath.Dir(fileName)); err != nil { + glog.Warningf("fsync dir for %s: %v", fileName, err) + return &NotCrashDurableError{FileName: fileName, Err: err} } return nil diff --git a/weed/storage/volume_loading_corrupt_idx_test.go b/weed/storage/volume_loading_corrupt_idx_test.go index aeba1269c..f872ec622 100644 --- a/weed/storage/volume_loading_corrupt_idx_test.go +++ b/weed/storage/volume_loading_corrupt_idx_test.go @@ -20,7 +20,9 @@ func TestLoad_CorruptIdx_NoSegfault(t *testing.T) { if _, _, _, err := v.writeNeedle2(newRandomNeedle(1), true, false, false); err != nil { t.Fatalf("seed write: %v", err) } - v.PersistReadOnly(true, false) // reload goes through SortedFileNeedleMap + if err := v.PersistReadOnly(true, false); err != nil { // reload goes through SortedFileNeedleMap + t.Fatalf("persist read-only: %v", err) + } v.Close() // Truncate .idx to a non-aligned size so the walk rejects it. diff --git a/weed/storage/volume_mark_writable_test.go b/weed/storage/volume_mark_writable_test.go index 33e1097ec..2b120e1a1 100644 --- a/weed/storage/volume_mark_writable_test.go +++ b/weed/storage/volume_mark_writable_test.go @@ -27,7 +27,9 @@ func TestMarkVolumeWritable_ReopensPersistedReadOnly(t *testing.T) { // Persist read-only state into .vif, then simulate a server restart by // closing and re-opening the volume from the same directory. - v.PersistReadOnly(true, false) + if err := v.PersistReadOnly(true, false); err != nil { + t.Fatalf("persist read-only: %v", err) + } v.Close() v2, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) diff --git a/weed/storage/volume_write_test.go b/weed/storage/volume_write_test.go index e479367ee..9073de325 100644 --- a/weed/storage/volume_write_test.go +++ b/weed/storage/volume_write_test.go @@ -187,7 +187,9 @@ func TestWriteNeedleBlobRejectedOnReadOnlyVolume(t *testing.T) { if err != nil { t.Fatalf("read needle blob: %v", err) } - v.PersistReadOnly(true, false) + if err := v.PersistReadOnly(true, false); err != nil { + t.Fatalf("persist read-only: %v", err) + } v.Close() v, err = NewVolume(dir, dir, "", 7, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)