Add support for Consul as store engine

[borismartinovic01]

This PR introduces Consul as a store engine and tries to align the implementation with the implementations of etcd and zookeeper as much as possible.

Constants description:

1. sessionTTL = 10 seconds - Minimum required by Consul
2. lockDelay = 1 millisecond - Consul defines a default lock delay of 15 seconds. In the docs it is mentioned that this can be disabled by setting this param to zero value but looking at the source code of Consul's session.go implementation this is not possible, so it is set to the minimum that's bigger than zero value
3. defaultElectPath = "kvrocks/controller/leader" - Removed the backslash from the beginning since Consul requires keys not to start with a backslash.

Testing

1. Tested the implementation by running Consul as Docker container locally and running UTs

Tests were kept almost the same as for etcd and zookeeper except the backslash was removed from keys due to limitation mentioned above and wait duration was increased from 15 to 25 seconds in the second test since the sessionTTL is a bit bigger.

[git-hulk]

@borismartinovic01 Thanks for your contribution.

The CI failed due to forgetting to add github.com/hashicorp/consul/api into go.mod. You might need to add the consul to scripts/docker/docker-compose.yml to make the test work.

Another concern is consul[1] is licensed under MPL 2.0 which belongs to category B[2] in ASF. So we cannot accept this change into our codebase according to the ASF policy. cc @apache/kvrocks-committers

[1] https://github.com/hashicorp/consul?tab=License-1-ov-file#readme
[2] https://www.apache.org/legal/resolved.html#weak-copyleft-licenses

[aleksraiden]

Awesome, we use kvrocks at consul+nomad cluster and very need this feature

[borismartinovic01]

Okay, I wasn't aware of the license issues.
I updated PR per comments so it can stay as PoC.

[PragmaTwice]

Yeah it seems https://github.com/hashicorp/consul is licensed under BSL, which is not an open-source license and not allowed by ASF to be used as a direct dependency of an ASF project. (Ahh sorry, the api directory is licensed under MPL, not BSL.)

However, I think there may be several ways to workaround it (one of these two below):

• Replace github.com/hashicorp/consul/api with another project which is under a compatible license. (maybe you need to check the license of other newly-introduced projects, too), or implement a simple client by our own. (I'm not familiar with this project so I don't know how much work it will be. From its docs here, it should not be impossible since it's HTTP-based. Note that we just need the consul client to be license-compatible. )
• Make this feature totally optional, and disable by default. In this way, it might be compliant, from the perspective of the ASF view. Note that, it means that the official release should not include this feature, and all code of these dependencies should not be compiled into the released binary.

I'm not an expert in this area. See if @tisonkun has any comment.

[PragmaTwice]

Sorry for clicking wrong button : )

[PragmaTwice]

After carefully reading the content of Category B, it appears that the main restriction is that our source releases should not include MPL source files. As kvrocks-controller is a golang project, we typically do not include these dependent source files in the source release, but instead we only specify the version and origin of dependencies through go.mod/go.sum (and users will download these sources by their own through make/go build/run). From this perspective, using these MPL projects as dependencies seems to be compliant. cc @tisonkun @git-hulk

[PragmaTwice]

Hmm.. I think we can continue to review this PR, and merge it after approving by reviewers. We'll handle further issues if there's any concern from the ASF.

@borismartinovic01 Thank you for your contribution, and I think for now you don't need to remove these MPL dependencies. But you can review these newly introduced dependencies again to prevent other issues.

[PragmaTwice]

It shouldn't be an "indirect" dependency.

[PragmaTwice]

Could you also add an example config file in https://github.com/apache/kvrocks-controller/tree/unstable/config?

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 60.65574% with 72 lines in your changes missing coverage. Please review.

Project coverage is 46.77%. Comparing base (6c56470) to head (67e3bd3).
Report is 20 commits behind head on unstable.

Files with missing lines	Patch %	Lines
store/engine/consul/consul.go	61.66%	57 Missing and 12 partials ⚠️
server/server.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #240      +/-   ##
============================================
+ Coverage     43.38%   46.77%   +3.39%     
============================================
  Files            37       43       +6     
  Lines          2971     3878     +907     
============================================
+ Hits           1289     1814     +525     
- Misses         1544     1873     +329     
- Partials        138      191      +53     

Flag	Coverage Δ
unittests	46.77% <60.65%> (+3.39%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[PragmaTwice]

PTAL @git-hulk

[git-hulk]

LGTM

[borismartinovic01]

Raft tests failed. This is not very clear to me. Do you have any information on why this is happening?

[git-hulk]

@borismartinovic01 No worry about this. I have rerun the CI since it's not related to this PR: #245

[PragmaTwice]

Thank you for your contribution!