Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add status messages to podman --remote commit #20377

Merged
merged 1 commit into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/api/handlers/compat/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {

commitImage, err := ctr.Commit(r.Context(), destImage, options)
if err != nil && !strings.Contains(err.Error(), "is not running") {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("CommitFailure: %w", err))
utils.Error(w, http.StatusInternalServerError, err)
return
}
utils.WriteResponse(w, http.StatusCreated, entities.IDResponse{ID: commitImage.ID()})
Expand Down
14 changes: 4 additions & 10 deletions pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/containers/podman/v4/pkg/api/handlers/utils"
api "github.com/containers/podman/v4/pkg/api/types"
"github.com/containers/podman/v4/pkg/auth"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/channel"
"github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/pkg/util"
Expand Down Expand Up @@ -781,14 +782,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
var stepErrors []string

for {
type BuildResponse struct {
Stream string `json:"stream,omitempty"`
Error *jsonmessage.JSONError `json:"errorDetail,omitempty"`
// NOTE: `error` is being deprecated check https://github.com/moby/moby/blob/master/pkg/jsonmessage/jsonmessage.go#L148
ErrorMessage string `json:"error,omitempty"` // deprecate this slowly
Aux json.RawMessage `json:"aux,omitempty"`
}
m := BuildResponse{}
m := images.BuildResponse{}

select {
case e := <-stdout.Chan():
Expand Down Expand Up @@ -818,7 +812,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
// output all step errors irrespective of quiet
// flag.
for _, stepError := range stepErrors {
t := BuildResponse{}
t := images.BuildResponse{}
t.Stream = stepError
if err := enc.Encode(t); err != nil {
stderr.Write([]byte(err.Error()))
Expand All @@ -827,7 +821,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
m.ErrorMessage = string(e)
m.Error = &jsonmessage.JSONError{
Message: m.ErrorMessage,
Message: string(e),
}
if err := enc.Encode(m); err != nil {
logrus.Warnf("Failed to json encode error %v", err)
Expand Down
84 changes: 79 additions & 5 deletions pkg/api/handlers/libpod/images.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package libpod

import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
Expand All @@ -18,6 +20,8 @@ import (
"github.com/containers/podman/v4/pkg/api/handlers"
"github.com/containers/podman/v4/pkg/api/handlers/utils"
api "github.com/containers/podman/v4/pkg/api/types"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/channel"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/domain/entities/reports"
"github.com/containers/podman/v4/pkg/domain/infra/abi"
Expand All @@ -30,7 +34,9 @@ import (
"github.com/containers/storage/pkg/archive"
"github.com/containers/storage/pkg/chrootarchive"
"github.com/containers/storage/pkg/idtools"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/gorilla/schema"
"github.com/sirupsen/logrus"
)

// Commit
Expand Down Expand Up @@ -442,6 +448,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
Pause bool `schema:"pause"`
Squash bool `schema:"squash"`
Repo string `schema:"repo"`
Stream bool `schema:"stream"`
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
Tag string `schema:"tag"`
}{
Format: "oci",
Expand Down Expand Up @@ -480,7 +487,6 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
SystemContext: sc,
PreferredManifestType: mimeType,
}

if len(query.Tag) > 0 {
tag = query.Tag
}
Expand All @@ -498,12 +504,80 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) {
if len(query.Repo) > 0 {
destImage = fmt.Sprintf("%s:%s", query.Repo, tag)
}
commitImage, err := ctr.Commit(r.Context(), destImage, options)
if err != nil && !strings.Contains(err.Error(), "is not running") {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("CommitFailure: %w", err))

if !query.Stream {
commitImage, err := ctr.Commit(r.Context(), destImage, options)
if err != nil && !strings.Contains(err.Error(), "is not running") {
utils.Error(w, http.StatusInternalServerError, err)
return
}
utils.WriteResponse(w, http.StatusOK, entities.IDResponse{ID: commitImage.ID()})
return
}
utils.WriteResponse(w, http.StatusOK, entities.IDResponse{ID: commitImage.ID()})

// Channels all mux'ed in select{} below to follow API commit protocol
stdout := channel.NewWriter(make(chan []byte))
defer stdout.Close()
// Channels all mux'ed in select{} below to follow API commit protocol
options.CommitOptions.ReportWriter = stdout
var (
commitImage *libimage.Image
commitErr error
)
runCtx, cancel := context.WithCancel(r.Context())
go func() {
defer cancel()
commitImage, commitErr = ctr.Commit(r.Context(), destImage, options)
}()

flush := func() {
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
if flusher, ok := w.(http.Flusher); ok {
flusher.Flush()
}
}

enc := json.NewEncoder(w)

statusWritten := false
writeStatusCode := func(code int) {
if !statusWritten {
w.WriteHeader(code)
w.Header().Set("Content-Type", "application/json")
flush()
statusWritten = true
}
}

for {
m := images.BuildResponse{}

select {
case e := <-stdout.Chan():
writeStatusCode(http.StatusOK)
m.Stream = string(e)
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
if err := enc.Encode(m); err != nil {
logrus.Errorf("%v", err)
}
flush()
case <-runCtx.Done():
if commitErr != nil {
m.Error = &jsonmessage.JSONError{
Message: commitErr.Error(),
}
} else {
m.Stream = commitImage.ID()
}
if err := enc.Encode(m); err != nil {
logrus.Errorf("%v", err)
}
flush()
return
case <-r.Context().Done():
cancel()
logrus.Infof("Client disconnect reported for commit")
return
}
}
}

func UntagImage(w http.ResponseWriter, r *http.Request) {
Expand Down
36 changes: 22 additions & 14 deletions pkg/api/server/register_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -1281,35 +1281,43 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
// description: the name or ID of a container
// required: true
// - in: query
// name: repo
// name: author
// type: string
// description: the repository name for the created image
// description: author of the image
// - in: query
// name: tag
// type: string
// description: tag name for the created image
// name: changes
// description: instructions to apply while committing in Dockerfile format (i.e. "CMD=/bin/foo")
// type: array
// items:
// type: string
// - in: query
// name: comment
// type: string
// description: commit message
// - in: query
// name: author
// name: format
// type: string
// description: author of the image
// description: format of the image manifest and metadata (default "oci")
// - in: query
// name: pause
// type: boolean
// description: pause the container before committing it
// - in: query
// name: changes
// description: instructions to apply while committing in Dockerfile format (i.e. "CMD=/bin/foo")
// type: array
// items:
// type: string
// name: squash
// type: boolean
// description: squash the container before committing it
// - in: query
// name: format
// name: repo
// type: string
// description: format of the image manifest and metadata (default "oci")
// description: the repository name for the created image
// - in: query
// name: stream
// type: boolean
// description: output from commit process
// - in: query
// name: tag
// type: string
// description: tag name for the created image
// produces:
// - application/json
// responses:
Expand Down
59 changes: 59 additions & 0 deletions pkg/bindings/containers/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@ package containers

import (
"context"
"errors"
"fmt"
"io"
"net/http"
"os"
"strings"

"github.com/containers/podman/v4/pkg/bindings"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/storage/pkg/regexp"
)

var iidRegex = regexp.Delayed(`^[0-9a-f]{12}`)

// Commit creates a container image from a container. The container is defined by nameOrID. Use
// the CommitOptions for finer grain control on characteristics of the resulting image.
func Commit(ctx context.Context, nameOrID string, options *CommitOptions) (entities.IDResponse, error) {
Expand All @@ -30,5 +39,55 @@ func Commit(ctx context.Context, nameOrID string, options *CommitOptions) (entit
}
defer response.Body.Close()

if !response.IsSuccess() {
return id, response.Process(err)
}

if !options.GetStream() {
return id, response.Process(&id)
}
stderr := os.Stderr
body := response.Body.(io.Reader)
dec := json.NewDecoder(body)
for {
var s images.BuildResponse
select {
// FIXME(vrothberg): it seems we always hit the EOF case below,
// even when the server quit but it seems desirable to
// distinguish a proper build from a transient EOF.
case <-response.Request.Context().Done():
return id, nil
default:
// non-blocking select
}

if err := dec.Decode(&s); err != nil {
if errors.Is(err, io.ErrUnexpectedEOF) {
return id, fmt.Errorf("server probably quit: %w", err)
}
// EOF means the stream is over in which case we need
// to have read the id.
if errors.Is(err, io.EOF) && id.ID != "" {
break
}
return id, fmt.Errorf("decoding stream: %w", err)
}

switch {
case s.Stream != "":
raw := []byte(s.Stream)
stderr.Write(raw)
if iidRegex.Match(raw) {
id.ID = strings.TrimSuffix(s.Stream, "\n")
return id, nil
}
case s.Error != nil:
// If there's an error, return directly. The stream
// will be closed on return.
return id, errors.New(s.Error.Message)
default:
return id, errors.New("failed to parse build results stream, unexpected input")
}
}
return id, response.Process(&id)
}
1 change: 1 addition & 0 deletions pkg/bindings/containers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type CommitOptions struct {
Comment *string
Format *string
Pause *bool
Stream *bool
Squash *bool
Repo *string
Tag *string
Expand Down
15 changes: 15 additions & 0 deletions pkg/bindings/containers/types_commit_options.go

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

19 changes: 12 additions & 7 deletions pkg/bindings/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containers/storage/pkg/fileutils"
"github.com/containers/storage/pkg/ioutils"
"github.com/containers/storage/pkg/regexp"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/docker/go-units"
"github.com/hashicorp/go-multierror"
jsoniter "github.com/json-iterator/go"
Expand All @@ -40,6 +41,14 @@ type devino struct {

var iidRegex = regexp.Delayed(`^[0-9a-f]{12}`)

type BuildResponse struct {
Stream string `json:"stream,omitempty"`
Error *jsonmessage.JSONError `json:"errorDetail,omitempty"`
// NOTE: `error` is being deprecated check https://github.com/moby/moby/blob/master/pkg/jsonmessage/jsonmessage.go#L148
ErrorMessage string `json:"error,omitempty"` // deprecate this slowly
Aux json.RawMessage `json:"aux,omitempty"`
}

// Build creates an image using a containerfile reference
func Build(ctx context.Context, containerFiles []string, options entities.BuildOptions) (*entities.BuildReport, error) {
if options.CommonBuildOpts == nil {
Expand Down Expand Up @@ -603,11 +612,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO

var id string
for {
var s struct {
Stream string `json:"stream,omitempty"`
Error string `json:"error,omitempty"`
}

var s BuildResponse
select {
// FIXME(vrothberg): it seems we always hit the EOF case below,
// even when the server quit but it seems desirable to
Expand Down Expand Up @@ -637,10 +642,10 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
if iidRegex.Match(raw) {
id = strings.TrimSuffix(s.Stream, "\n")
}
case s.Error != "":
case s.Error != nil:
// If there's an error, return directly. The stream
// will be closed on return.
return &entities.BuildReport{ID: id, SaveFormat: saveFormat}, errors.New(s.Error)
return &entities.BuildReport{ID: id, SaveFormat: saveFormat}, errors.New(s.Error.Message)
default:
return &entities.BuildReport{ID: id, SaveFormat: saveFormat}, errors.New("failed to parse build results stream, unexpected input")
}
Expand Down
Loading