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.

Sign up for GitHub

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

Jump to bottom

swift: fix file already closed issue #1489

Closed
wants to merge 1 commit into from
Closed

swift: fix file already closed issue #1489

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 11 additions & 2 deletions pkg/objstore/swift/swift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package swift

import (
"bytes"
"context"
"fmt"
"io"
Expand All @@ -19,7 +20,7 @@ import (
"github.com/gophercloud/gophercloud/pagination"
"github.com/pkg/errors"
"github.com/thanos-io/thanos/pkg/objstore"
yaml "gopkg.in/yaml.v2"
"gopkg.in/yaml.v2"
)

// DirDelim is the delimiter used to model a directory structure in an object store bucket.
Expand Down Expand Up @@ -149,7 +150,15 @@ func (c *Container) IsObjNotFoundErr(err error) bool {

// Upload writes the contents of the reader as an object into the container.
func (c *Container) Upload(ctx context.Context, name string, r io.Reader) error {
options := &objects.CreateOpts{Content: r}
// to avoid file already closed error pass a Reader that supports seek
buf := new(bytes.Buffer)
_, err := buf.ReadFrom(r)
if err != nil {
return err
}
readSeeker := strings.NewReader(buf.String())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue seems to be due to a bug in the vendored gophercloud dependency and looks like it has been fixed upstream: https://github.com/rackspace/gophercloud/blob/master/openstack/objectstorage/v1/objects/requests.go#L219. Do you think you could re-vendor gophercloud and run your tests?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure I can check it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sudhi-vm looks like gophercloud was moved from github.com/rackspace to https://github.com/gophercloud and it looks like the version 0.3.0 is the latest one.
I am not an io.Reader expert but I guess the ioutil.ReadAll will close the reader and causes this issue.
https://github.com/gophercloud/gophercloud/blob/master/openstack/objectstorage/v1/objects/requests.go#L198-L199


options := &objects.CreateOpts{Content: readSeeker}
res := objects.Create(c.client, c.name, name, options)
return res.Err
}
Expand Down