Merge pull request #58 from simonjbeaumont/ca-185923

CA-185923: Don't silently fail if archiving SR RRDs fails

Author: simonjbeaumont

rrdd/rrdd_server.ml

@@ -21,36 +21,38 @@ open Pervasiveext
 open Xstringext
 open Threadext
 open Rrdd_shared
+open Rrd_interface
 
 module D = Debug.Make(struct let name="rrdd_server" end)
 open D
 
-let sr_rrds_path _ ~(sr_uuid : string) : string =
-	Filename.concat sr_rrds_location (sr_uuid ^ ".gz")
-
-let archive_sr_rrd _ ~(sr_uuid : string) : unit =
-	if Mutex.try_lock mutex then begin
-		let srrds =
-			try
-				Hashtbl.fold (fun k v acc -> (k,v.rrd)::acc) sr_rrds []
-			with exn ->
-				Mutex.unlock mutex;
-				raise exn
-		in
-		Mutex.unlock mutex;
-		try
-			let rrd = List.assoc sr_uuid srrds in
-			archive_rrd ~uuid:sr_uuid ~rrd ()
-		with Not_found -> ()
-	end
-
-let get_sr_rrd ~sr_uuid =
-	let path = Filename.concat sr_rrds_location sr_uuid in
-	rrd_of_gzip path
-
-let push_sr_rrd _ ~(sr_uuid : string) : unit =
+let archive_sr_rrd _ ~(sr_uuid : string) : string =
+	let sr_rrd = Mutex.execute mutex (fun () ->
+		try (Hashtbl.find sr_rrds sr_uuid)
+		with Not_found ->
+			let msg = Printf.sprintf "No RRD found for SR: %s." sr_uuid in
+			raise (Archive_failed(msg))
+	) in
 	try
-		let rrd = get_sr_rrd ~sr_uuid in
+		archive_rrd ~uuid:sr_uuid ~rrd:sr_rrd.rrd ();
+		let archive_path =
+			Filename.concat Constants.rrd_location (sr_uuid ^ ".gz") in
+		if not (Unixext.file_exists archive_path) then begin
+			let msg = Printf.sprintf "Archive not found: %s." archive_path in
+			raise (Archive_failed(msg))
+		end;
+		archive_path
+	with e ->
+		let msg = Printf.sprintf "Exception raised: %s." (Printexc.to_string e) in
+		raise (Archive_failed(msg))
+
+let push_sr_rrd _ ~(sr_uuid : string) ~(path : string) : unit =
+	try
+		let path =
+			if Filename.check_suffix path ".gz"
+			then Filename.chop_suffix path ".gz"
+			else path in
+		let rrd = rrd_of_gzip path in
 		debug "Pushing RRD for SR uuid=%s locally" sr_uuid;
 		Mutex.execute mutex (fun _ ->
 			Hashtbl.replace sr_rrds sr_uuid {rrd; dss=[]; domid=0}

rrdd/rrdd_shared.ml

@@ -34,8 +34,6 @@ let cache_sr_lock = Mutex.create ()
 let default_ssl_port = 443
 let https_port = ref default_ssl_port
 
-let sr_rrds_location = Constants.rrd_location
-
 (** Pool secret. *)
 let get_pool_secret () =
 	try

rrdd/test_rrdd_monitor.ml

@@ -43,10 +43,10 @@ let check_host_dss expected_dss =
 			(List.sort compare expected_dss)
 			(List.sort compare (dump_dss info.Rrdd_shared.dss))
 
-let update_rrds_test ~dss ~uuid_domids ~paused_vms
+let update_rrds_test ~dss ~uuid_domids ~rebooting_vms ~paused_vms
 	~expected_vm_rrds ~expected_sr_rrds ~expected_host_dss = fun ctxt ->
 	OUnit2.bracket reset_rrdd_shared_state (fun () -> ignore) ctxt;
-	Rrdd_monitor.update_rrds 12345.0 dss uuid_domids paused_vms;
+	Rrdd_monitor.update_rrds 12345.0 dss uuid_domids rebooting_vms paused_vms;
 	check_rrd_hash Rrdd_shared.vm_rrds expected_vm_rrds;
 	check_rrd_hash Rrdd_shared.sr_rrds expected_sr_rrds;
 	check_host_dss expected_host_dss
@@ -56,6 +56,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -64,6 +65,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(Host, ds_a)]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:[]
 		~expected_host_dss:["ds_a"];
@@ -72,6 +74,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(Host, ds_a); (Host, ds_a)]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:[]
 		~expected_host_dss:["ds_a"; "ds_a"];
@@ -80,6 +83,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(VM "a", ds_a)]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -88,6 +92,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(VM "a", ds_a); (VM "b", ds_a)]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -96,6 +101,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(VM "a", ds_a)]
 		~uuid_domids:[("a", 1)]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds: ["a", ["ds_a"]]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -104,6 +110,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(VM "a", ds_a); (VM "b", ds_a); (VM "b", ds_b)]
 		~uuid_domids:[("a", 1); ("b", 1)]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:["a", ["ds_a"]; "b", ["ds_a"; "ds_b"]]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -112,6 +119,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(VM "a", ds_a); (VM "b", ds_a); (VM "c", ds_a)]
 		~uuid_domids:[("a", 1); ("b", 1)]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:["a", ["ds_a"]; "b", ["ds_a"]]
 		~expected_sr_rrds:[]
 		~expected_host_dss:[];
@@ -120,6 +128,7 @@ let update_rrds = "update_rrds" >::: let open Rrd in [
 		~dss:[(SR "a", ds_a); (SR "b", ds_a); (SR "b", ds_b)]
 		~uuid_domids:[]
 		~paused_vms:[]
+		~rebooting_vms:[]
 		~expected_vm_rrds:[]
 		~expected_sr_rrds:["a", ["ds_a"]; "b", ["ds_a"; "ds_b"]]
 		~expected_host_dss:[];