See kontena/kontena#1677 agent/lib/etcd/health.rb : https://github.com/kontena/kontena/blob/etcd-health/agent/lib/etcd/health.rb

All of the Kontena::Etcd::Model operations currently use a global Kontena::Etcd::Model.etcd client, which must be initialized by the app/tests using a Kontena::Etcd::Model.etcd = Kontena::Etcd::Client.* boilerplate.

OTOH the Kontena::Etcd::Reader/Writer classes are hardcoded to use a new Kontena::Etcd::Client.from_env for each instance.

Using a Kontena::Etcd::Client.from_env client isn't necessarily a bad default, and sharing it at the class level shouldn't be a (threading) problem either, but:

• This could just implicitly default to Kontena::Etcd::Client.from_env, as the Kontena::Etcd::Reader/Writer classes do, avoiding the requirement for app-level boilerplate

• The client used by the model, reader and writer classes should be configurable by the application

  For the Kontena::Etcd::Model class, there could be some kind of Scope that contains the etcd client used for operations? This could also be relevant if implementing any relational operations...

The Kontena::Etcd::Test::TestServer requires a single prefix for all keys used in the test. This is so that reset! can use a recursive delete, and etcd does not allow deleting /.

This prefix is currently hardcoded to be /kontena, but it should be configurable.

The prefix is used for nodes and logs, but modified? triggers on all etcd writes, including those outside of the root.

Testing the behaviour of things like Kontena::Etcd::Writer which use etcd TTLs needs support for faking TTL expirty. For example, refresh should raise an exception if any nodes have expired before the refresh cycle kicks in.

Implementing this in the tests against a real ETCD_ENDPOINT=... would require support from the etcd server... however, we could feasibly implemenet support for faking etcd TTL expiry in the Kontena::Etcd::Test::FakeServer.

The Kontena::Etcd::Client could support a per-application etcd key prefix like ETCD_ENDPOINT=http://localhost:2379/kontena/test.

The application would then use relative keys paths like etcd.set('foo/bar') or Kontena::Etcd::Model#etcd_path 'foo/:bar', which would be translated to /kontena/test/foo/....

This would allow easily running multiple copies of the same application on the same etcd cluster if desired.

Having such a configurable key prefix would also fix #4 for the etcd key path root used for specs.

The Kontena::JSON::Model and Kontena::Etcd::Model should be decoupled, where different etcd models can use different kinds of value encodings. Currently the model classes must include both Kontena::JSON::Model as well as Kontena::Etcd::Model.

There should be a clean split between the etcd node path identity provided by the etcd model (the *args positional arguments) and the etcd node value provided by the JSON model (the **attrs keyword arguments).

Kontena::Etcd::Model

Current coupling points:

• initialize(*key, **attrs) -> self.initialize_json(**attrs)
• <=> -> self.cmp_json
• load!(node) -> self.from_json(node.value)
• create!, update!, value: self.to_json

The kontena-etcd Gemfile marks the gems used by Kontena::Etcd::Test and Kontena::Etcd::Rspec as development/test dependencies.

However, other projects depending on kontena-etcd also need those gems if using Kontena::Etcd::Rspec for their tests. Currently that means copy-pasting those transitive development/testing deps into depending projects.

Is it possible to have transitive development/testing dependencies for bundler/ruby gems?

The README should document the use of the per-model Invalid, NotFound, Conflict exceptions.

Currently the Kontena::Etcd::Writer#update implementation will always mark a node as shared if updating an existing node with a new value, even if the prev_node is our own exclusive node.

Trying to rspec with expect(etcd_server.logs).to eq [...] after a refresh operation fails with a timeout:

I, [2017-01-04T13:26:12.771708 #7660]  INFO -- Kontena::Etcd::Writer: set /kontena/test1: {"test":1}
D, [2017-01-04T13:26:12.776202 #7660] DEBUG -- Kontena::Etcd::Client: set /kontena/test1 {:ttl=>30, :value=>"{\"test\":1}"}: set /kontena/test1@1520: {"test":1}
D, [2017-01-04T13:26:12.782380 #7660] DEBUG -- Kontena::Etcd::Client: refresh /kontena/test1 {:refresh=>true, :ttl=>30, :prevExist=>true, :prevValue=>"{\"test\":1}"}: compareAndSwap /kontena/test1@1521: {"test":1}
D, [2017-01-04T13:26:12.784441 #7660] DEBUG -- Kontena::Etcd::Client: get /kontena {}: get /kontena/@1520: test1
D, [2017-01-04T13:26:12.786881 #7660] DEBUG -- Kontena::Etcd::Client: watch /kontena {:wait=>true, :recursive=>true, :waitIndex=>1520, :timeout=>1.0}: set /kontena/test1@1520: {"test":1}
F

Failures:

  1) Kontena::Etcd::Writer for etcd with one node set #refresh updates the node
     Failure/Error:
       return @connection.request(method: method, path: path, query: query,
         headers: headers,
         body: body,
         expects: expects,
       )
     
     Timeout::Error:
       execution expired
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/socket.rb:263:in `select'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/socket.rb:263:in `select_with_timeout'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/socket.rb:49:in `rescue in readline'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/socket.rb:45:in `readline'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/response.rb:128:in `parse'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/response_parser.rb:7:in `response_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/connection.rb:388:in `response'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/connection.rb:252:in `request'
     # ./vendor/bundle/ruby/2.3.0/gems/webmock-2.3.1/lib/webmock/http_lib_adapters/excon_adapter.rb:62:in `handle_request'
     # ./vendor/bundle/ruby/2.3.0/gems/webmock-2.3.1/lib/webmock/http_lib_adapters/excon_adapter.rb:40:in `block in add_excon_stub'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/mock.rb:30:in `request_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/instrumentor.rb:26:in `request_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/base.rb:16:in `request_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/base.rb:16:in `request_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/middlewares/base.rb:16:in `request_call'
     # ./vendor/bundle/ruby/2.3.0/gems/excon-0.54.0/lib/excon/connection.rb:249:in `request'
     # ./lib/kontena/etcd/client.rb:52:in `http_request'
     # ./lib/kontena/etcd/keys.rb:52:in `keys_request'
     # ./lib/kontena/etcd/keys.rb:137:in `watch'
     # ./lib/kontena/etcd/test/test_server.rb:44:in `get_logs'
     # ./lib/kontena/etcd/test/test_server.rb:122:in `logs'
     # ./spec/kontena/etcd/writer_spec.rb:109:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:61:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:60:in `block (2 levels) in <top (required)>'

This is because the Kontena::Etcd::Test::TestServer expects to get a watch event for the current X-Etcd-Index, but because this action was a refresh, etcd does not return it from GET ...?wait=true.

The test should touch the root directory to ensure that it gets the final watch even for the resulting etcd index.