mistifyio/go-zfs

"error while getting pool information strconv.ParseUint: parsing "": invalid syntax" in docker info

Boldie opened this issue · 15 comments

While installing docker I had a serious problem with my ZFS-Installation. My installation had no fragmentation determination enabled and therefore the command

root@atomic:~# zpool get -p name,health,allocated,size,free,readonly,dedupratio,fragmentation,freeing,leaked tank
NAME  PROPERTY       VALUE    SOURCE
tank  name           tank     -
tank  health         ONLINE   -
tank  allocated      2098558267392  -
tank  size           2989297238016  -
tank  free           890738970624  -
tank  readonly       off      -
tank  dedupratio     1.00x    -
tank  fragmentation  -        -
tank  freeing        0        default
tank  leaked         0        default

can not be parsed. the lib struggles with the "-" in fragmentation. This results in the following error message: "Zpool: error while getting pool information strconv.ParseUint: parsing "": invalid syntax"

Original issue in Docker issue trakcer: moby/moby#30551

ping @Mic92 ptal

looks related to #62 ?

Mic92 commented

I think this was fixed upstream here: 4a9c7e5
Docker needs to update its go-zfs fork.

UPDATE: no, this is a different fix.

Ah, even better; I can bump the vendored package

Mic92 commented

I can quickly make fix.

Oh, already working on it, I'll ping you when it's up

Mic92 commented

@thaJeztah

untested:

diff --git a/utils.go b/utils.go
index 3ae4236..2a27718 100644
--- a/utils.go
+++ b/utils.go
@@ -340,12 +340,15 @@ func (z *Zpool) parseLine(line []string) error {
        case "free":
                err = setUint(&z.Free, val)
        case "fragmentation":
-               // Trim trailing "%" before parsing uint
-               i := strings.Index(val, "%")
-               if i < 0 {
-                       i = len(val)
+               // if no fragmentation determination is enabled, this field is a `-`
+               if val != "-" {
+                       // Trim trailing "%" before parsing uint
+                       i := strings.Index(val, "%")
+                       if i < 0 {
+                               i = len(val)
+                       }
+                       err = setUint(&z.Fragmentation, val[:i])
                }
-               err = setUint(&z.Fragmentation, val[:i])
        case "readonly":
                z.ReadOnly = val == "on"
        case "freeing":

The code is vendored, so bumping the commit should probably do it :)

Mic92 commented

@thaJeztah this fix is not upstream yet: #56 (comment)

UPDATE I work on a pull request.

oh! I must've misunderstood; isn't bumping the vendored github.com/mistifyio/go-zfs package bringing in the change?

Oh! it's the if val != "-" { that you added; now I see 😊 yes, if you can open a PR for that, then I can bump the vendor after it's merged

I get the same error from docker with the following output from the zfs command:

# zpool get -p name,health,allocated,size,free,readonly,dedupratio,fragmentation,freeing,leaked rpool
NAME   PROPERTY       VALUE    SOURCE
rpool  name           rpool    -
rpool  health         ONLINE   -
rpool  allocated      548820271104  -
rpool  size           2989297238016  -
rpool  free           2440476966912  -
rpool  readonly       off      -
rpool  dedupratio     1.00     -
rpool  fragmentation  0        -
rpool  freeing        0        -
rpool  leaked         0        -

Is it safe to assume that this is only a cosmetic issue and doesn't affect the performance of docker?

Same question as @spamwax; the question is over a year old. The issue is 3 years old. This project appears to be inactive.

Mic92 commented

Well none of the people having this problem tested my patch, so we cannot merge it.
On my systems zfs + docker works fine.

@Mic92 Thank you for providing the patch. I've replied on moby/moby#30551.