Kine does not handle transactions with `mod(key) = "0"` properly

[brandond]

Transactions with a comparison operation of mod("/x") = "0" should not result in an error when the target key already exists. If the key exists and the create fails, the ELSE case of the comparison should execute.

For example, in the second transaction in the following sequence of operations, kine should execute the ELSE operation and return the current value, instead of returning the conflict from the insert.

brandond@dev01:~$ etcdctl --endpoints=http://172.17.0.2:2379 txn <<EOF
mod("/x") = "0"

put /x "Hello, world"

get /x

EOF
SUCCESS

OK
brandond@dev01:~$ etcdctl --endpoints=http://172.17.0.2:2379 txn <<EOF
mod("/x") = "0"

put /x "Hello, world"

get /x

EOF
{"level":"warn","ts":"2023-10-26T21:13:49.136Z","caller":"clientv3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"endpoint://client-38e80423-8e2f-40e5-aec6-d8d6afe62fe8/172.17.0.2:2379","attempt":0,"error":"rpc error: code = InvalidArgument desc = etcdserver: duplicate key given in txn request"}
Error: etcdserver: duplicate key given in txn request

Kine does the correct thing if you specify a mod revision other than 0:

brandond@dev01:~$ etcdctl --endpoints=http://172.17.0.2:2379 txn <<EOF
mod("/x") = "1"

put /x "Hello, world"

get /x

EOF
FAILURE

/x
Hello, world

Compare that to the correct behavior of etcd, which executes the ELSE case :

brandond@dev01:~$ etcdctl --endpoints=http://172.17.0.2:2379 txn <<EOF
mod("/x") = "0"

put /x "Hello, world"

get /x

EOF
SUCCESS

OK
brandond@dev01:~$ etcdctl --endpoints=http://172.17.0.2:2379 txn <<EOF
mod("/x") = "0"

put /x "Hello, world"

get /x

EOF
FAILURE

/x
Hello, world
brandond@dev01:~$

[brandond]

I suspect this may have been a shortcut on the kine side, as mod(key) = "0" usually indicates that the server expects the resource to not exist and wants to create it, and returning a conflict error is handled correctly by Kubernetes.

However, returning an error instead of executing the ELSE puts extra work on the client as it now has to explicitly GET the key instead of getting it in the the txn response - and is also technically incorrect from a compatibility standpoint.

[brandond]

Apparently Kubernetes CREATE txn does not put any statement in the ELSE, so this probably isn't necessary to get right to work properly with Kubernetes.

[brandond]

Found a single use of an update with mod(key) = "0" transaction in Kubernetes, in GuaranteedUpdate with an empty initial state:

https://github.com/kubernetes/kubernetes/blob/v1.28.3/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go#L514-L520

[bb010g]

Was this resolved by #229?

[brandond]

Yes should be.