On master, key doesn't be deleted when bound lease expires

[zyf0330]

step:

• etcdctl lease grant 30
• etcdctl put a 123 --lease leaseid
• etcdctl lease timetolive leaseid and got lease 4289604df903d409 granted with TTL(30s), remaining(-4s)

Lease expired but it and key still exists. Correct output of expired lease should be lease 4289604df903d409 granted with TTL(0s), remaining(-1s)

[hexfusion]

Lease expired but it and key still exists. Correct output of expired lease should be lease 4289604df903d409 granted with TTL(0s), remaining(-1s)

Hi @zyf0330 I can not replicate this can you post logs? What version of go are you using and what are steps to compile? Thanks!

[samb@ob1 etcd]$ bin/etcd --version
etcd Version: 3.2.0+git
Git SHA: c886bda7f
Go Version: go1.9
Go OS/Arch: linux/amd64

$ printenv
ETCDCTL_API=3
...

$ etcdctl lease grant 30
lease 694d604f1cc13e0f granted with TTL(30s)

$ etcdctl put a 123 --lease 694d604f1cc13e0f
OK

$ etcdctl lease timetolive  694d604f1cc13e0f
lease 694d604f1cc13e0f granted with TTL(30s), remaining(10s)

$ etcdctl lease timetolive  694d604f1cc13e0f
lease 694d604f1cc13e0f granted with TTL(0s), remaining(-1s)

[zyf0330]

go version 1.9.2
etcd commit c886bda

It seems that only go version is different. But I have problem still.

[zyf0330]

I have test that at release v3.2.11, this problem doesn't exist.

[hexfusion]

@zyf0330 I just built 1.9.2 and tested against master. Can you confirm your build process?

$ ./build
$ rm -rf default.etcd/
$ bin/etcd&

[zyf0330]

You are right, I test again with deleting old data and no problem.
I use old etcd data dir, so this problem occurs.
So how to solve it?

[xiang90]

We need a way to reproduce this problem.

I am not sure what is the old data. If you can reproduce it with old data, you probably need to upload the old data somewhere, and tell us the steps to reproduce the problem.

[zyf0330]

So do you need data dir or real k/v?

[xiang90]

We need a way to reproduce it. You should write down the exact steps to reproduce the problem. If any data is needed to reproduce it, it should be uploaded somewhere.

[zyf0330]

I found it. This is reproduce step.

1. use c886bda and build
2. add user root:root and enable auth
3. grant lease and put k v --lease leaseid
4. wait to see it

[xiang90]

@gyuho @hexfusion

are you able to reproduce this? it seems like a bug.

[gyuho]

@xiang90 Yeah, easily reproducible. Will fix.

ETCDCTL_API=3 ./bin/etcdctl \
  put abc def

ETCDCTL_API=3 ./bin/etcdctl \
  get abc

ETCDCTL_API=3 ./bin/etcdctl \
  role add root

ETCDCTL_API=3 ./bin/etcdctl \
  role grant-permission root readwrite foo

ETCDCTL_API=3 ./bin/etcdctl \
  role get root

ETCDCTL_API=3 ./bin/etcdctl \
  --interactive=false \
  user add root:123

ETCDCTL_API=3 ./bin/etcdctl \
  user grant-role root root

ETCDCTL_API=3 ./bin/etcdctl \
  user get root

ETCDCTL_API=3 ./bin/etcdctl \
  auth enable

ETCDCTL_API=3 ./bin/etcdctl \
  --user=root:123 \
  put foo bar

ETCDCTL_API=3 ./bin/etcdctl \
  --user=root:123 \
  lease grant 30

ETCDCTL_API=3 ./bin/etcdctl \
  --user=root:123 \
  put foo bar --lease 694d60581f75b714

ETCDCTL_API=3 ./bin/etcdctl \
  --user=root:123 \
  lease timetolive 694d60581f75b714

ETCDCTL_API=3 ./bin/etcdctl \
  --user=root:123 \
  get foo

[gyuho]

Also the lease timetolive output is broken, when auth is enabled.

ETCDCTL_API=3 ./bin/etcdctl lease timetolive 694d60581f75b714
lease 694d60581f75b714 granted with TTL(30s), remaining(-4s)

Should be

lease 694d60581f75b714 granted with TTL(0s), remaining(-4s)

[hexfusion]

@xiang90 added a test for via #9017 but it was passing for me.

[gyuho]

Server-side bug when it upgraded gRPC to v1.4.x (#7896).
Affects etcd v3.2.10+ with gRPC v1.7.x.

I have a fix #9018; we need another v3.2 release.

[zyf0330]

About format mentioned in comment, should it keep being lease 694d60581f75b714 granted with TTL(0s), remaining(-1s) after expiring?

[gyuho]

@zyf0330 The output should be fixed as well.

lease 694d60581f75b714 granted with TTL(0s), remaining(-4s)

is the correct format (not -1s).

[xiang90]

Why the remaining can be negative?

…

On Thu, Dec 14, 2017 at 10:44 PM Gyuho Lee ***@***.***> wrote: @zyf0330 <https://github.com/zyf0330> Yes, the output should be fixed as well. lease 694d60581f75b714 granted with TTL(0s), remaining(-4s) is the correct format. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9006 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AERby1VPy5eKZ0D6TukIM_aTQEcELw-Eks5tAhVpgaJpZM4Q_-b-> .

[gyuho]

@xiang90 This is fixed via #9018. Also the output, we return TTL -1 on lease not found. So lease 694d60581f75b714 granted with TTL(0s), remaining(-1s) is the correct output.

[xiang90]

Is it better to explicitly return lease not found? -1 still seems not intuitive.

…

On Fri, Dec 15, 2017 at 8:57 AM Gyuho Lee ***@***.***> wrote: @xiang90 <https://github.com/xiang90> This is fixed via #9018 <#9018>. Also the output, we return TTL -1 on lease not found. So lease 694d60581f75b714 granted with TTL(0s), remaining(-1s) is the correct output. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9006 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AERby_V-_dT4rPxtjYolbnPzha7PtF0Kks5tAqT4gaJpZM4Q_-b-> .

[gyuho]

@xiang90 The change was made from 3.1 to 3.2. https://github.com/coreos/etcd/blob/master/Documentation/upgrades/upgrade_3_2.md#client-upgrade-checklists-320

Also, just found out this only happens in master since we didn't backport #8031.

No need for another v3.2.12 release.

[xiang90]

Do we have a test written to avoid the regression? If yes, we probably should backport the test regardless.

…

On Fri, Dec 15, 2017 at 9:09 AM Gyuho Lee ***@***.***> wrote: @xiang90 <https://github.com/xiang90> The change was made from 3.1 to 3.2. https://github.com/coreos/etcd/blob/master/Documentation/upgrades/upgrade_3_2.md#client-upgrade-checklists-320 Also, just found out this only happens in master since we didn't backport #8031 <#8031>. No need for another v3.2.12 release. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9006 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AERby5Fou5wrdNWV3ZIa8uMS1QZAt8Laks5tAqfmgaJpZM4Q_-b-> .

[gyuho]

Just backported 9fb7bbd to release-3.2 branch.