search

open-iscsi / targetd

Remote configuration of a LIO-based storage appliance
GNU General Public License v3.0
72 stars 28 forks source link

go library #64

Closed dsonck92 closed 3 years ago

dsonck92 commented 4 years ago

It might be useful to create a go library to wrap the functionality of targetd. I'm already working on the mentioned targetd provisioner so once this has enough functionality, I can extract the interface to a separate library which could be adopted.

It does then become a question of whether we want to ship the library inside this repository, ensuring that it will always stay matched with targetd. There are possibilities to create a vanity url that essentially rewrites the package to something else to match the expectations of the user. E.g. "go.open-iscsi.com/targetd" which then points to some subdirectory of this, say github.com/open-iscsi/targetd/go/

dsonck92 commented 4 years ago

Obviously, that vanity url can also point to github.com/open-iscsi/targetd-go/ if embedded alongside targetd is not a requirement

tasleson commented 4 years ago

No strong preference where the golang client library resides, whatever works best for you.

If we construct the library and API correctly it should always work, it just might not have all the functionality that is currently available in the targetd service, so I'm not concerned with them being in lock step.

dsonck92 commented 4 years ago

Agreed, then I propose to keep it a separate repository. When I've incorporated the chown feature inside the provisioner and refactored it to extract the client part, it can be forked under open-iscsi

whywaita commented 3 years ago

FYI: I implemented Go library for targetd!

https://github.com/lovi-cloud/go-targetd

dsonck92 commented 3 years ago

@whywaita interesting, I will take a look at it and try to utilize it with the targetd-provisioner and see if it covers all the requirements. One thing I would like to point out is that your library currently doesn't seem to parse error codes.

During testing of my provisioner with my own code, one problem I found was that sometimes I need to delete exports that were already deleted. Obviously, this is an error, as something that does not exist, cannot be removed. However, in the case of the provisioner, I consider the "export not found" during delete a non-error, and return success. So I had to look at this and came up with this:

https://git.sonck.nl/misc/targetd-provisioner/-/commit/d593a300237ff82055d902b31b20f9bef4c2a74e#c69d68e2ffa4180e1047dcfa69119ec295a1e2b2_138_144

So probably you want to create a higher level error format. But, that's not for this issue so I might open an issue for it on your repo.

whywaita commented 3 years ago

@dsonck92 okay, please open an issue in my repo if you found a problem 🙏