Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow what documentation says we should do we're a perf standby and perfstandbyok=true #7241

Merged
merged 3 commits into from
Aug 5, 2019
Merged

Follow what documentation says we should do we're a perf standby and perfstandbyok=true #7241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c43821c
Follow what documentation says we should do we're a perf standby and
ncabatoff Aug 1, 2019
155d52d
Rework switch statement for readability.
ncabatoff Aug 5, 2019
cfcc27e
Fix typos in docs.
ncabatoff Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions http/sys_health.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,14 @@ func getSysHealth(core *vault.Core, r *http.Request) (int, *HealthResponse, erro
code = sealedCode
case replicationState.HasState(consts.ReplicationDRSecondary):
code = drSecondaryCode
case !perfStandbyOK && perfStandby:
code = perfStandbyCode
case !standbyOK && standby:
code = standbyCode
case perfStandby:
if !perfStandbyOK {
code = perfStandbyCode
}
case standby:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we want to still allow standbyOK to return 200 on performance standbys? It may be unexpected to stop returning 200 if perf standbys are turned on. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're talking about where a customer converts standbys to perf standbys or vice versa? I mean, I don't entirely understand why we have two different params (standbyOK and perfStandbyOK), but given that we do, if I were setting up monitoring of my vault instances and wanted this feature I'd simply provide both params.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncabatoff I talked to BK and we're in agreement on this change -- one thing I noticed is that the website docs have some errors around standbyok and perfstandbyok ("load balance" for instance) -- can you give those a once-through as well? After that I think merge this and cherry-pick it back.

if !standbyOK {
code = standbyCode
}
}

// Fetch the local cluster name and identifier
Expand Down
4 changes: 2 additions & 2 deletions website/source/api/system/health.html.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ The default status codes are:

- `standbyok` `(bool: false)` – Specifies if being a standby should still return
the active status code instead of the standby status code. This is useful when
Vault is behind a non-configurable load balance that just wants a 200-level
Vault is behind a non-configurable load balancer that just wants a 200-level
response. This will not apply if the node is a performance standby.

- `perfstandbyok` `(bool: false)` – Specifies if being a performance standby should
still return the active status code instead of the performance standby status code.
This is useful when Vault is behind a non-configurable load balance that just wants
This is useful when Vault is behind a non-configurable load balancer that just wants
a 200-level response.

- `activecode` `(int: 200)` – Specifies the status code that should be returned
Expand Down