-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: system prune JSON unmarshalling error in remote client #27269
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
package bindings_test | ||
|
||
import ( | ||
"bytes" | ||
"encoding/json" | ||
"errors" | ||
"io" | ||
"net/http" | ||
|
||
"github.com/containers/podman/v5/pkg/bindings" | ||
"github.com/containers/podman/v5/pkg/domain/entities/reports" | ||
"github.com/containers/podman/v5/pkg/domain/entities/types" | ||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
) | ||
|
||
var _ = Describe("APIResponse Process method", func() { | ||
|
||
createMockResponse := func(jsonResponse []byte, statusCode int) *bindings.APIResponse { | ||
response := &http.Response{ | ||
StatusCode: statusCode, | ||
Body: io.NopCloser(bytes.NewBuffer(jsonResponse)), | ||
Header: make(http.Header), | ||
} | ||
response.Header.Set("Content-Type", "application/json") | ||
return &bindings.APIResponse{Response: response} | ||
} | ||
|
||
It("unmarshal the response with ContainerPruneReport with error", func() { | ||
errorStr := `replacing mount point "/tmp/CI_7Qsy/podman-e2e/b33ae357af1/merged": directory not empty` | ||
responseReport := types.SystemPruneReport{ | ||
ContainerPruneReports: []*reports.PruneReport{ | ||
{Id: "aec04392e9b2fe7c4a3", Size: 8219}, | ||
{ | ||
Id: "3d8a8789524a0c44a61", | ||
Size: 7238, | ||
Err: errors.New(errorStr), | ||
}, | ||
{Id: "e9ef46f3a3cd43c929b", Size: 0}, | ||
}, | ||
PodPruneReport: nil, | ||
ImagePruneReports: nil, | ||
NetworkPruneReports: nil, | ||
VolumePruneReports: nil, | ||
ReclaimedSpace: 15457, | ||
} | ||
|
||
jsonResponse, err := json.Marshal(responseReport) | ||
Expect(err).ToNot(HaveOccurred()) | ||
apiResponse := createMockResponse(jsonResponse, 200) | ||
|
||
var report types.SystemPruneReport | ||
err = apiResponse.Process(&report) | ||
Expect(err).ToNot(HaveOccurred()) | ||
|
||
Expect(report.ContainerPruneReports).To(HaveLen(3)) | ||
Expect(report.ReclaimedSpace).To(Equal(uint64(15457))) | ||
|
||
first := report.ContainerPruneReports[0] | ||
Expect(first.Id).To(Equal("aec04392e9b2fe7c4a3")) | ||
Expect(first.Size).To(Equal(uint64(8219))) | ||
Expect(first.Err).ToNot(HaveOccurred()) | ||
|
||
second := report.ContainerPruneReports[1] | ||
Expect(second.Id).To(Equal("3d8a8789524a0c44a61")) | ||
Expect(second.Size).To(Equal(uint64(7238))) | ||
Expect(second.Err).To(HaveOccurred()) | ||
Expect(second.Err.Error()).To(Equal(errorStr)) | ||
|
||
third := report.ContainerPruneReports[2] | ||
Expect(third.Id).To(Equal("e9ef46f3a3cd43c929b")) | ||
Expect(third.Size).To(Equal(uint64(0))) | ||
Expect(third.Err).ToNot(HaveOccurred()) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
package reports | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we actually running unit tests from this directory? I don't see any others in the directory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if we're running this. Can I remove it, and we just rely on the bindings test instead? |
||
|
||
import ( | ||
"encoding/json" | ||
"errors" | ||
"testing" | ||
|
||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
) | ||
|
||
func TestReports(t *testing.T) { | ||
RegisterFailHandler(Fail) | ||
RunSpecs(t, "Reports Suite") | ||
} | ||
|
||
var _ = Describe("PruneReport JSON", func() { | ||
Context("when marshaling and unmarshaling", func() { | ||
tests := []struct { | ||
name string | ||
report *PruneReport | ||
wantErr bool | ||
}{ | ||
{ | ||
name: "report with error", | ||
report: &PruneReport{ | ||
Id: "test-container-id", | ||
Err: errors.New("test error message"), | ||
Size: 1024, | ||
}, | ||
}, | ||
{ | ||
name: "report without error", | ||
report: &PruneReport{ | ||
Id: "test-container-id", | ||
Err: nil, | ||
Size: 2048, | ||
}, | ||
}, | ||
{ | ||
name: "empty report", | ||
report: &PruneReport{ | ||
Id: "", | ||
Err: nil, | ||
Size: 0, | ||
}, | ||
}, | ||
{ | ||
name: "report with complex error message from failing test case", | ||
report: &PruneReport{ | ||
Id: "3d8a8789524a0c44a61baa49ceedda7be069b0b3d01255b24013d2fb82168c7e", | ||
Err: errors.New(`replacing mount point "/tmp/CI_7Qsy/podman-e2e-213135586/subtest-1767990215/p/root/overlay/d9f554276b923c07bf708858b5f35774f9d2924fa4094b1583e56b33ae357af1/merged": directory not empty`), | ||
Size: 7238, | ||
}, | ||
}, | ||
{ | ||
name: "report with special characters in error", | ||
report: &PruneReport{ | ||
Id: "container-special", | ||
Err: errors.New(`error with "quotes" and \backslashes\ and newlines`), | ||
Size: 512, | ||
}, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
It("should handle "+tt.name, func() { | ||
jsonData, err := json.Marshal(tt.report) | ||
Expect(err).ToNot(HaveOccurred()) | ||
|
||
var unmarshalled PruneReport | ||
err = json.Unmarshal(jsonData, &unmarshalled) | ||
Expect(err).ToNot(HaveOccurred()) | ||
|
||
Expect(unmarshalled.Id).To(Equal(tt.report.Id)) | ||
Expect(unmarshalled.Size).To(Equal(tt.report.Size)) | ||
|
||
if tt.report.Err == nil { | ||
Expect(unmarshalled.Err).ToNot(HaveOccurred()) | ||
} else { | ||
Expect(unmarshalled.Err).To(HaveOccurred()) | ||
Expect(unmarshalled.Err.Error()).To(Equal(tt.report.Err.Error())) | ||
} | ||
}) | ||
} | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the
encoding/json
package doesn't know how to marshal and unmarshalerror
types, it translates them intostrings
. I'm not sure if a better implementation exists in Go