Library does not work when API Version is not an integer

[microreset]

Hello,

I'm trying to connect a kubernetes cluster to our Isilon.
After installing the Isilon simulator v8.1.0.4 I've tried to configure both RexRay and k8s_ici_provisioner using goisilon library with no success.
The API version number seems to be the problem:

Log output from RexRay:

[...]
DEBU[0000] got driver name                               driverName=isilon server=silver-servant-dz service=isilon time=1534170203774
DEBU[0000] 
    -------------------------- GOISILON HTTP REQUEST -------------------------
    GET /platform/latest/ HTTP/1.1
    Host: xxx.xxx.xxx.xxx:8080
    Authorization: Basic xxxx
   
 
DEBU[0000] 
    -------------------------- GOISILON HTTP RESPONSE -------------------------
    HTTP/1.1 200 Ok
    Transfer-Encoding: chunked
    Content-Type: application/json
    Date: Mon, 13 Aug 2018 14:23:24 GMT
    Server: Apache/2.4.29 (FreeBSD) OpenSSL/1.0.2o-fips mod_fastcgi/mod_fastcgi-SNAP-0910052141
    
    16
    
    {
    "latest" : "5.1"
    }
 
    0    

Oops, an error occured!
  libstorage: error creating isilon client

Log output from k8s_ici_provisioner

 kubectl logs k8s-isi-provisioner
I0815 17:50:59.031836       1 k8s_isi_provisioner.go:200] Starting Isilon Dynamic Provisioner version: v0.9-17-gc4f0
I0815 17:50:59.066990       1 k8s_isi_provisioner.go:249] ISI_QUOTA_ENABLED not set.  Quota support disabled
I0815 17:50:59.067037       1 k8s_isi_provisioner.go:253] Connecting to Isilon at: https://132.208.244.104:8080
I0815 17:50:59.067056       1 k8s_isi_provisioner.go:254] Creating exports at: /ifs/data
panic: strconv.ParseUint: parsing "5.1": invalid syntax
goroutine 1 [running]:
main.main()
        /home/travis/gopath/src/github.com/xphyr/k8s_isi_provisioner/k8s_isi_provisioner.go:266 +0xb0c

What I understand is that the library is trying to parse a integer and gets a float instead
panic: strconv.ParseUint: parsing "5.1": invalid syntax
This is the code from api/api.go line 179

Any ideas how to fix this?

Thanks

[anilreddyv]

Having similar issues, any update on this issue and how to fix it?

I1031 15:58:13.772658 1 k8s_isi_provisioner.go:200] Starting Isilon Dynamic Provisioner version: v0.9-17-gc4f0
I1031 15:58:13.790597 1 k8s_isi_provisioner.go:246] Isilon quotas enabled
I1031 15:58:13.790623 1 k8s_isi_provisioner.go:253] Connecting to Isilon at: https://x.x.x.x:8080
I1031 15:58:13.790632 1 k8s_isi_provisioner.go:254] Creating exports at: /ifs/data1/test
panic: invalid character '<' looking for beginning of value
goroutine 1 [running]:
main.main()
/home/travis/gopath/src/github.com/xphyr/k8s_isi_provisioner/k8s_isi_provisioner.go:266 +0xb0c

[tenortim]

I've forked the repo and provided a fix for this at https://github.com/tenortim/goisilon.
Could you possibly try changing your Go import to "github.com/tenortim/goisilon" and see if that fixes the problem for you?
FYI, I work for Isilon Engineering.

[hpanike]

I submitted a fix for this in PR #38. The fix involves changing the string parse to a float. All types and getter functions were changed to reflect float32.

[tenortim]

Hi Hayden,
I'd recommend against making it a float. That isn't what we mean by e.g. "3.5". The first number is the major release of the API, and the option ".N" component is for minor fixes in maintenance releases. As such, it's really intended to be two different numbers, and in most cases, the consumer is only going to care about the major revision.

[hpanike]

Hi Tim,
I before I had it Parsing as a float and then converted back to the standard uint8. If the minor version is not needed, then it would be best to go back that direction. Is there a reason that /platform/lastest returns the major and minor version instead of just the major? I assume most people's use case is to use the resulting value to access that version of the platform API.

[tenortim]

Hi Hayden,
it's for small fixes inside of a maintenance release train. We didn't actually use this in the 7.2 release train, but in subsequent releases, we have made small fixes to the API within a maintenance release train (e.g. in 8.0.0.7 in the 8.0.0 release train) and that [.N] part of the return from /platorm/latest is for that. I think in general, nobody will need to change behavior based on this, and so reverting to just converting the initial integer is fine.

[ebressler]

I see this is still open and this also that it is still an issue in the current release. What is the right solution here? Seems like it should be just be dropping the decimal places, but the current code doesn't do that still.

[ebressler]

@tenortim, would you want a PR that would fix this as described where the decimal is dropped?

[tenortim]

Hi @ebressler,
I do not have administrative rights to this project. As I understand it, the code team at Dell was dissolved and we have other methods to engage in Open Source efforts.
I personally forked this project at https://github.com/tenortim/goisilon and fixed this issue in commit tenortim@16a3c8c

The API major version increments with each major release of OneFS (and, thus far, there has always been added functionality in each of those releases). If we add functionality within an OS release, or if we need to fix a problem in an existing endpoint, the API stability contract means we can't change the existing behavior, and so we create a ".N" (N=1, 2, ...) endpoint within that API version to make the change. Those are legitimate endpoints that can be called and that differ from the base endpoint in some way e.g. schema or semantics. As such, it is necessary to parse and support them to be strictly correct.

[ebressler]

Thanks for the info @tenortim. So it looks like this isn't being maintained anymore. :( I will have to decide what that means. It is kinda a real pain to use a fork in go mod and work to fix things easily. I also did a fork and fixed this for my needs (a little different than then way you do it but similar).