Skip to content

Commit

Permalink
Report StatusConflict on Pod opt partial failures
Browse files Browse the repository at this point in the history 
- When one or more containers in the Pod reports an error on an operation
report StatusConflict and report the error(s)

- jsoniter type encoding used to marshal error as string using error.Error()

- Update test framework to allow setting any flag when creating pods

Fixes containers#8865

Signed-off-by: Jhon Honce <jhonce@redhat.com>
  • Loading branch information
@jwhonce
jwhonce committed Feb 1, 2021
1 parent 4ee66c2 commit 3744eea
Show file tree
Hide file tree
Showing 22 changed files with 313 additions and 187 deletions.
90 changes: 50 additions & 40 deletions pkg/api/handlers/libpod/pods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,20 @@ func PodStop(w http.ResponseWriter, r *http.Request) {
logrus.Errorf("Error cleaning up pod %s container %s: %v", pod.ID(), id, err)
}
}
var errs []error //nolint

report := entities.PodStopReport{Id: pod.ID()}
for id, err := range responses {
errs = append(errs, errors.Wrapf(err, "error stopping container %s", id))
report.Errs = append(report.Errs, errors.Wrapf(err, "error stopping container %s", id))
}
report := entities.PodStopReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, report)
utils.WriteResponse(w, code, report)
}

func PodStart(w http.ResponseWriter, r *http.Request) {
var errs []error //nolint
runtime := r.Context().Value("runtime").(*libpod.Runtime)
name := utils.GetName(r)
pod, err := runtime.LookupPod(name)
Expand All @@ -168,19 +169,23 @@ func PodStart(w http.ResponseWriter, r *http.Request) {
utils.WriteResponse(w, http.StatusNotModified, "")
return
}

responses, err := pod.Start(r.Context())
if err != nil && errors.Cause(err) != define.ErrPodPartialFail {
utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
utils.Error(w, "Something went wrong", http.StatusConflict, err)
return
}

report := entities.PodStartReport{Id: name}
for id, err := range responses {
errs = append(errs, errors.Wrapf(err, "error starting container %s", id))
report.Errs = append(report.Errs, errors.Wrapf(err, "error starting container "+id))
}
report := entities.PodStartReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, report)
utils.WriteResponse(w, code, report)
}

func PodDelete(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -209,14 +214,11 @@ func PodDelete(w http.ResponseWriter, r *http.Request) {
utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
return
}
report := entities.PodRmReport{
Id: pod.ID(),
}
report := entities.PodRmReport{Id: pod.ID()}
utils.WriteResponse(w, http.StatusOK, report)
}

func PodRestart(w http.ResponseWriter, r *http.Request) {
var errs []error //nolint
runtime := r.Context().Value("runtime").(*libpod.Runtime)
name := utils.GetName(r)
pod, err := runtime.LookupPod(name)
Expand All @@ -229,14 +231,17 @@ func PodRestart(w http.ResponseWriter, r *http.Request) {
utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
return
}

report := entities.PodRestartReport{Id: pod.ID()}
for id, err := range responses {
errs = append(errs, errors.Wrapf(err, "error restarting container %s", id))
report.Errs = append(report.Errs, errors.Wrapf(err, "error restarting container %s", id))
}
report := entities.PodRestartReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, report)
utils.WriteResponse(w, code, report)
}

func PodPrune(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -267,7 +272,6 @@ func PodPruneHelper(r *http.Request) ([]*entities.PodPruneReport, error) {
}

func PodPause(w http.ResponseWriter, r *http.Request) {
var errs []error //nolint
runtime := r.Context().Value("runtime").(*libpod.Runtime)
name := utils.GetName(r)
pod, err := runtime.LookupPod(name)
Expand All @@ -280,18 +284,20 @@ func PodPause(w http.ResponseWriter, r *http.Request) {
utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
return
}

report := entities.PodPauseReport{Id: pod.ID()}
for id, v := range responses {
errs = append(errs, errors.Wrapf(v, "error pausing container %s", id))
report.Errs = append(report.Errs, errors.Wrapf(v, "error pausing container %s", id))
}
report := entities.PodPauseReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, report)
utils.WriteResponse(w, code, report)
}

func PodUnpause(w http.ResponseWriter, r *http.Request) {
var errs []error //nolint
runtime := r.Context().Value("runtime").(*libpod.Runtime)
name := utils.GetName(r)
pod, err := runtime.LookupPod(name)
Expand All @@ -304,14 +310,17 @@ func PodUnpause(w http.ResponseWriter, r *http.Request) {
utils.Error(w, "failed to pause pod", http.StatusInternalServerError, err)
return
}

report := entities.PodUnpauseReport{Id: pod.ID()}
for id, v := range responses {
errs = append(errs, errors.Wrapf(v, "error unpausing container %s", id))
report.Errs = append(report.Errs, errors.Wrapf(v, "error unpausing container %s", id))
}
report := entities.PodUnpauseReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, &report)
utils.WriteResponse(w, code, &report)
}

func PodTop(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -361,7 +370,6 @@ func PodKill(w http.ResponseWriter, r *http.Request) {
runtime = r.Context().Value("runtime").(*libpod.Runtime)
decoder = r.Context().Value("decoder").(*schema.Decoder)
signal = "SIGKILL"
errs []error //nolint
)
query := struct {
Signal string `schema:"signal"`
Expand Down Expand Up @@ -413,16 +421,18 @@ func PodKill(w http.ResponseWriter, r *http.Request) {
return
}

report := &entities.PodKillReport{Id: pod.ID()}
for _, v := range responses {
if v != nil {
errs = append(errs, v)
report.Errs = append(report.Errs, v)
}
}
report := &entities.PodKillReport{
Errs: errs,
Id: pod.ID(),

code := http.StatusOK
if len(report.Errs) > 0 {
code = http.StatusConflict
}
utils.WriteResponse(w, http.StatusOK, report)
utils.WriteResponse(w, code, report)
}

func PodExists(w http.ResponseWriter, r *http.Request) {
Expand Down
46 changes: 45 additions & 1 deletion pkg/api/handlers/utils/handler.go
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package utils

import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/url"
"os"
"strings"
"unsafe"

"github.com/blang/semver"
"github.com/gorilla/mux"
jsoniter "github.com/json-iterator/go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -144,6 +145,49 @@ func WriteResponse(w http.ResponseWriter, code int, value interface{}) {
}
}

func init() {
jsoniter.RegisterTypeEncoderFunc("error", MarshalErrorJSON, MarshalErrorJSONIsEmpty)
jsoniter.RegisterTypeEncoderFunc("[]error", MarshalErrorSliceJSON, MarshalErrorSliceJSONIsEmpty)
}

var json = jsoniter.ConfigCompatibleWithStandardLibrary

// MarshalErrorJSON writes error to stream as string
func MarshalErrorJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
p := *((*error)(ptr))
if p == nil {
stream.WriteString(`""`)
} else {
stream.WriteString(p.Error())
}
}

// MarshalErrorSliceJSON writes []error to stream as []string JSON blob
func MarshalErrorSliceJSON(ptr unsafe.Pointer, stream *jsoniter.Stream) {
a := *((*[]error)(ptr))
if len(a) == 0 {
stream.WriteEmptyArray()
} else {
stream.WriteArrayStart()
for i, e := range a {
if i > 0 {
stream.WriteMore()
}
stream.WriteString(e.Error())
}
stream.WriteArrayEnd()
}
}

func MarshalErrorJSONIsEmpty(_ unsafe.Pointer) bool {
return false
}

func MarshalErrorSliceJSONIsEmpty(_ unsafe.Pointer) bool {
return false
}

// WriteJSON writes an interface value encoded as JSON to w
func WriteJSON(w http.ResponseWriter, code int, value interface{}) {
// FIXME: we don't need to write the header in all/some circumstances.
w.Header().Set("Content-Type", "application/json")
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/server/register_pods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ func (s *APIServer) registerPodsHandlers(r *mux.Router) error {
// $ref: "#/responses/PodAlreadyStartedError"
// 404:
// $ref: "#/responses/NoSuchPod"
// 409:
// $ref: '#/responses/PodStartReport'
// 500:
// $ref: "#/responses/InternalError"
r.Handle(VersionedPath("/libpod/pods/{name}/start"), s.APIHandler(libpod.PodStart)).Methods(http.MethodPost)
Expand Down
67 changes: 59 additions & 8 deletions test/apiv2/rest_api/test_rest_v2_0_0.py
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import json
import os
import random
import shutil
import string
import subprocess
import sys
Expand Down Expand Up @@ -357,24 +356,25 @@ def test_pull(self):

def test_search_compat(self):
url = PODMAN_URL + "/v1.40/images/search"

# Had issues with this test hanging when repositories not happy
def do_search1():
payload = {'term': 'alpine'}
payload = {"term": "alpine"}
r = requests.get(url, params=payload, timeout=5)
self.assertEqual(r.status_code, 200, r.text)
objs = json.loads(r.text)
self.assertIn(type(objs), (list,))

def do_search2():
payload = {'term': 'alpine', 'limit': 1}
payload = {"term": "alpine", "limit": 1}
r = requests.get(url, params=payload, timeout=5)
self.assertEqual(r.status_code, 200, r.text)
objs = json.loads(r.text)
self.assertIn(type(objs), (list,))
self.assertEqual(len(objs), 1)

def do_search3():
payload = {'term': 'alpine', 'filters': '{"is-official":["true"]}'}
payload = {"term": "alpine", "filters": '{"is-official":["true"]}'}
r = requests.get(url, params=payload, timeout=5)
self.assertEqual(r.status_code, 200, r.text)
objs = json.loads(r.text)
Expand All @@ -383,14 +383,14 @@ def do_search3():
self.assertEqual(len(objs), 1)

def do_search4():
headers = {'X-Registry-Auth': 'null'}
payload = {'term': 'alpine'}
headers = {"X-Registry-Auth": "null"}
payload = {"term": "alpine"}
r = requests.get(url, params=payload, headers=headers, timeout=5)
self.assertEqual(r.status_code, 200, r.text)

def do_search5():
headers = {'X-Registry-Auth': 'invalid value'}
payload = {'term': 'alpine'}
headers = {"X-Registry-Auth": "invalid value"}
payload = {"term": "alpine"}
r = requests.get(url, params=payload, headers=headers, timeout=5)
self.assertEqual(r.status_code, 400, r.text)

Expand Down Expand Up @@ -619,6 +619,57 @@ def test_prune_compat(self):
# self.assertIn(img["Id"], prune_payload["ImagesDeleted"][1]["Deleted"])
self.assertIsNotNone(prune_payload["ImagesDeleted"][1]["Deleted"])

def test_pod_start_conflict(self):
"""Verify issue #8865"""

pod_name = list()
pod_name.append("Pod_" + "".join(random.choice(string.ascii_letters) for i in range(10)))
pod_name.append("Pod_" + "".join(random.choice(string.ascii_letters) for i in range(10)))

r = requests.post(
_url("/pods/create"),
json={
"name": pod_name[0],
"no_infra": False,
"portmappings": [{"host_ip": "127.0.0.1", "host_port": 8080, "container_port": 80}],
},
)
self.assertEqual(r.status_code, 201, r.text)
r = requests.post(
_url("/containers/create"),
json={
"pod": pod_name[0],
"image": "docker.io/alpine:latest",
"command": ["top"],
},
)
self.assertEqual(r.status_code, 201, r.text)

r = requests.post(
_url("/pods/create"),
json={
"name": pod_name[1],
"no_infra": False,
"portmappings": [{"host_ip": "127.0.0.1", "host_port": 8080, "container_port": 80}],
},
)
self.assertEqual(r.status_code, 201, r.text)
r = requests.post(
_url("/containers/create"),
json={
"pod": pod_name[1],
"image": "docker.io/alpine:latest",
"command": ["top"],
},
)
self.assertEqual(r.status_code, 201, r.text)

r = requests.post(_url(f"/pods/{pod_name[0]}/start"))
self.assertEqual(r.status_code, 409, r.text)

start = json.loads(r.text)
self.assertGreater(len(start["Errs"]), 0, r.text)


if __name__ == "__main__":
unittest.main()
Loading

0 comments on commit 3744eea

Please sign in to comment.