Simplify health check to ignore response

internal/controller/providerconfig/healthcheck/healthcheck_controller.go

@@ -18,7 +18,6 @@ package healthcheck
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"time"
 
@@ -50,7 +49,6 @@ const (
 	errDeleteLCValidationBucket = "failed to delete lifecycle configuration validation bucket"
 	errUpdateHealthStatus       = "failed to update health status of provider config"
 	errFailedHealthCheckReq     = "failed to forward health check request"
-	errUnexpectedResp           = "unexpected response from health check request: %s"
 
 	healthCheckSuffix = "-health-check"
 
@@ -199,23 +197,15 @@ func (c *Controller) doHealthCheck(ctx context.Context, providerConfig *apisv1al
 
 		return doErr
 	}
+	// We don't actually check the response code or body for the health check.
+	// This is because a HTTP Get request to RGW equates to a ListBuckets S3 request and
+	// it is possible that an authorisation error will occur, resulting in a 4XX error.
+	// Instead, we assume healthiness by simply making the connection successfully.
 
-	defer func() {
-		if err := resp.Body.Close(); err != nil {
-			c.log.Info("failed to close response reader after health check", "error", err.Error())
-		}
-	}()
-
-	if resp.StatusCode != http.StatusOK {
-		statusErr := errors.New(fmt.Sprintf(errUnexpectedResp, resp.Status))
-		traces.SetAndRecordError(span, statusErr)
-
-		return statusErr
-	}
 	// Health check completed successfully, update status.
 	providerConfig.Status.SetConditions(v1alpha1.HealthCheckSuccess())
 
-	return nil
+	return resp.Body.Close()
 }
 
 // unpauseBuckets lists all buckets that exist on the given backend by using the custom

internal/controller/providerconfig/healthcheck/healthcheck_controller_test.go

@@ -18,8 +18,8 @@ package healthcheck
 
 import (
 	"context"
-	"fmt"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -59,6 +59,8 @@ func NewTestClient(fn RoundTripFunc) *http.Client {
 func TestReconcile(t *testing.T) {
 	t.Parallel()
 	backendName := "test-backend"
+	someErr := errors.New("some error")
+	urlErr := url.Error{Op: "Get", URL: "http:", Err: someErr}
 
 	type fields struct {
 		fakeS3Client   func(*backendstorefakes.FakeS3Client)
@@ -88,10 +90,7 @@ func TestReconcile(t *testing.T) {
 		"ProviderConfig has been deleted": {
 			fields: fields{
 				testHttpClient: NewTestClient(func(req *http.Request) (*http.Response, error) {
-					return &http.Response{
-						StatusCode: http.StatusOK,
-						Status:     http.StatusText(http.StatusOK),
-					}, nil
+					return &http.Response{}, nil
 				}),
 				fakeS3Client: func(fake *backendstorefakes.FakeS3Client) {
 					// cleanup calls HeadBucket
@@ -163,10 +162,7 @@ func TestReconcile(t *testing.T) {
 		"ProviderConfig goes from healthy to unhealthy with bad request": {
 			fields: fields{
 				testHttpClient: NewTestClient(func(req *http.Request) (*http.Response, error) {
-					return &http.Response{
-						StatusCode: http.StatusBadRequest,
-						Status:     http.StatusText(http.StatusBadRequest),
-					}, nil
+					return &http.Response{}, someErr
 				}),
 				providerConfig: &apisv1alpha1.ProviderConfig{
 					ObjectMeta: metav1.ObjectMeta{
@@ -195,7 +191,7 @@ func TestReconcile(t *testing.T) {
 			},
 			want: want{
 				res: ctrl.Result{},
-				err: errors.New(fmt.Sprintf(errUnexpectedResp, http.StatusText(http.StatusBadRequest))),
+				err: errors.Wrap(&urlErr, errFailedHealthCheckReq),
 				pc: &apisv1alpha1.ProviderConfig{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: backendName,
@@ -208,7 +204,7 @@ func TestReconcile(t *testing.T) {
 							ConditionedStatus: xpv1.ConditionedStatus{
 								Conditions: []xpv1.Condition{
 									v1alpha1.HealthCheckFail().
-										WithMessage(fmt.Sprintf(errUnexpectedResp, http.StatusText(http.StatusBadRequest))),
+										WithMessage(errors.Wrap(&urlErr, errFailedHealthCheckReq).Error()),
 								},
 							},
 						},
@@ -219,10 +215,7 @@ func TestReconcile(t *testing.T) {
 		"ProviderConfig goes from unhealthy to healthy so its buckets should be unpaused": {
 			fields: fields{
 				testHttpClient: NewTestClient(func(req *http.Request) (*http.Response, error) {
-					return &http.Response{
-						StatusCode: http.StatusOK,
-						Status:     http.StatusText(http.StatusOK),
-					}, nil
+					return &http.Response{}, nil
 				}),
 				providerConfig: &apisv1alpha1.ProviderConfig{
 					ObjectMeta: metav1.ObjectMeta{
@@ -353,10 +346,7 @@ func TestReconcile(t *testing.T) {
 		"ProviderConfig goes from health check disabled to healthy so its buckets should be unpaused": {
 			fields: fields{
 				testHttpClient: NewTestClient(func(req *http.Request) (*http.Response, error) {
-					return &http.Response{
-						StatusCode: http.StatusOK,
-						Status:     http.StatusText(http.StatusOK),
-					}, nil
+					return &http.Response{}, nil
 				}),
 				providerConfig: &apisv1alpha1.ProviderConfig{
 					ObjectMeta: metav1.ObjectMeta{