Skip to content

Commit

Permalink
Merge pull request #491 from yast/bsc1015794
Browse files Browse the repository at this point in the history
Do not retry release notes download after getting a 4xx
  • Loading branch information
imobachgs committed Jan 17, 2017
2 parents cab7125 + 608a12d commit 98a90bf
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 6 deletions.
7 changes: 7 additions & 0 deletions package/yast2-installation.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Tue Jan 17 15:34:22 UTC 2017 - igonzalezsosa@suse.com

- Do not retry to download release notes if a previous attempt
failed (bsc#1015794)
- 3.1.217.11

-------------------------------------------------------------------
Mon Jan 16 12:42:52 UTC 2017 - jreidinger@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-installation.spec
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


Name: yast2-installation
Version: 3.1.217.10
Version: 3.1.217.11
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down
21 changes: 16 additions & 5 deletions src/lib/installation/clients/inst_download_release_notes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ module Yast
class InstDownloadReleaseNotesClient < Client
include Yast::Logger

# When cURL returns one of those codes, the download won't be retried
# @see man curl
# 7 = Failed to connect to host.
# 28 = Operation timeout.
CURL_GIVE_UP_RETURN_CODES = [7, 28].freeze

# Download all release notes mentioned in Product::relnotesurl_all
#
# @return true when successful
Expand Down Expand Up @@ -89,12 +95,18 @@ def download_release_notes
url_base = url[0, pos]
url_template = url_base + filename_templ
log.info("URL template: #{url_base}")
[Language.language, Language.language[0..1], "en"].each do |lang|
[Language.language, Language.language[0..1], "en"].uniq.each do |lang|
url = Builtins.sformat(url_template, lang)
log.info("URL: #{url}")
# Where we want to store the downloaded release notes
filename = Builtins.sformat("%1/relnotes",
SCR.Read(path(".target.tmpdir")))

if InstData.failed_release_notes.include?(url)
log.info("Skipping download of already failed release notes at #{url}")
next
end

# download release notes now
cmd = Builtins.sformat(
"/usr/bin/curl --location --verbose --fail --max-time 300 --connect-timeout 15 %1 '%2' --output '%3' > '%4/%5' 2>&1",
Expand All @@ -111,13 +123,12 @@ def download_release_notes
InstData.release_notes[product["short_name"]] = SCR.Read(path(".target.string"), filename)
InstData.downloaded_release_notes << product["short_name"]
break
# exit codes (see "man curl"):
# 7 = Failed to connect to host.
# 28 = Operation timeout.
elsif ret == 7 || ret == 28
elsif CURL_GIVE_UP_RETURN_CODES.include?(ret)
log.info "Communication with server for release notes download failed, skipping further attempts."
InstData.stop_relnotes_download = true
break
else
InstData.failed_release_notes << url
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions src/modules/InstData.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ def main
# only product names, not the actual RN text
@downloaded_release_notes = []

# list of release notes that YaST failed to download
@failed_release_notes = []

# remember that downloading release notes failed due to communication
# issues with the server, avoid further attempts then
@stop_relnotes_download = false
Expand All @@ -118,6 +121,7 @@ def main
publish variable: :release_notes, type: "map<string,string>"
publish variable: :downloaded_release_notes, type: "list<string>"
publish variable: :stop_relnotes_download, type: "boolean"
publish variable: :failed_release_notes, type: "list<string>"
end

InstData = InstDataClass.new
Expand Down
151 changes: 151 additions & 0 deletions test/lib/inst_download_release_notes_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
#!/usr/bin/env rspec

require_relative "../test_helper"
require "installation/clients/inst_download_release_notes"

Yast.import "InstData"
Yast.import "Pkg"
Yast.import "Language"

describe Yast::InstDownloadReleaseNotesClient do
CURL_NOT_FOUND_CODE = 22
CURL_SUCCESS_CODE = 0
CURL_HARD_ERROR = 7

subject(:client) { described_class.new }

let(:relnotes_url) do
"http://doc.opensuse.org/release-notes/x86_64/openSUSE/Leap42.1/release-notes-openSUSE.rpm"
end

let(:product) do
{
"arch" => "x86_64", "description" => "openSUSE Leap", "category" => "base",
"status" => :selected, "short_name" => "openSUSE",
"relnotes_url" => relnotes_url
}
end

describe "#main" do
let(:proxy) { double("proxy", "Read" => nil, "enabled" => false) }
let(:curl_code) { CURL_SUCCESS_CODE }
let(:language) { "en_US" }

before do
stub_const("Yast::Proxy", proxy)
allow(Yast::Pkg).to receive(:ResolvableProperties).with("", :product, "")
.and_return([product])

allow(Yast::SCR).to receive(:Execute)
.with(Yast::Path.new(".target.bash"), /curl.*relnotes/)
.and_return(curl_code)

allow(Yast::SCR).to receive(:Read)
.with(Yast::Path.new(".target.tmpdir"))
allow(Yast::SCR).to receive(:Read)
.with(Yast::Path.new(".target.string"), /relnotes/)
.and_return("RELNOTES CONTENT")

allow(Yast::Language).to receive(:language).and_return(language)

Yast::InstData.main # reset installation data
end

it "returns :auto" do
expect(client.main).to eq(:auto)
end

context "when release notes are downloaded correctly" do
let(:curl_code) { 0 }

it "saves release notes in InstData" do
client.main
expect(Yast::InstData.release_notes["openSUSE"]).to eq("RELNOTES CONTENT")
end
end

context "when release notes cannot be downloaded due to a hard error" do
let(:curl_code) { CURL_HARD_ERROR }

it "does not retry" do
expect(Yast::SCR).to receive(:Execute)
.with(Yast::Path.new(".target.bash"), /curl.*relnotes/)
.once.and_return(curl_code)
client.main
end

it "does not save release notes" do
allow(Yast::SCR).to receive(:Execute)
.with(Yast::Path.new(".target.bash"), /curl.*relnotes/)
.and_return(curl_code)
client.main
expect(Yast::InstData.release_notes["openSUSE"]).to be_nil
end
end

context "when release notes are not found for the default language" do
let(:language) { "es_ES" }

before do
allow(Yast::SCR).to receive(:Execute)
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.#{language}/)
.and_return(CURL_NOT_FOUND_CODE) # not found
end

it "falls back to the generic language (es_ES -> es)" do
expect(Yast::SCR).to receive(:Execute).once
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.#{language[0..1]}.rtf/)
.and_return(CURL_NOT_FOUND_CODE)
client.main
end

context "and are not found for the generic language" do
before do
allow(Yast::SCR).to receive(:Execute)
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.#{language[0..1]}.rtf/)
.and_return(CURL_NOT_FOUND_CODE) # not found
end

it "falls back to 'en'" do
expect(Yast::SCR).to receive(:Execute).once
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.en.rtf/)
.and_return(CURL_NOT_FOUND_CODE)
client.main
end
end

context "and default language is 'en_*'" do
let(:language) { "en_US" }

# bsc#1015794
it "tries only 1 time with 'en'" do
expect(Yast::SCR).to receive(:Execute).once
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.en.rtf/)
.and_return(CURL_NOT_FOUND_CODE)
client.main
end
end
end

context "when called twice" do
let(:language) { "en" }
let(:curl_code) { 22 }

it "does not try to download again already failed release notes" do
expect(Yast::SCR).to receive(:Execute).once
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.en.rtf/)
.and_return(CURL_NOT_FOUND_CODE)
client.main
client.main # call it a second time
end

it "does not download again already downloaded release notes" do
expect(Yast::SCR).to receive(:Execute).once
.with(Yast::Path.new(".target.bash"), /curl.*RELEASE-NOTES.en.rtf/)
.and_return(CURL_SUCCESS_CODE)
client.main
client.main # call it a second time
end
end
end
end

0 comments on commit 98a90bf

Please sign in to comment.