Skip to content

Commit 5993d7b

Browse files
rook: apply patch to use jq to fix invalid JSON from ceph osd dump
The ceph osd dump -f json command can output invalid JSON containing values like "inf" which cannot be parsed by encoding/json. This causes errors like "invalid character 'i' looking for beginning of value". Fix this by piping the output through jq, which accepts such invalid JSON and outputs valid JSON that encoding/json can parse. Signed-off-by: Ryotaro Banno <ryotaro.banno@gmail.com>
1 parent 58e9c86 commit 5993d7b

File tree

3 files changed

+129
-1
lines changed

3 files changed

+129
-1
lines changed

rook/Dockerfile

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ RUN HELM_VERSION=$(grep "^HELM_VERSION" ${ROOK_DIR}/build/makelib/helm.mk | cut
1919
ln -s ${ROOK_DIR}/.cache/tools/linux_amd64/helm-${HELM_VERSION} ${ROOK_DIR}/.cache/tools/linux_amd64/helm
2020

2121
RUN git checkout v${ROOK_VERSION}
22+
23+
# NOTE: This patch should be removed after the following issue is resolved:
24+
# https://tracker.ceph.com/issues/66215
25+
COPY use-jq-for-ceph-osd-dump-json-output.patch .
26+
RUN git apply use-jq-for-ceph-osd-dump-json-output.patch
27+
2228
RUN mkdir -p /tmp/rook
2329
RUN make build IMAGES="ceph" BUILD_CONTAINER_IMAGE=false BUILD_CONTEXT_DIR=/tmp/rook SAVE_BUILD_CONTEXT_DIR=true
2430
# Copy output artifacts to root directory to minimize differences between Stage2 and upstream Dockerfile.
@@ -38,6 +44,10 @@ COPY --from=build rook toolbox.sh set-ceph-debug-level /usr/local/bin/
3844
COPY --from=build ceph-monitoring /etc/ceph-monitoring
3945
COPY --from=build rook-external /etc/rook-external/
4046

47+
# NOTE: This check should be removed after the following issue is resolved:
48+
# https://tracker.ceph.com/issues/66215
49+
RUN jq --version
50+
4151
# create or modify owner and permissions to make a watch-active container of a MGR work properly
4252
RUN groupadd rook -g 2016 && \
4353
useradd rook -u 2016 -g rook

rook/TAG

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.18.6.1
1+
1.18.6.2
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
diff --git a/pkg/daemon/ceph/client/osd.go b/pkg/daemon/ceph/client/osd.go
2+
index 49d082e90881..95dbbbef3d66 100644
3+
--- a/pkg/daemon/ceph/client/osd.go
4+
+++ b/pkg/daemon/ceph/client/osd.go
5+
@@ -16,9 +16,11 @@ limitations under the License.
6+
package client
7+
8+
import (
9+
+ "bytes"
10+
"encoding/json"
11+
"fmt"
12+
"math"
13+
+ "os/exec"
14+
"strconv"
15+
"strings"
16+
17+
@@ -303,8 +305,16 @@ func GetOSDDump(context *clusterd.Context, clusterInfo *ClusterInfo) (*OSDDump,
18+
return nil, errors.Wrap(err, "failed to get osd dump")
19+
}
20+
21+
+ // Use jq to fix potentially invalid JSON from ceph osd dump
22+
+ jqCmd := exec.Command("jq", ".")
23+
+ jqCmd.Stdin = bytes.NewReader(buf)
24+
+ validJSON, err := jqCmd.Output()
25+
+ if err != nil {
26+
+ return nil, errors.Wrap(err, "failed to run jq on osd dump output")
27+
+ }
28+
+
29+
var osdDump OSDDump
30+
- if err := json.Unmarshal(buf, &osdDump); err != nil {
31+
+ if err := json.Unmarshal(validJSON, &osdDump); err != nil {
32+
return nil, errors.Wrap(err, "failed to unmarshal osd dump response")
33+
}
34+
35+
diff --git a/pkg/daemon/ceph/client/osd_test.go b/pkg/daemon/ceph/client/osd_test.go
36+
index 282cbf3f3703..2d4550607f6a 100644
37+
--- a/pkg/daemon/ceph/client/osd_test.go
38+
+++ b/pkg/daemon/ceph/client/osd_test.go
39+
@@ -214,3 +214,79 @@ func TestOSDOkToStop(t *testing.T) {
40+
assert.Equal(t, "--max=0", seenArgs[3])
41+
})
42+
}
43+
+
44+
+func TestGetOSDDump(t *testing.T) {
45+
+ // Valid JSON output from ceph osd dump
46+
+ validOSDDump := `{
47+
+ "osds": [
48+
+ {"osd": 0, "up": 1, "in": 1},
49+
+ {"osd": 1, "up": 1, "in": 1}
50+
+ ],
51+
+ "flags": "nodown,sortbitwise",
52+
+ "crush_node_flags": {},
53+
+ "full_ratio": 0.95,
54+
+ "backfillfull_ratio": 0.9,
55+
+ "nearfull_ratio": 0.85
56+
+ }`
57+
+
58+
+ // Invalid JSON with "inf" value that ceph osd dump can produce
59+
+ // This causes "invalid character 'i' looking for beginning of value" error
60+
+ invalidOSDDumpWithInf := `{
61+
+ "osds": [
62+
+ {"osd": 0, "up": 1, "in": 1}
63+
+ ],
64+
+ "flags": "nodown",
65+
+ "crush_node_flags": {},
66+
+ "full_ratio": 0.95,
67+
+ "backfillfull_ratio": 0.9,
68+
+ "nearfull_ratio": inf
69+
+ }`
70+
+
71+
+ t.Run("valid JSON is parsed correctly", func(t *testing.T) {
72+
+ executor := &exectest.MockExecutor{}
73+
+ executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
74+
+ logger.Infof("Command: %s %v", command, args)
75+
+ if args[0] == "osd" && args[1] == "dump" {
76+
+ return validOSDDump, nil
77+
+ }
78+
+ return "", errors.Errorf("unexpected ceph command %q", args)
79+
+ }
80+
+
81+
+ context := &clusterd.Context{Executor: executor}
82+
+ clusterInfo := AdminTestClusterInfo("mycluster")
83+
+
84+
+ dump, err := GetOSDDump(context, clusterInfo)
85+
+ assert.NoError(t, err)
86+
+ assert.NotNil(t, dump)
87+
+ assert.Equal(t, 2, len(dump.OSDs))
88+
+ assert.Equal(t, "nodown,sortbitwise", dump.Flags)
89+
+ assert.Equal(t, 0.95, dump.FullRatio)
90+
+ assert.Equal(t, 0.9, dump.BackfillFullRatio)
91+
+ assert.Equal(t, 0.85, dump.NearFullRatio)
92+
+ })
93+
+
94+
+ t.Run("invalid JSON with inf is fixed by jq and parsed correctly", func(t *testing.T) {
95+
+ executor := &exectest.MockExecutor{}
96+
+ executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
97+
+ logger.Infof("Command: %s %v", command, args)
98+
+ if args[0] == "osd" && args[1] == "dump" {
99+
+ return invalidOSDDumpWithInf, nil
100+
+ }
101+
+ return "", errors.Errorf("unexpected ceph command %q", args)
102+
+ }
103+
+
104+
+ context := &clusterd.Context{Executor: executor}
105+
+ clusterInfo := AdminTestClusterInfo("mycluster")
106+
+
107+
+ dump, err := GetOSDDump(context, clusterInfo)
108+
+ assert.NoError(t, err)
109+
+ assert.NotNil(t, dump)
110+
+ assert.Equal(t, 1, len(dump.OSDs))
111+
+ assert.Equal(t, "nodown", dump.Flags)
112+
+ assert.Equal(t, 0.95, dump.FullRatio)
113+
+ assert.Equal(t, 0.9, dump.BackfillFullRatio)
114+
+ // jq converts "inf" to 1.7976931348623157e+308 (max float64)
115+
+ // The important thing is that parsing succeeds without error
116+
+ assert.NotEqual(t, 0.0, dump.NearFullRatio)
117+
+ })
118+
+}

0 commit comments

Comments
 (0)