hmcollector: Resolved some scaling/resource issues

CASMHMS-6295

.version

@@ -1 +1 @@
-2.33.0
+2.34.0

CHANGELOG.md

@@ -23,6 +23,14 @@ Removed - for now removed features
 Fixed - for any bug fixes
 Security - in case of vulnerabilities
 -->
+
+## [2.34.0] - 2024-12-06
+
+### Fixed
+
+- Resolved some scaling/resource issues
+- Updated version of Go to 1.23
+
 ## [2.33.0] - 2024-05-28
 
 ### Added

Dockerfile

@@ -1,6 +1,6 @@
 # MIT License
 #
-# (C) Copyright [2019-2022] Hewlett Packard Enterprise Development LP
+# (C) Copyright [2019-2022,2024] Hewlett Packard Enterprise Development LP
 #
 # Permission is hereby granted, free of charge, to any person obtaining a
 # copy of this software and associated documentation files (the "Software"),
@@ -26,7 +26,7 @@
 ARG LIBRDKAFKA_VER_MIN=1.1.0
 
 # Build base just has the packages installed we need.
-FROM artifactory.algol60.net/docker.io/library/golang:1.16-alpine AS build-base
+FROM artifactory.algol60.net/docker.io/library/golang:1.23-alpine AS build-base
 
 ARG LIBRDKAFKA_VER_MIN
 

Dockerfile.testing

@@ -1,6 +1,6 @@
 # MIT License
 #
-# (C) Copyright [2023] Hewlett Packard Enterprise Development LP
+# (C) Copyright [2023-2024] Hewlett Packard Enterprise Development LP
 #
 # Permission is hereby granted, free of charge, to any person obtaining a
 # copy of this software and associated documentation files (the "Software"),
@@ -26,7 +26,7 @@
 ARG LIBRDKAFKA_VER_MIN=1.1.0
 
 # Build base just has the packages installed we need.
-FROM artifactory.algol60.net/docker.io/library/golang:1.16-alpine AS build-base
+FROM artifactory.algol60.net/docker.io/library/golang:1.23-alpine AS build-base
 
 ARG LIBRDKAFKA_VER_MIN
 

cmd/echo_server/echo_server.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright 2020-2022 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2020-2022,2024 Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -26,6 +26,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -72,6 +73,15 @@ func parseRequest(w http.ResponseWriter, r *http.Request) {
 		log.Println(string(prettyJSON.Bytes()) + "\n") // Debug
 	}
 
+	// While it is generally not a requirement to close request bodies in server
+	// handlers, it is good practice.  If a body is only partially read, there can
+	// be a resource leak.  Additionally, if the body is not read at all, the
+	// network connection will be closed and will not be reused even though the
+	// http server will properly drain and close the request body.
+	if r != nil && r.Body != nil {
+		_, _ = io.Copy(io.Discard, r.Body) // ok even if already drained
+		r.Body.Close()
+	}
 }
 
 func main() {

cmd/hmcollector/hmcollector.go

@@ -25,6 +25,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"os/signal"
@@ -206,6 +207,27 @@ func fillMap(m map[string]struct{}, values string) {
 
 }
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// network connection will be closed and will not be reused even though the
+// http server will properly drain and close the request body.
+func DrainAndCloseRequestBody(req *http.Request) {
+       if req != nil && req.Body != nil {
+               _, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+               req.Body.Close()
+       }
+}
+
+// Response bodies should always be drained and closed, else we leak resources
+// and fail to reuse network connections.
+func DrainAndCloseResponseBody(resp *http.Response) {
+       if resp != nil && resp.Body != nil {
+               _, _ = io.Copy(io.Discard, resp.Body) // ok even if already drained
+               resp.Body.Close()
+       }
+}
+
 func SetupLogging() {
 	logLevel := os.Getenv("LOG_LEVEL")
 	logLevel = strings.ToUpper(logLevel)

cmd/hmcollector/http.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2020-2021] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2020-2021,2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -25,10 +25,11 @@ package main
 import (
 	"bytes"
 	"fmt"
-	"go.uber.org/zap"
 	"io/ioutil"
 	"net/http"
+
 	rf "github.com/Cray-HPE/hms-smd/pkg/redfish"
+	"go.uber.org/zap"
 )
 
 func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
@@ -57,12 +58,10 @@ func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
 	var resp *http.Response
 	var doErr error
 
-	rfClientLock.RLock()
+	rfClientLock.RLock()	// TODO: Are locks really necessary here?
 	resp, doErr = rfClient.Do(request)
-	if resp != nil {
-		defer resp.Body.Close()
-	}
 	rfClientLock.RUnlock()
+	defer DrainAndCloseResponseBody(resp)
 	if doErr != nil {
 		endpointLogger.Error("Unable to do request!", 
 			zap.Error(doErr), zap.String("fullURL", fullURL))
@@ -76,11 +75,6 @@ func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
 	if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
 		endpointLogger.Warn("Got Unauthorized response from endpoint, refreshing credentials...")
 
-		//Drain the unused response body.
-		if (resp.Body != nil) {
-			_,_ = ioutil.ReadAll(resp.Body)
-		}
-
 		// Keep track of the previous credentials to see if they change.
 		previousUsername := endpoint.User
 		previousPassword := endpoint.Password

cmd/hmcollector/rest.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2020-2023] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2020-2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -116,8 +116,9 @@ func unmarshalEvents(bodyBytes []byte) (events hmcollector.Events, err error) {
 
 func parseRequest(w http.ResponseWriter, r *http.Request) {
 	bodyBytes, err := ioutil.ReadAll(r.Body)
+	DrainAndCloseRequestBody(r)
 	if err != nil {
-		logger.Error("Unable to ready body!", zap.Error(err))
+		logger.Error("Unable to read body!", zap.Error(err))
 		return
 	}
 
@@ -237,13 +238,16 @@ func parseRequest(w http.ResponseWriter, r *http.Request) {
 // Kubernetes liveness probe - if this responds with anything other than success (code <400) it will cause the
 // pod to be restarted (eventually).
 func doLiveness(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
 
 	w.WriteHeader(http.StatusNoContent)
 }
 
 // Kubernetes liveness probe - if this responds with anything other than success (code <400) multiple times in
 // a row it will cause the pod to be restarted.  Only fail this probe for issues that we expect a restart to fix.
 func doReadiness(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
+
 	ready := true
 
 	// If the Kafka bus isn't good, then return not ready since any incoming data will be dropped.  A restart may not
@@ -281,6 +285,8 @@ type EndpointStatus struct {
 }
 
 func doHealth(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
+
 	// Only allow 'GET' calls.
 	if r.Method != http.MethodGet {
 		w.Header().Set("Allow", "GET")

go.mod

@@ -1,6 +1,6 @@
 module github.com/Cray-HPE/hms-hmcollector
 
-go 1.16
+go 1.23
 
 require (
 	github.com/Cray-HPE/hms-base v1.15.0
@@ -10,10 +10,47 @@ require (
 	github.com/Cray-HPE/hms-smd v1.30.9
 	github.com/Cray-HPE/hms-xname v1.1.0
 	github.com/Shopify/sarama v1.23.1
-	github.com/eapache/go-resiliency v1.2.0 // indirect
 	github.com/namsral/flag v1.7.4-pre
-	github.com/rcrowley/go-metrics v0.0.0-20190706150252-9beb055b7962 // indirect
 	go.uber.org/zap v1.15.0
 	gopkg.in/confluentinc/confluent-kafka-go.v1 v1.1.0
+)
+
+require (
+	github.com/DataDog/zstd v1.3.6-0.20190409195224-796139022798 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/eapache/go-resiliency v1.2.0 // indirect
+	github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect
+	github.com/eapache/queue v1.1.0 // indirect
+	github.com/fsnotify/fsnotify v1.4.9 // indirect
+	github.com/golang/snappy v0.0.1 // indirect
+	github.com/hashicorp/errwrap v1.0.0 // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.1 // indirect
+	github.com/hashicorp/go-multierror v1.1.0 // indirect
+	github.com/hashicorp/go-retryablehttp v0.7.0 // indirect
+	github.com/hashicorp/go-rootcerts v1.0.1 // indirect
+	github.com/hashicorp/go-sockaddr v1.0.2 // indirect
+	github.com/hashicorp/go-uuid v1.0.1 // indirect
+	github.com/hashicorp/hcl v1.0.0 // indirect
+	github.com/hashicorp/vault/api v1.0.4 // indirect
+	github.com/hashicorp/vault/sdk v0.1.13 // indirect
+	github.com/jcmturner/gofork v0.0.0-20190328161633-dc7c13fece03 // indirect
+	github.com/mitchellh/go-homedir v1.1.0 // indirect
+	github.com/mitchellh/mapstructure v1.3.0 // indirect
+	github.com/pierrec/lz4 v2.4.1+incompatible // indirect
+	github.com/rcrowley/go-metrics v0.0.0-20190706150252-9beb055b7962 // indirect
+	github.com/ryanuber/go-glob v1.0.0 // indirect
+	github.com/sirupsen/logrus v1.8.1 // indirect
+	go.uber.org/atomic v1.6.0 // indirect
+	go.uber.org/multierr v1.5.0 // indirect
+	golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899 // indirect
+	golang.org/x/net v0.0.0-20200813134508-3edf25e44fcc // indirect
+	golang.org/x/sys v0.0.0-20200817155316-9781c653f443 // indirect
+	golang.org/x/text v0.3.3 // indirect
+	golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e // indirect
+	gopkg.in/jcmturner/aescts.v1 v1.0.1 // indirect
+	gopkg.in/jcmturner/dnsutils.v1 v1.0.1 // indirect
 	gopkg.in/jcmturner/goidentity.v3 v3.0.0 // indirect
+	gopkg.in/jcmturner/gokrb5.v7 v7.2.3 // indirect
+	gopkg.in/jcmturner/rpc.v1 v1.1.0 // indirect
+	gopkg.in/square/go-jose.v2 v2.3.1 // indirect
 )

internal/hmcollector/smd_api.go

@@ -25,10 +25,12 @@ package hmcollector
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
-	"github.com/Cray-HPE/hms-smd/pkg/redfish"
+
 	"github.com/Cray-HPE/hms-certs/pkg/hms_certs"
+	rf "github.com/Cray-HPE/hms-smd/pkg/redfish"
 )
 
 func GetEndpointList(httpClient *hms_certs.HTTPClientPair, gatewayUrl string) ([]rf.RedfishEPDescription, error) {
@@ -39,6 +41,13 @@ func GetEndpointList(httpClient *hms_certs.HTTPClientPair, gatewayUrl string) ([
 		return nil,fmt.Errorf("Unable to create HTTP request: %v",qerr)
 	}
 	rsp,serr := httpClient.Do(request)
+	defer func () {
+		if rsp != nil && rsp.Body != nil {
+				_, _ = io.Copy(io.Discard, rsp.Body)
+				rsp.Body.Close()
+		}
+	}()
+
 	if (serr != nil) {
 		return nil, fmt.Errorf("Error in HTTP request: %v",serr)
 	}