Merge pull request #12051 from machacekondra/fix_http409_errorhandling

Handle HTTP 409 error messages properly
This commit is contained in:
OpenShift Merge Robot
2021-11-02 16:11:28 +01:00
committed by GitHub
11 changed files with 94 additions and 23 deletions

View File

@ -390,6 +390,11 @@ func (h *APIResponse) IsClientError() bool {
return h.Response.StatusCode/100 == 4
}
// IsConflictError returns true if the response code is 409
func (h *APIResponse) IsConflictError() bool {
return h.Response.StatusCode == 409
}
// IsServerError returns true if the response code is 5xx
func (h *APIResponse) IsServerError() bool {
return h.Response.StatusCode/100 == 5

View File

@ -12,17 +12,22 @@ var (
ErrNotImplemented = errors.New("function not implemented")
)
func handleError(data []byte) error {
e := errorhandling.ErrorModel{}
if err := json.Unmarshal(data, &e); err != nil {
func handleError(data []byte, unmarshalErrorInto interface{}) error {
if err := json.Unmarshal(data, unmarshalErrorInto); err != nil {
return err
}
return e
return unmarshalErrorInto.(error)
}
// Process drains the response body, and processes the HTTP status code
// Note: Closing the response.Body is left to the caller
func (h APIResponse) Process(unmarshalInto interface{}) error {
return h.ProcessWithError(unmarshalInto, &errorhandling.ErrorModel{})
}
// Process drains the response body, and processes the HTTP status code
// Note: Closing the response.Body is left to the caller
func (h APIResponse) ProcessWithError(unmarshalInto interface{}, unmarshalErrorInto interface{}) error {
data, err := ioutil.ReadAll(h.Response.Body)
if err != nil {
return errors.Wrap(err, "unable to process API response")
@ -33,14 +38,22 @@ func (h APIResponse) Process(unmarshalInto interface{}) error {
}
return nil
}
if h.IsConflictError() {
return handleError(data, unmarshalErrorInto)
}
// TODO should we add a debug here with the response code?
return handleError(data)
return handleError(data, &errorhandling.ErrorModel{})
}
func CheckResponseCode(inError error) (int, error) {
e, ok := inError.(errorhandling.ErrorModel)
if !ok {
switch e := inError.(type) {
case *errorhandling.ErrorModel:
return e.Code(), nil
case *errorhandling.PodConflictErrorModel:
return e.Code(), nil
default:
return -1, errors.New("error is not type ErrorModel")
}
return e.Code(), nil
}

View File

@ -9,6 +9,7 @@ import (
"github.com/containers/podman/v3/pkg/api/handlers"
"github.com/containers/podman/v3/pkg/bindings"
"github.com/containers/podman/v3/pkg/domain/entities"
"github.com/containers/podman/v3/pkg/errorhandling"
jsoniter "github.com/json-iterator/go"
)
@ -97,7 +98,7 @@ func Kill(ctx context.Context, nameOrID string, options *KillOptions) (*entities
}
defer response.Body.Close()
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Pause pauses all running containers in a given pod.
@ -117,7 +118,7 @@ func Pause(ctx context.Context, nameOrID string, options *PauseOptions) (*entiti
}
defer response.Body.Close()
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Prune by default removes all non-running pods in local storage.
@ -184,7 +185,7 @@ func Restart(ctx context.Context, nameOrID string, options *RestartOptions) (*en
}
defer response.Body.Close()
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Remove deletes a Pod from from local storage. The optional force parameter denotes
@ -232,7 +233,8 @@ func Start(ctx context.Context, nameOrID string, options *StartOptions) (*entiti
report.Id = nameOrID
return &report, nil
}
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Stop stops all containers in a Pod. The optional timeout parameter can be
@ -260,7 +262,7 @@ func Stop(ctx context.Context, nameOrID string, options *StopOptions) (*entities
report.Id = nameOrID
return &report, nil
}
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Top gathers statistics about the running processes in a pod. The nameOrID can be a pod name
@ -316,7 +318,7 @@ func Unpause(ctx context.Context, nameOrID string, options *UnpauseOptions) (*en
}
defer response.Body.Close()
return &report, response.Process(&report)
return &report, response.ProcessWithError(&report, &errorhandling.PodConflictErrorModel{})
}
// Stats display resource-usage statistics of one or more pods.

View File

@ -225,12 +225,23 @@ func (b *bindingTest) RunTopContainer(containerName *string, podName *string) (s
// This method creates a pod with the given pod name.
// Podname is an optional parameter
func (b *bindingTest) Podcreate(name *string) {
b.PodcreateAndExpose(name, nil)
}
// This method creates a pod with the given pod name and publish port.
// Podname is an optional parameter
// port is an optional parameter
func (b *bindingTest) PodcreateAndExpose(name *string, port *string) {
command := []string{"pod", "create"}
if name != nil {
podname := *name
b.runPodman([]string{"pod", "create", "--name", podname}).Wait(45)
} else {
b.runPodman([]string{"pod", "create"}).Wait(45)
command = append(command, "--name", podname)
}
if port != nil {
podport := *port
command = append(command, "--publish", podport)
}
b.runPodman(command).Wait(45)
}
// StringInSlice returns a boolean based on whether a given

View File

@ -1,6 +1,7 @@
package test_bindings
import (
"fmt"
"net/http"
"strings"
"time"
@ -9,7 +10,9 @@ import (
"github.com/containers/podman/v3/pkg/bindings"
"github.com/containers/podman/v3/pkg/bindings/pods"
"github.com/containers/podman/v3/pkg/domain/entities"
"github.com/containers/podman/v3/pkg/errorhandling"
"github.com/containers/podman/v3/pkg/specgen"
"github.com/containers/podman/v3/utils"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/gexec"
@ -208,6 +211,29 @@ var _ = Describe("Podman pods", func() {
}
})
It("start pod with port conflict", func() {
randomport, err := utils.GetRandomPort()
Expect(err).To(BeNil())
portPublish := fmt.Sprintf("%d:%d", randomport, randomport)
var podwithport string = "newpodwithport"
bt.PodcreateAndExpose(&podwithport, &portPublish)
// Start pod and expose port 12345
_, err = pods.Start(bt.conn, podwithport, nil)
Expect(err).To(BeNil())
// Start another pod and expose same port 12345
var podwithport2 string = "newpodwithport2"
bt.PodcreateAndExpose(&podwithport2, &portPublish)
_, err = pods.Start(bt.conn, podwithport2, nil)
Expect(err).ToNot(BeNil())
code, _ := bindings.CheckResponseCode(err)
Expect(code).To(BeNumerically("==", http.StatusConflict))
Expect(err).To(BeAssignableToTypeOf(&errorhandling.PodConflictErrorModel{}))
})
It("start stop restart pod", func() {
// Start an invalid pod
_, err = pods.Start(bt.conn, "dummyName", nil)

View File

@ -228,7 +228,7 @@ func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []st
for _, name := range namesOrIds {
inspect, err := containers.Inspect(ic.ClientCtx, name, options)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, nil, err
}

View File

@ -188,7 +188,7 @@ func (ir *ImageEngine) Inspect(ctx context.Context, namesOrIDs []string, opts en
for _, i := range namesOrIDs {
r, err := images.GetImage(ir.ClientCtx, i, options)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, nil, err
}

View File

@ -25,7 +25,7 @@ func (ic *ContainerEngine) NetworkInspect(ctx context.Context, namesOrIds []stri
for _, name := range namesOrIds {
report, err := network.Inspect(ic.ClientCtx, name, options)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, nil, err
}

View File

@ -28,7 +28,7 @@ func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string
for _, name := range nameOrIDs {
inspected, err := secrets.Inspect(ic.ClientCtx, name, nil)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, nil, err
}
@ -67,7 +67,7 @@ func (ic *ContainerEngine) SecretRm(ctx context.Context, nameOrIDs []string, opt
for _, name := range nameOrIDs {
secret, err := secrets.Inspect(ic.ClientCtx, name, nil)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, err
}

View File

@ -59,7 +59,7 @@ func (ic *ContainerEngine) VolumeInspect(ctx context.Context, namesOrIds []strin
for _, id := range namesOrIds {
data, err := volumes.Inspect(ic.ClientCtx, id, nil)
if err != nil {
errModel, ok := err.(errorhandling.ErrorModel)
errModel, ok := err.(*errorhandling.ErrorModel)
if !ok {
return nil, nil, err
}

View File

@ -83,6 +83,12 @@ func Contains(err error, sub error) bool {
return strings.Contains(err.Error(), sub.Error())
}
// PodConflictErrorModel is used in remote connections with podman
type PodConflictErrorModel struct {
Errs []string
Id string //nolint
}
// ErrorModel is used in remote connections with podman
type ErrorModel struct {
// API root cause formatted for automated parsing
@ -106,3 +112,11 @@ func (e ErrorModel) Cause() error {
func (e ErrorModel) Code() int {
return e.ResponseCode
}
func (e PodConflictErrorModel) Error() string {
return strings.Join(e.Errs, ",")
}
func (e PodConflictErrorModel) Code() int {
return 409
}