Simplify a lot of the mount tuning code #3285
Conversation
Make leases much simpler by not trying to compare against system default/max -- a bit of a fool's errand since a change to config files can flip whether it's even valid. For extra credit, add the ability to tune description; closes #2645
vault/logical_system.go
Outdated
| b.Backend.Logger().Error("sys: tune failed: no mount entry found", "path", path) | ||
| return handleError(fmt.Errorf("sys: tune of path '%s' failed: no mount entry found", path)) | ||
| } | ||
| if mountEntry != nil && !mountEntry.Local && repState == consts.ReplicationSecondary { |
There was a problem hiding this comment.
This should use the HasState function:
repState.HasState(consts.ReplicationPerformanceSecondary)
There was a problem hiding this comment.
It's a copy and paste from above, which means it got missed before too :-/
There was a problem hiding this comment.
It was changed in my locking PR, so i think it has since been updated.
vault/logical_system.go
Outdated
| @@ -1586,7 +1606,31 @@ func (b *SystemBackend) handleTuneWriteCommon( | |||
| } | |||
| } | |||
|
|
|||
| return nil, nil | |||
| var resp *logical.Response | |||
There was a problem hiding this comment.
This doesn't seem to be used anywhere, should we just return nil, nil below?
There was a problem hiding this comment.
Ah, it was used for a warning before but that code is now removed, will fix it.
| return fmt.Errorf("new backend default lease TTL of %d greater than backend max lease TTL of %d", | ||
| int(newDefault.Seconds()), int(meConfig.MaxLeaseTTL.Seconds())) | ||
| } | ||
| case newDefault != zero && newMax != zero: |
There was a problem hiding this comment.
This switch statement might be cleaner if it only had this case and a default case. Unless you like the verbosity of the other statements + comments.
There was a problem hiding this comment.
In this instance I found it useful to think through the different cases and leave appropriate comments.
calvn
changed the title
|
LGTM aside from the comments that Brian left. |
|
Travis failure is from intermittent Cassandra failure; merging. |
Make leases much simpler by not trying to compare against system
default/max -- a bit of a fool's errand since a change to config files
can flip whether it's even valid.
For extra credit, add the ability to tune description; closes #2645