Skip to content

Commit

Permalink
Return errors on unsuccessful status codes
Browse files Browse the repository at this point in the history
This change updates both the HTTP metadata service and the HTTP registry
to return errors when the response has a status code that is not in the
2xx range. This is a result of bumping bosh-utils, which changes the
behavior of the underlying retry client to not return errors on non 2xx
status codes.

[#156329007](https://www.pivotaltracker.com/story/show/156329007)
  • Loading branch information
jfmyers9 committed Apr 2, 2018
1 parent 717e695 commit b76be3d
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 52 deletions.
7 changes: 1 addition & 6 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions infrastructure/http_metadata_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,13 @@ func (ms HTTPMetadataService) getUserData() (UserDataContentsType, error) {
userDataURL := fmt.Sprintf("%s%s", ms.metadataHost, ms.userdataPath)
userDataResp, err := ms.client.GetCustomized(userDataURL, ms.addHeaders())
if err != nil {
return userData, bosherr.WrapErrorf(err, "Getting user data from url %s", userDataURL)
return userData, bosherr.WrapErrorf(err, "request failed from url %s", userDataURL)
}
defer userDataResp.Body.Close()

defer func() {
if err := userDataResp.Body.Close(); err != nil {
ms.logger.Warn(ms.logTag, "Failed to close response body when getting user data: %s", err.Error())
}
}()
if !isSuccessful(userDataResp) {
return userData, fmt.Errorf("invalid status from url %s: %d", userDataURL, userDataResp.StatusCode)
}

userDataBytes, err := ioutil.ReadAll(userDataResp.Body)
if err != nil {
Expand Down Expand Up @@ -291,3 +290,7 @@ func createRetryClient(delay time.Duration, logger boshlog.Logger) *httpclient.H
httpclient.CreateDefaultClient(nil), 10, delay, logger),
logger)
}

func isSuccessful(resp *http.Response) bool {
return resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices
}
7 changes: 2 additions & 5 deletions infrastructure/http_metadata_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
boshlog "github.com/cloudfoundry/bosh-utils/logger"

"encoding/base64"

. "github.com/cloudfoundry/bosh-agent/infrastructure"
)

Expand Down Expand Up @@ -406,7 +407,6 @@ func describeHTTPMetadataService() {
Expect(err).ToNot(HaveOccurred())
Expect(endpoint).To(Equal("http://fake-registry.com"))
})

})

Context("when server returns an HTTP Response with status code !=2xx (as defined by the request retryable) more than 10 times", func() {
Expand All @@ -416,12 +416,9 @@ func describeHTTPMetadataService() {
metadataService = NewHTTPMetadataServiceWithCustomRetryDelay(ts.URL, metadataHeaders, "/user-data", "/instanceid", "/ssh-keys", dnsResolver, platform, logger, 0*time.Second)

_, err := metadataService.GetRegistryEndpoint()
Expect(err).ToNot(BeNil())
Expect(err.Error()).To(Equal(fmt.Sprintf("Getting user data: Getting user data from url %s/user-data: Performing GET request: Request failed, response: Response{ StatusCode: 500, Status: '500 Internal Server Error' }", ts.URL)))
Expect(err).To(MatchError(fmt.Sprintf("Getting user data: invalid status from url %s/user-data: 500", ts.URL)))
})

})

})

Describe("GetServerName from url encoded user data", func() {
Expand Down
9 changes: 6 additions & 3 deletions infrastructure/http_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,18 @@ func (r httpRegistry) GetSettings() (boshsettings.Settings, error) {
}

settingsURL := fmt.Sprintf("%s/instances/%s/settings", registryEndpoint, identifier)

client := httpclient.NewRetryClient(httpclient.CreateDefaultClient(nil), 10, r.retryDelay, r.logger)

wrapperResponse, err := httpclient.NewHTTPClient(client, r.logger).Get(settingsURL)
if err != nil {
return settings, bosherr.WrapError(err, "Getting settings from url")
}
defer wrapperResponse.Body.Close()

defer func() {
_ = wrapperResponse.Body.Close()
}()
if !isSuccessful(wrapperResponse) {
return settings, fmt.Errorf("invalid status: %d", wrapperResponse.StatusCode)
}

wrapperBytes, err := ioutil.ReadAll(wrapperResponse.Body)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions infrastructure/http_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"time"

. "github.com/cloudfoundry/bosh-agent/infrastructure"
fakeinf "github.com/cloudfoundry/bosh-agent/infrastructure/fakes"
fakeplat "github.com/cloudfoundry/bosh-agent/platform/fakes"
boshsettings "github.com/cloudfoundry/bosh-agent/settings"
boshlog "github.com/cloudfoundry/bosh-utils/logger"
"time"
)

var _ = Describe("httpRegistry", describeHTTPRegistry)
Expand Down Expand Up @@ -394,7 +395,7 @@ func describeHTTPRegistry() {
It("returns settings fetched from http server", func() {
_, err := registry.GetSettings()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("Response{ StatusCode: 500, Status: '500 Internal Server Error' }"))
Expect(err.Error()).To(ContainSubstring("invalid status: 500"))
})
})

Expand Down
21 changes: 3 additions & 18 deletions jobsupervisor/monit/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,7 @@ func (c httpClient) StartService(serviceName string) error {
if err != nil {
return bosherr.WrapError(err, "Sending start request to monit")
}

defer func() {
if err = response.Body.Close(); err != nil {
c.logger.Warn("http-client", "Failed to close monit start POST response body: %s", err.Error())
}
}()
defer response.Body.Close()

err = c.validateResponse(response)
if err != nil {
Expand All @@ -94,12 +89,7 @@ func (c httpClient) StopService(serviceName string) error {
if err != nil {
return bosherr.WrapErrorf(err, "Sending stop request for service '%s'", serviceName)
}

defer func() {
if err = response.Body.Close(); err != nil {
c.logger.Warn("http-client", "Failed to close monit stop POST response body: %s", err.Error())
}
}()
defer response.Body.Close()

err = c.validateResponse(response)
if err != nil {
Expand All @@ -114,12 +104,7 @@ func (c httpClient) UnmonitorService(serviceName string) error {
if err != nil {
return bosherr.WrapError(err, "Sending unmonitor request to monit")
}

defer func() {
if err = response.Body.Close(); err != nil {
c.logger.Warn("http-client", "Failed to close monit unmonitor POST response body: %s", err.Error())
}
}()
defer response.Body.Close()

err = c.validateResponse(response)
if err != nil {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b76be3d

Please sign in to comment.