Some adjustments to protobuf field and request names

[gbregman]

Fixes #518

[gbregman]

@epuertat notice that I didn't rename get_subsystems() and set_ana_state() to have the verb in the end as they are used in the Ceph monitor. We might do it later before merging the monitor.

[epuertat]

CC: @nizamial09

At this point in the release cycle, I'm wondering if we should go for this or wait for a 2.y.z release (and even though, we should aim at minimizing the amount of breaking changes, and keeping as much as possible backward-compatible interfaces).

As is, this PR currently brings breaking changes and would need updates in the Ceph side (Dashboard embeds a .proto file).

The alternative of making this backward compatible is viable though, and involves leaving both interfaces. Something along the lines of:

service Gateway {
	rpc list_namespaces(list_namespaces_req) returns(namespaces_info) {}
	message list_namespaces_req {
		string subsystem = 1;
		optional uint32 nsid = 2;
		optional string uuid = 3;
	}
  ...
}

service GatewayV2 {
	rpc namespace_list(namespace_list_req) returns(namespaces_info) {}
	message namespace_list_req {
		string subsystem_nqn = 1;
		optional uint32 nsid = 2;
		optional string uuid = 3;
	}
  ...
}

What do you think @gbregman ?

[gbregman]

@epuertat we plan to advance the version in the near future. I'm not sure having a double interface is worth it. I would rather to either leave it as it is now or change it now before we release.

[epuertat]

@epuertat we plan to advance the version in the near future. I'm not sure having a double interface is worth it. I would rather to either leave it as it is now or change it now before we release.

I understand that after the ceph-mon based HA, ceph-nvmeof and ceph projects are no longer loosely coupled, and versioning looks granted 'cause everything is deployed together.

Quite the opposite, Ceph Dashboard is committed to maintain a stable RESTful API across major Ceph releases (that including clients outside the Ceph ecosystem), and that can be unattainable if the underlying gRPC API is arbitrarily mutating (not to mention the complexities of version upgrades).

So, before making any post-v1 breaking-changes to the gRPC proto, please let's sit all together and discuss about versioning & stability approaches (CC: @caroav @oritwas @jdurgin ). I'm sure that you'll greatly appreciate it when a new Ceph release is deployed and you don't have to worry about upgrade/downgrade processes.

[epuertat]

@gbregman if backward compatibility is not considered for this PR, then I'm ok with closing it. As said, thank you very much for the effort you put here! ❤️

[gbregman]

@epuertat as the code is not released yet we didn't bother to maintain backward compatibility. Of course this will be different once it's used by customers. Anyway, I'll let @caroav decide about that.

[caroav]

@epuertat if these changes require work that we cannot afford right now in the API, then we will not merge it now. I wanted to have a clean first version. But its up to you. We can always do it in V2.
Another point @epuertat is that we also made a small change to the add listener command. See #529. This was changed to use host-name instead of GW name. This was a must because ceph adm assigns different names to daemons after rm command. So this change is something you will also need to do in the API for V1.

[epuertat]

Thanks @caroav. @gbregman already told me about the "listener add" change.

Regarding this PR, I think it's too risky to deliver big changes to the API at this time, specially given those are backward-incompatible. Let's keep this open if you want for future reference.

[epuertat]

BTW, it'd be great if we would increase the GW version digits with API changes. That would make it easy to "name" at what stage every component is (e.g.: Ceph Dashboard technically supports some 1.0.0 version, but that 1.0.0 means something different in these last PRs).

My suggestion is that, at least during intra-release changes, we still increase some version digit (e.g.: according to Semantic Versioning: x.y.z-<pre-release-digit>: 1.0.0-1, 1.0.0-2).

[caroav]

@epuertat so you suggest that the GW version will track only API changes, or also code fixes? Do we need one version to API and another one to general code fixes/releases maybe?

[gbregman]

PR was closed for now. We'll consider to re-do it in a future version.

[epuertat]

@epuertat so you suggest that the GW version will track only API changes, or also code fixes? Do we need one version to API and another one to general code fixes/releases maybe?

@caroav: API changes is the top reason to include versioning, but not only API changes. Quoting Semantic Versioning spec:

Given a version number MAJOR.MINOR.PATCH, increment the:

• MAJOR version when you make incompatible API changes
• MINOR version when you add functionality in a backward compatible manner
• PATCH version when you make backward compatible bug fixes

Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

I understand that within pre-release development, versioning might be relaxed, but here we have at least 3 components from different codebases (Ceph NVMe-oF Gateway, ceph-mon and Ceph-Dashboard) that need to be in sync. If versioning is not respected even within pre-release period, it basically shifts all the burden to the consumers of the NVMe-oF API (ceph-mon and Ceph-Dashboard).

NVMe-oF team is taking care of keeping ceph-mon in sync, but not the Dashboard, so honoring an API versioning is the best way to coordinate work across components (and detect potential version mismatches).