From: donghee yang Date: Thu, 18 Apr 2013 08:41:35 +0000 (+0900) Subject: [Title] Refactored error handling for building job X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8857be25a04aee721997e0702c1db52ade2065ee;p=sdk%2Ftools%2Fsdk-build.git [Title] Refactored error handling for building job --- diff --git a/src/build_server/BuildJob.rb b/src/build_server/BuildJob.rb index b172c7a..97e9654 100644 --- a/src/build_server/BuildJob.rb +++ b/src/build_server/BuildJob.rb @@ -477,45 +477,35 @@ class BuildJob < CommonJob protected def job_main() @log.info( "Invoking a thread for building Job #{@id}", Log::LV_USER) - if @status == "ERROR" then return end @log.info( "New Job #{@id} is started", Log::LV_USER) # checking build dependency - if not @is_remote_job and not @is_verified and - not check_build_dependency() then - @status = "ERROR" - return false + if not @is_remote_job and not @is_verified then + check_build_dependency() end # clean build - if not build() then - if @is_verified then copy_result_files_to_master() end - - @status = "ERROR" - return false - end + build() lock_event - # upload - if not @is_rev_build_check_job and not @send_result_back and - @parent.nil? and not upload() then + if not @parent.nil? and not @is_rev_build_check_job then + copy_result_files(@parent.source_path) + end - @status = "ERROR" - return false + # upload + if not @is_rev_build_check_job and not @send_result_back and @parent.nil? then + upload() end # copy result files to transport path - if @send_result_back then + if @send_result_back and not @is_rev_build_check_job then copy_result_files_to_master() - elsif not @parent.nil? and not @is_rev_build_check_job then - copy_result_files(@parent.source_path) end # INFO. don't change this string @log.info( "Job is completed!", Log::LV_USER) @status = "FINISHED" - return true end @@ -600,19 +590,17 @@ class BuildJob < CommonJob # unmet dependencies found , report the errors if not unmet_bdeps.empty? or not unmet_ideps.empty? then - @log.error( "Unmet dependency found!", Log::LV_USER) + unmet_pkgs = [] unmet_bdeps.each do |d| os = (d.target_os_list.empty?) ? @os : d.target_os_list[0] - @log.error( " * #{d.package_name}(#{os}) for build-dependency", Log::LV_USER) + unmet_pkgs.push "#{d.package_name}(#{os})" end unmet_ideps.each do |d| os = (d.target_os_list.empty?) ? @os : d.target_os_list[0] - @log.error( " * #{d.package_name}(#{os}) for install-dependency", Log::LV_USER) + unmet_pkgs.push "#{d.package_name}(#{os})" end - - return false - else - return true + unmet_pkgs.uniq! + raise BuildServerException.new("ERR200"), unmet_pkgs.join(",") end end @@ -682,9 +670,8 @@ class BuildJob < CommonJob else builder = Builder.create( "JB#{@id}", @pkgsvr_url, nil, "#{@buildroot_dir}", @server.build_cache_dir ) - if builder.nil? - @log.error( "Creating job builder failed", Log::LV_USER) - return false + if builder.nil? then + raise BuildServerException.new("ERR201") end @log.info( "JobBuilder##{@id} is created", Log::LV_USER) @log.info( " - Package Server : #{@pkgsvr_url}" ) @@ -750,23 +737,16 @@ class BuildJob < CommonJob result = builder.build_job(self, use_clean, local_pkgs, false ) end if not result then - @log.error( "Building job failed", Log::LV_USER) - write_log_url() - return false + raise BuildServerException.new("ERR202") end end # check reverse dependecy if not sub jobs + if not @no_reverse and not @is_rev_build_check_job and not @is_verified and + not ReverseBuildChecker.check( self, true ).empty? then - if not @no_reverse then - if not @is_rev_build_check_job and not @is_verified and - not ReverseBuildChecker.check( self, true ).empty? then - @log.error( "Reverse-build-check failed!" ) - return false - end + raise BuildServerException.new("ERR203") end - - return true end @@ -783,8 +763,7 @@ class BuildJob < CommonJob snapshot = u_client.upload( @pkgsvr_ip, @pkgsvr_port, binpkg_path_list, @server.ftp_addr, @server.ftp_port, @server.ftp_username, @server.ftp_passwd, @pkgsvr_password) if snapshot.nil? then - @log.info( "Upload failed...", Log::LV_USER) - return false + raise BuildServerException.new("ERR204") end # update local @@ -795,8 +774,6 @@ class BuildJob < CommonJob ensure add_timestamp("UPLOAD_END") end - - return true end @@ -811,8 +788,6 @@ class BuildJob < CommonJob @log.info( " * #{file}", Log::LV_USER) FileUtils.ln(file,"#{dst_path}/", :force => true) end - - return true end @@ -823,12 +798,8 @@ class BuildJob < CommonJob FileUtils.mkdir_p outgoing_dir end - # copy result files, if not reverse build - if not @is_rev_build_check_job then - return copy_result_files( outgoing_dir ) - else - return true - end + # copy result files + copy_result_files( outgoing_dir ) end @@ -923,16 +894,6 @@ class BuildJob < CommonJob end - # write web url for log - protected - def write_log_url() - url = get_log_url() - if not url.empty? then - @log.info( " ** Log1: #{url}", Log::LV_USER) - end - end - - # get target os of dependency protected def get_os_of_dependency(dep) diff --git a/src/build_server/BuildServerException.rb b/src/build_server/BuildServerException.rb index e26cc5b..bb71e52 100644 --- a/src/build_server/BuildServerException.rb +++ b/src/build_server/BuildServerException.rb @@ -38,6 +38,13 @@ class BuildServerException < Exception "ERR109" => "Extracting package file failed!", "ERR110" => "Initializing sub jobs failed!", + "ERR200" => "Unmet dependency found!", + "ERR201" => "Creating job builder failed!", + "ERR202" => "Building job failed!", + "ERR203" => "Reverse-build-check failed!", + "ERR204" => "Uploading package file(s) failed!", + "ERR205" => "Job stopped by failure of its sub job!", + "ERR900" => "Cancel event received!", "ERR901" => "Job already upload status. Cancel failed!" } diff --git a/src/build_server/CommonJob.rb b/src/build_server/CommonJob.rb index 283b39f..87fd591 100644 --- a/src/build_server/CommonJob.rb +++ b/src/build_server/CommonJob.rb @@ -154,18 +154,20 @@ class CommonJob @thread = Thread.new do begin job_main() - - # parent job will call sub job's terminate method - if not is_sub_job? then terminate() end rescue BuildServerException => e - @log.error e.message + @log.error( e.err_message, Log::LV_USER ) # "CANCEL" operation(ERR900) must be excluded if @status == "WORKING" and e.err_code != "ERR900" then @status = "ERROR" end rescue => e @log.error e.message @log.error e.backtrace.inspect - if @status == "WORKING" then @status = "ERROR" end + @status = "ERROR" ensure + # parent job will call sub job's terminate method + # if "WORKING"(canceled) and "ERROR", must call "terminate" here + if not is_sub_job? and (@status == "ERROR" or @status == "FINISHED") then + terminate() + end @thread = nil end end diff --git a/src/build_server/JobManager.rb b/src/build_server/JobManager.rb index 7266437..23a7423 100644 --- a/src/build_server/JobManager.rb +++ b/src/build_server/JobManager.rb @@ -185,6 +185,7 @@ class JobManager rescue => e @server.log.error e.message @server.log.error e.backtrace.inspect + job.status = "ERROR" ensure job.thread = nil end @@ -210,11 +211,13 @@ class JobManager # start build set_remote(job, rserver) + + # status change & job control + job.status = "REMOTE_WORKING" + save_job_status(job) + job.add_timestamp("WORK_START") + if job.execute() then - # status change & job control - job.status = "REMOTE_WORKING" - save_job_status(job) - job.add_timestamp("WORK_START") @server.log.info "Moved the job \"#{job.id}\" to remote job list" else @server.log.info "Moving the job \"#{job.id}\" to remote failed" diff --git a/src/build_server/MultiBuildJob.rb b/src/build_server/MultiBuildJob.rb index d475540..468b579 100644 --- a/src/build_server/MultiBuildJob.rb +++ b/src/build_server/MultiBuildJob.rb @@ -364,7 +364,6 @@ class MultiBuildJob < CommonJob protected def job_main() @log.info( "Invoking a thread for MULTI-BUILD Job #{@id}", Log::LV_USER) - if @status == "ERROR" then return end @log.info( "New Job #{@id} is started", Log::LV_USER) # initialize status map @@ -380,9 +379,6 @@ class MultiBuildJob < CommonJob @server.jobmgr.add_internal_job(job) @log.info( "Added new job \"#{job.get_project().name}\" for #{job.os}! (#{job.id})", Log::LV_USER) - if not @server.job_log_url.empty? then - @log.info( " * Log URL : #{@server.job_log_url}/#{job.id}/log", Log::LV_USER) - end end end @@ -405,8 +401,6 @@ class MultiBuildJob < CommonJob # check there is some error or cancel if stop_status == "FINISHED" and (job_status == "ERROR" or job_status == "CANCELED") then - # write url - write_log_url(job) # cancel all other un-finished jobs @sub_jobs.each do |sub| sub_status = sub.status @@ -424,17 +418,18 @@ class MultiBuildJob < CommonJob check_event end - if stop_status == "ERROR" or stop_status == "CANCELED" then - @status = stop_status - return + if stop_status == "ERROR" then + raise BuildServerException.new("ERR205") + elsif stop_status == "CANCELED" then + @event = "CANCEL" if @event == "NONE" + raise BuildServerException.new("ERR205") end lock_event # upload - if not @send_result_back and not upload() then - @status = "ERROR" - return + if not @send_result_back then + upload() end # copy result files to transport path @@ -460,27 +455,13 @@ class MultiBuildJob < CommonJob snapshot = u_client.upload( @pkgsvr_ip, @pkgsvr_port, binpkg_path_list, @server.ftp_addr, @server.ftp_port, @server.ftp_username, @server.ftp_passwd, @pkgsvr_password) if snapshot.nil? then - @log.info( "Upload failed...", Log::LV_USER) - - return false + raise BuildServerException.new("ERR204") end # update local @log.info( "Upload succeeded. Sync local pkg-server again...", Log::LV_USER) @pkgsvr_client.update @log.info("Snapshot: #{snapshot}", Log::LV_USER) - - return true - end - - - # write web url for log - private - def write_log_url(job) - url = job.get_log_url() - if not url.empty? then - @log.info( " ** Log1: #{url}", Log::LV_USER) - end end @@ -492,8 +473,8 @@ class MultiBuildJob < CommonJob FileUtils.mkdir_p outgoing_dir end - # copy result files, if not reverse build - return copy_result_files( outgoing_dir ) + # copy result files + copy_result_files( outgoing_dir ) end @@ -509,8 +490,6 @@ class MultiBuildJob < CommonJob @log.info( " * #{file}", Log::LV_USER) FileUtils.ln(file,"#{dst_path}/", :force => true) end - - return true end end diff --git a/src/build_server/RegisterPackageJob.rb b/src/build_server/RegisterPackageJob.rb index 66e693c..d88d796 100644 --- a/src/build_server/RegisterPackageJob.rb +++ b/src/build_server/RegisterPackageJob.rb @@ -391,14 +391,11 @@ class RegisterPackageJob < CommonJob protected def job_main() @log.info( "Invoking a thread for REGISTER Job #{@id}", Log::LV_USER) - if @status == "ERROR" then return end @log.info( "New Job #{@id} is started", Log::LV_USER) # clean build if not ReverseBuildChecker.check( self, true ).empty? then - @status = "ERROR" - @log.error( "Reverse-build-check failed!" ) - return + raise BuildServerException.new("ERR203") end # if this package has compatible OS, check @@ -425,9 +422,7 @@ class RegisterPackageJob < CommonJob # reverse check if not @no_reverse then if not ReverseBuildChecker.check( self, true, os ) then - @status = "ERROR" - @log.error( "Reverse-build-check failed!" ) - return + raise BuildServerException.new("ERR203") end end end @@ -436,10 +431,7 @@ class RegisterPackageJob < CommonJob lock_event # upload - if not upload() then - @status = "ERROR" - return - end + upload() # INFO. don't change this string @log.info( "Job is completed!", Log::LV_USER) @@ -524,17 +516,13 @@ class RegisterPackageJob < CommonJob snapshot = u_client.upload( @pkgsvr_ip, @pkgsvr_port, binpkg_path_list, @server.ftp_addr, @server.ftp_port, @server.ftp_username, @server.ftp_passwd, @pkgsvr_password) if snapshot.nil? then - @log.info( "Upload failed...", Log::LV_USER) - - return false + raise BuildServerException.new("ERR204") end # update local @log.info( "Upload succeeded. Sync local pkg-server again...", Log::LV_USER) @pkgsvr_client.update @log.info("Snapshot: #{snapshot}", Log::LV_USER) - - return true end diff --git a/src/build_server/ReverseBuildChecker.rb b/src/build_server/ReverseBuildChecker.rb index ad8ddd2..990be4b 100644 --- a/src/build_server/ReverseBuildChecker.rb +++ b/src/build_server/ReverseBuildChecker.rb @@ -141,7 +141,6 @@ class ReverseBuildChecker if not is_project_included?(failure_list, rev_prj, rev_os) then log.info( " * Reverse-build FAIL ... #{rev_prj.name}(#{rev_os}) (#{rev_job.id})", Log::LV_USER) failure_list.push [ rev_prj, rev_os ] - write_log_url(log, rev_job) end # if "exist on error" cancel all other jobs @@ -191,14 +190,4 @@ class ReverseBuildChecker return false end - - - # write web url for log - private - def self.write_log_url(log, job) - url = job.get_log_url() - if not url.empty? then - log.info( " ** Log1: #{url}", Log::LV_USER) - end - end end diff --git a/test/build-server.basic1/build-cli-09.testcase b/test/build-server.basic1/build-cli-09.testcase index b20cb5b..ae12946 100644 --- a/test/build-server.basic1/build-cli-09.testcase +++ b/test/build-server.basic1/build-cli-09.testcase @@ -14,6 +14,6 @@ Info: Invoking a thread for building Job Info: New Job Info: Checking build dependency ... Info: Checking install dependency ... -Error: Unmet dependency found! -Error: * a(ubuntu-32) for build-dependency +Error: Error: Unmet dependency found!: a(ubuntu-32) Error: Job is stopped by ERROR +Error: Building job on remote server failed! diff --git a/test/build-server.basic1/build-cli-18.testcase b/test/build-server.basic1/build-cli-18.testcase index 32218a2..8e567a7 100644 --- a/test/build-server.basic1/build-cli-18.testcase +++ b/test/build-server.basic1/build-cli-18.testcase @@ -25,7 +25,9 @@ Info: Generatiing pkginfo.manifest... Info: Zipping... Info: Creating package file ... a_0.0.2_ubuntu-32.zip Info: Checking reverse build dependency ... -Info: * Will check reverse-build for projects: testb(ubuntu-32) -Info: * Added new job for reverse-build ... testb(ubuntu-32) -Info: * Reverse-build FAIL ... testb(ubuntu-32) +Info: * Will check reverse-build for projects: +Info: * Added new job for reverse-build ... testb +Info: * Reverse-build FAIL ... testb +Error: Error: Reverse-build-check failed! Error: Job is stopped by ERROR +Error: Building job on remote server failed! diff --git a/test/build-server.basic2/build-svr-02.testcase b/test/build-server.basic2/build-svr-02.testcase index 78e1e4e..0f25cb9 100644 --- a/test/build-server.basic2/build-svr-02.testcase +++ b/test/build-server.basic2/build-svr-02.testcase @@ -35,7 +35,7 @@ Subcommand usage: build-svr create -n [-t ] build-svr remove -n build-svr migrate -n [--dsn [--dbuser --dbpassword ] ] -build-svr start -n -p +build-svr start -n [-p ] build-svr stop -n build-svr upgrade -n build-svr add-svr -n -d