From d9a7fe6c55e36b0e3cfc7685e3c31e2b60edf05c Mon Sep 17 00:00:00 2001 From: Paolo Invernizzi Date: Fri, 18 Oct 2024 17:44:02 +0200 Subject: [PATCH 1/3] Add a configurable timeout for HTTP calls (default 60 mins) --- README.md | 4 ++++ cmd/root.go | 32 ++++++++++++++++++++++++++++++++ cmd/sync.go | 35 ++++++++--------------------------- cmd/updates.go | 14 +++++++------- updates/obs.go | 13 ++++++++----- 5 files changed, 59 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index f8ade1e8..448697f9 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,10 @@ http: # obs: # username: "" # password: "" + +# optional timeout for HTTP requests, in minutes +# the default is 60 minutes, 0 means no timeout +timeout_minutes: 30 ``` diff --git a/cmd/root.go b/cmd/root.go index a208c70f..d04b962a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -6,6 +6,9 @@ import ( "os" "github.com/spf13/cobra" + "github.com/uyuni-project/minima/get" + "github.com/uyuni-project/minima/updates" + yaml "gopkg.in/yaml.v2" ) var ( @@ -14,6 +17,17 @@ var ( cfgString string ) +const defaultTimeoutMinutes = 60 + +// Config maps the configuration in minima.yaml +type Config struct { + Storage get.StorageConfig + SCC get.SCC + OBS updates.OBS + HTTP []get.HTTPRepoConfig + TimeoutMinutes uint `yaml:"timeout_minutes"` +} + // RootCmd represents the base command when called without any subcommands var RootCmd = &cobra.Command{ Use: "minima", @@ -67,3 +81,21 @@ func initConfig() { fmt.Println("Using config file:", cfgFile) } } + +func parseConfig(configString string) (Config, error) { + config := Config{} + if err := yaml.Unmarshal([]byte(configString), &config); err != nil { + return config, fmt.Errorf("configuration parse error: %v", err) + } + + storageType := config.Storage.Type + if storageType != "file" && storageType != "s3" { + return config, fmt.Errorf("configuration parse error: unrecognised storage type") + } + + if config.TimeoutMinutes == 0 { + log.Printf("Applying default timeout of %d minutes to each request\n", defaultTimeoutMinutes) + config.TimeoutMinutes = defaultTimeoutMinutes + } + return config, nil +} diff --git a/cmd/sync.go b/cmd/sync.go index 2ab745c9..95ddfaf2 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -1,17 +1,16 @@ package cmd import ( - "fmt" "log" + "net/http" "net/url" "os" "path/filepath" "strings" + "time" "github.com/spf13/cobra" "github.com/uyuni-project/minima/get" - "github.com/uyuni-project/minima/updates" - yaml "gopkg.in/yaml.v2" ) const sccUrl = "https://scc.suse.com" @@ -54,12 +53,12 @@ var ( Run: func(cmd *cobra.Command, args []string) { initConfig() - var errorflag bool = false syncers, err := syncersFromConfig(cfgString) if err != nil { log.Fatal(err) - errorflag = true } + + var errorflag bool = false for _, syncer := range syncers { log.Printf("Processing repo: %s", syncer.URL.String()) err := syncer.StoreRepo() @@ -80,21 +79,16 @@ var ( skipLegacyPackages bool ) -// Config maps the configuration in minima.yaml -type Config struct { - Storage get.StorageConfig - SCC get.SCC - OBS updates.OBS - HTTP []get.HTTPRepoConfig -} - func syncersFromConfig(configString string) ([]*get.Syncer, error) { config, err := parseConfig(configString) if err != nil { return nil, err } - //---passing the flag value to a global variable in get package, to disables syncing of i586 and i686 rpms (usually inside x86_64) + // passing the flag value to a global variable in get package, to disables syncing of i586 and i686 rpms (usually inside x86_64) get.SkipLegacy = skipLegacyPackages + // Go's default timeout for HTTP clients is 0, meaning there's no timeout + // this can lead to connections hanging indefinitely + http.DefaultClient.Timeout = time.Duration(config.TimeoutMinutes) * time.Minute if config.SCC.Username != "" { if thisRepo != "" { @@ -144,19 +138,6 @@ func syncersFromConfig(configString string) ([]*get.Syncer, error) { return syncers, nil } -func parseConfig(configString string) (Config, error) { - config := Config{} - if err := yaml.Unmarshal([]byte(configString), &config); err != nil { - return config, fmt.Errorf("configuration parse error: %v", err) - } - - storageType := config.Storage.Type - if storageType != "file" && storageType != "s3" { - return config, fmt.Errorf("configuration parse error: unrecognised storage type") - } - return config, nil -} - func init() { RootCmd.AddCommand(syncCmd) // local flags diff --git a/cmd/updates.go b/cmd/updates.go index 709f5c18..374e5eb7 100644 --- a/cmd/updates.go +++ b/cmd/updates.go @@ -79,18 +79,18 @@ func init() { } func muFindAndSync() { - config := Config{} updateList := []Updates{} - err := yaml.Unmarshal([]byte(cfgString), &config) + config, err := parseConfig(cfgString) if err != nil { - log.Fatalf("Error reading configuration: %v", err) + log.Fatal(err) } + timeoutMinutes := time.Duration(config.TimeoutMinutes) * time.Minute if cleanup { // DO CLEANUP - TO BE IMPLEMENTED log.Println("searching for outdated MU repos...") - updateList, err = GetUpdatesAndChannels(config.OBS.Username, config.OBS.Password, true) + updateList, err = GetUpdatesAndChannels(config.OBS.Username, config.OBS.Password, timeoutMinutes, true) if err != nil { log.Fatalf("Error searching for outdated MUs repos: %v", err) } @@ -101,7 +101,7 @@ func muFindAndSync() { log.Println("...done!") } else { if thisMU == "" { - updateList, err = GetUpdatesAndChannels(config.OBS.Username, config.OBS.Password, justSearch) + updateList, err = GetUpdatesAndChannels(config.OBS.Username, config.OBS.Password, timeoutMinutes, justSearch) if err != nil { log.Fatalf("Error finding updates and channels: %v", err) } @@ -322,8 +322,8 @@ func cleanWebChunks(chunks []string) []string { return products } -func GetUpdatesAndChannels(usr, passwd string, justsearch bool) (updlist []Updates, err error) { - client := updates.NewClient(usr, passwd) +func GetUpdatesAndChannels(usr, passwd string, timeout time.Duration, justsearch bool) (updlist []Updates, err error) { + client := updates.NewClient(usr, passwd, timeout) rrs, err := client.GetReleaseRequests("qam-manager", "new,review") if err != nil { return updlist, fmt.Errorf("error while getting response from obs: %v", err) diff --git a/updates/obs.go b/updates/obs.go index d1f5fd5c..355e4706 100644 --- a/updates/obs.go +++ b/updates/obs.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/url" + "time" ) type OBS struct { @@ -79,12 +80,14 @@ func (c *Client) do(req *http.Request, v interface{}) (*http.Response, error) { return resp, err } -func NewClient(username string, password string) *Client { +func NewClient(username string, password string, timeout time.Duration) *Client { return &Client{ - BaseURL: &url.URL{Host: baseUrl, Scheme: "https"}, - Username: username, - Password: password, - HttpClient: &http.Client{}, + BaseURL: &url.URL{Host: baseUrl, Scheme: "https"}, + Username: username, + Password: password, + HttpClient: &http.Client{ + Timeout: timeout, + }, } } From 4e36ac81a0e1e0eced73c50d6cf38600c6527378 Mon Sep 17 00:00:00 2001 From: Paolo Invernizzi Date: Fri, 18 Oct 2024 17:44:41 +0200 Subject: [PATCH 2/3] Add unit test for custom timeout --- cmd/{sync_test.go => root_test.go} | 14 ++++++++++++++ cmd/testdata/custom_timeout.yml | 5 +++++ 2 files changed, 19 insertions(+) rename cmd/{sync_test.go => root_test.go} (89%) create mode 100644 cmd/testdata/custom_timeout.yml diff --git a/cmd/sync_test.go b/cmd/root_test.go similarity index 89% rename from cmd/sync_test.go rename to cmd/root_test.go index 458cff01..2eff7361 100644 --- a/cmd/sync_test.go +++ b/cmd/root_test.go @@ -12,6 +12,7 @@ import ( const ( testdataDir = "testdata" invalidStoragefile = "invalid_storage.yaml" + customTimeoutFile = "custom_timeout.yml" validHTTPReposFile = "valid_http_repos.yaml" validSCCReposFile = "valid_scc_repos.yaml" ) @@ -40,6 +41,7 @@ func TestParseConfig(t *testing.T) { Archs: []string{"x86_64", "aarch64"}, }, }, + TimeoutMinutes: 60, }, false, }, @@ -64,6 +66,18 @@ func TestParseConfig(t *testing.T) { }, }, }, + TimeoutMinutes: 60, + }, + false, + }, + { + "Custom timeout", customTimeoutFile, + Config{ + Storage: get.StorageConfig{ + Type: "file", + Path: "/srv/mirror", + }, + TimeoutMinutes: 10, }, false, }, diff --git a/cmd/testdata/custom_timeout.yml b/cmd/testdata/custom_timeout.yml new file mode 100644 index 00000000..7bd70eb1 --- /dev/null +++ b/cmd/testdata/custom_timeout.yml @@ -0,0 +1,5 @@ +storage: + type: file + path: /srv/mirror + +timeout_minutes: 10 From d46152d406774635a4003eefceb35029ad1cea36 Mon Sep 17 00:00:00 2001 From: Paolo Invernizzi Date: Fri, 18 Oct 2024 17:46:08 +0200 Subject: [PATCH 3/3] Retry HTTP calls on a TCP connection reset --- get/syncer.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/get/syncer.go b/get/syncer.go index b817d281..e3de98e9 100644 --- a/get/syncer.go +++ b/get/syncer.go @@ -9,10 +9,13 @@ import ( "fmt" "io" "log" + "net" "net/url" + "os" "path" "path/filepath" "strings" + "syscall" "github.com/klauspost/compress/zstd" "github.com/uyuni-project/minima/util" @@ -138,6 +141,15 @@ func (r *Syncer) StoreRepo() (err error) { return } + netOpErr, ok := err.(*net.OpError) + if ok { + syscallErr, ok := netOpErr.Err.(*os.SyscallError) + if ok && syscallErr.Err == syscall.ECONNRESET { + log.Printf("Connection reset: %v. Retrying ...\n", netOpErr) + continue + } + } + uerr, unexpectedStatusCode := err.(*UnexpectedStatusCodeError) if unexpectedStatusCode { sc := uerr.StatusCode @@ -205,9 +217,9 @@ func (r *Syncer) storeRepo(checksumMap map[string]XMLChecksum) (err error) { // downloadStoreApply downloads a repo-relative path into a file, while applying a ReaderConsumer func (r *Syncer) downloadStoreApply(relativePath string, checksum string, description string, hash crypto.Hash, f util.ReaderConsumer) error { log.Printf("Downloading %v...", description) - //log.Printf("SYNCER: %v\n", r) url := r.URL url.Path = path.Join(r.URL.Path, relativePath) + body, err := ReadURL(url.String()) if err != nil { return err