LogDog: Implement prefix expiration.

LogDog: Implement prefix expiration.

Implement prefix space expiration. Previously, this field was present in
the protobuf, but not implemented.

This enforces that new stream creation will fail a predefined period of
time after a prefix space has been created. This value is the minimum of
the value specified in the project config, service config, and prefix
registration.

BUG=

Committed: https://github.com/luci/luci-go/commit/b168ce94eae6864e845a5bb71b4ac7718cd8cd79

[dnj (Google), 2016-05-11 22:52:09]

dnj@google.com changed reviewers:
+ dnj@google.com, hinoka@chromium.org, nodir@chromium.org

[dnj (Google), 2016-05-11 22:52:09]

PTAL

[dnj, 2016-05-17 20:40:05]

Description was changed from

==========
LogDog: Implement prefix expiration.

BUG=
==========

to

==========
LogDog: Implement prefix expiration.

Implement prefix space expiration. Previously, this field was present in
the protobuf, but not implemented.

This enforces that new stream creation will fail a predefined period of
time after a prefix space has been created. This value is the minimum of
the value specified in the project config, service config, and prefix
registration.

BUG=
==========

[nodir, 2016-05-19 00:54:00]

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/registerStream.go (right):

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/registerStream.go:123: if now :=
clock.Now(c); !now.Before(pfx.Expiration) {
I am not sure how you do the migration (CL description does not address that),
but it may make sense to skip this check if pfx.Expiration.IsZero()

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/registerStream.go:131: // The
prefix secret much match the request secret. If it does, we know this
must

[dnj (Google), 2016-05-19 02:06:19]

Added a migration strategy, should work!

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/registerStream.go (right):

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/registerStream.go:123: if now :=
clock.Now(c); !now.Before(pfx.Expiration) {
On 2016/05/19 00:54:00, nodir wrote:
> I am not sure how you do the migration (CL description does not address that),
> but it may make sense to skip this check if pfx.Expiration.IsZero()

Hmm good question. I have an idea: Since we're defining an expiration, I'll say
that if the Prefix expiration is zero, I'll add the smallest expiration
parameter to the Prefix's creation time and use that.

This will be okay as long as I make sure the config has a non-zero value for
that before I enable this, else all existing Prefixes will suddenly not work
anymore :)

[nodir, 2016-05-19 03:06:33]

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/registerStream.go (right):

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/registerStream.go:123: if now :=
clock.Now(c); !now.Before(pfx.Expiration) {
On 2016/05/19 02:06:19, dnj (Google) wrote:
> On 2016/05/19 00:54:00, nodir wrote:
> > I am not sure how you do the migration (CL description does not address
that),
> > but it may make sense to skip this check if pfx.Expiration.IsZero()
> 
> Hmm good question. I have an idea: Since we're defining an expiration, I'll
say
> that if the Prefix expiration is zero, I'll add the smallest expiration
> parameter to the Prefix's creation time and use that.
> 
> This will be okay as long as I make sure the config has a non-zero value for
> that before I enable this, else all existing Prefixes will suddenly not work
> anymore :)

Yeah, how you gonna fetch new configs without deploying new proto files? I'd be
on the safe side by having a default prefix expiration if service config doesn't
have it

[dnj (Google), 2016-05-19 03:20:07]

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
File appengine/logdog/coordinator/endpoints/services/registerStream.go (right):

https://codereview.chromium.org/1972753002/diff/20001/appengine/logdog/coordi...
appengine/logdog/coordinator/endpoints/services/registerStream.go:123: if now :=
clock.Now(c); !now.Before(pfx.Expiration) {
On 2016/05/19 03:06:33, nodir wrote:
> On 2016/05/19 02:06:19, dnj (Google) wrote:
> > On 2016/05/19 00:54:00, nodir wrote:
> > > I am not sure how you do the migration (CL description does not address
> that),
> > > but it may make sense to skip this check if pfx.Expiration.IsZero()
> > 
> > Hmm good question. I have an idea: Since we're defining an expiration, I'll
> say
> > that if the Prefix expiration is zero, I'll add the smallest expiration
> > parameter to the Prefix's creation time and use that.
> > 
> > This will be okay as long as I make sure the config has a non-zero value for
> > that before I enable this, else all existing Prefixes will suddenly not work
> > anymore :)
> 
> Yeah, how you gonna fetch new configs without deploying new proto files? I'd
be
> on the safe side by having a default prefix expiration if service config
doesn't
> have it

Oh that part's easy. I'll upload the new version, shut down the Collector fleet,
deploy the config, wait for luci-config to pick it up, and then hit the "Migrate
Traffic" button and bring the fleet back up. I'll have to do that anyway on
account of the other changes.

[nodir, 2016-05-19 05:29:06]

OK lgtm

[dnj (Google), 2016-05-26 17:04:13]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-05-26 17:04:14]

The patchset sent to the CQ was uploaded after l-g-t-m from nodir@chromium.org
Link to the patchset: https://codereview.chromium.org/1972753002/#ps80001
(title: "Updated patchset dependency")

[commit-bot: I haz the power, 2016-05-26 17:04:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1972753002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1972753002/80001

[commit-bot: I haz the power, 2016-05-26 17:12:54]

Description was changed from

==========
LogDog: Implement prefix expiration.

Implement prefix space expiration. Previously, this field was present in
the protobuf, but not implemented.

This enforces that new stream creation will fail a predefined period of
time after a prefix space has been created. This value is the minimum of
the value specified in the project config, service config, and prefix
registration.

BUG=
==========

to

==========
LogDog: Implement prefix expiration.

Implement prefix space expiration. Previously, this field was present in
the protobuf, but not implemented.

This enforces that new stream creation will fail a predefined period of
time after a prefix space has been created. This value is the minimum of
the value specified in the project config, service config, and prefix
registration.

BUG=

Committed:
https://github.com/luci/luci-go/commit/b168ce94eae6864e845a5bb71b4ac7718cd8cd79
==========

[commit-bot: I haz the power, 2016-05-26 17:12:56]

Committed patchset #5 (id:80001) as
https://github.com/luci/luci-go/commit/b168ce94eae6864e845a5bb71b4ac7718cd8cd79