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.

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

Enable persistent agent caching #229

Merged
merged 4 commits into from
Mar 16, 2021
Merged

Conversation

tvoran
Copy link
Member

@tvoran tvoran commented Mar 10, 2021

Adds support for persistent agent caching introduced in hashicorp/vault#10938

Controlled by the "vault.hashicorp.com/agent-cache-enable" annotation, with a few caveats:

If "vault.hashicorp.com/agent-cache-enable": "true",

  • ...and if init and sidecar enabled, persistent caching enabled
  • ...and if sidecar only, just memory caching without persistence enabled
  • ...and if init only, no caching at all

When persistent caching is enabled, a vault-agent-cache memory volume is mounted at /vault/agent-cache on the init and sidecar containers.

Also adds an annotation "vault.hashicorp.com/agent-cache-exit-on-err", which corresponds to the exit_on_err config setting introduced in hashicorp/vault#10938. Controls whether the agent will exit on an error while restoring the persistent cache.

Copy link
Contributor

@calvn calvn left a comment

Choose a reason for hiding this comment

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

Not super familiar with the codebase yet, but looks good! Left some smaller suggestions.

Comment on lines 355 to 357
ExitOnErr: agentCacheExitOnErr,
}
agent.VaultAgentCache.Persist = agent.agentCachePersist()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ExitOnErr: agentCacheExitOnErr,
}
agent.VaultAgentCache.Persist = agent.agentCachePersist()
ExitOnErr: agentCacheExitOnErr,
Persist: agent.agentCachePersist(),
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I reworked this a bit in 00f9152 to set it all in the struct, since agentCachePersist() also checks whether the cache is enabled.

@@ -550,6 +558,22 @@ func (a *Agent) agentCacheEnable() (bool, error) {
return strconv.ParseBool(raw)
}

func (a *Agent) agentCachePersist() bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the caller is *Agent in here so you could rename this to cachePersist (same with the func below).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, done in 00f9152

@@ -550,6 +558,22 @@ func (a *Agent) agentCacheEnable() (bool, error) {
return strconv.ParseBool(raw)
}

func (a *Agent) agentCachePersist() bool {
if a.VaultAgentCache.Enable && a.PrePopulate && !a.PrePopulateOnly {
Copy link
Contributor

Choose a reason for hiding this comment

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

Determining whether we need persist like so may be fine for now, though I do wonder if we'd have to expose this through annotations down the road.

@@ -306,8 +308,8 @@ func TestConfigVaultAgentCache(t *testing.T) {
t.Error("agent Cache should be enabled")
}

if config.Cache.UseAuthAuthToken != "force" {
t.Errorf("agent Cache use_auto_auth_token should be 'force', got %s instead", config.Cache.UseAuthAuthToken)
if config.Cache.UseAutoAuthToken != "force" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch.

Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

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

LGTM!

Removing some of the stutter from agent functions, and setting Persist
directly.
@tvoran
Copy link
Member Author

tvoran commented Mar 15, 2021

FWIW, I also tested these changes with the current version of vault-agent (1.6.3), and the extra persist cache config is ignored without errors.

So for someone using a pre-1.7 vault-agent, the main difference is the init container's vault-agent will also have caching enabled (as opposed to just the sidecar), but pre-1.7 this is just memory cache, so it shouldn't change behavior for users.

@tvoran tvoran merged commit 3f9800c into master Mar 16, 2021
@tvoran tvoran deleted the VAULT-1096/enable-persistent-caching branch March 16, 2021 06:02
RemcoBuddelmeijer pushed a commit to RemcoBuddelmeijer/vault-k8s that referenced this pull request Feb 22, 2022
Controlled by the "vault.hashicorp.com/agent-cache-enable" annotation,
with a few caveats:

If "vault.hashicorp.com/agent-cache-enable": "true",

...and if init and sidecar enabled, persistent caching enabled
...and if sidecar only, just memory caching without persistence enabled
...and if init only, no caching at all

When persistent caching is enabled, a `vault-agent-cache` memory volume
is mounted at `/vault/agent-cache` on the init and sidecar containers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants