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

Add timeout support for vault client #21

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

luanphantiki
Copy link
Contributor

@luanphantiki luanphantiki commented Oct 26, 2021

@luanphantiki
Copy link
Contributor Author

@Lucretius Please review it, thanks!

@luanphantiki
Copy link
Contributor Author

@devops-42 , please consider to test this feature, it's not harmful for your production, but you should test it at UAT if possible.

@devops-42
Copy link

@luanphantiki

unfortunately it seems not to work. Steps which I've done:

  • cloned your repo
  • switched into branch snapshot-timeout-support
  • built the new Docker image using docker build -t imagename .
  • pushed the new image into our registry
  • added timeout setting in config file:
{
   "addr":"http://vault-leader:8200",
   "retain":72,
   "frequency":"3600s",
   "timeout":"300s",
   "role_id": "***",
   "secret_id":"***",
   "aws_storage":{
      "access_key_id":"***",
      "secret_access_key":"***",
      "s3_region":"us-east-1",
      "s3_bucket":"***,
      "s3_endpoint":***",
      "s3_force_path_style":true
   }
}
  • performed pod re-deployment

The pod still exits after 60 secs with the well-known error:

2021/10/27 12:35:29 Reading configuration...
2021/10/27 12:36:30 Unable to generate snapshot context deadline exceeded (Client.Timeout or context cancellation while reading body)

I already logged into the pod and can confirm, that the timeout value is present in the mounted config file. I also grepped against the binary of the vault_raft_snapshot_agent to check whether the right image has been pull from our registry:

grep timeout vault_raft_snapshot_agent 
[...]
json:"timeout,ommitempty"
[...]

Maybe I missed something?

@luanphantiki
Copy link
Contributor Author

@devops-42 : there was a typo on my code, fixed, can you please pull the latest and try again ?

@devops-42
Copy link

@luanphantiki

Tested again. The log file now shows that the timeout parameter could parsed correctly, but the task still aborts after 60s:

2021/10/28 08:13:05 Reading configuration...
2021/10/28 08:13:05 Vault client timeout has been set to 5m0s
2021/10/28 08:14:05 Unable to generate snapshot context deadline exceeded (Client.Timeout or context cancellation while reading body)

@luanphantiki luanphantiki changed the title add timeout support for vault client WIP: add timeout support for vault client Oct 28, 2021
@luanphantiki
Copy link
Contributor Author

@devops-42 thanks, I might be missed somethings, will update it soon.

@luanphantiki
Copy link
Contributor Author

@devops-42: please test the latest commit once have a chance, thanks!

@devops-42
Copy link

Hey @luanphantiki

finally :) Thumbs up, good work!

2021/10/29 13:58:25 Reading configuration...
2021/10/29 13:58:25 Vault http client timeout has been set to 5m0s
2021/10/29 14:01:43 Successfully created aws snapshot to ....

Thanks

@luanphantiki luanphantiki changed the title WIP: add timeout support for vault client Add timeout support for vault client Oct 29, 2021
@luanphantiki
Copy link
Contributor Author

Alright, thanks for the confirmation @devops-42 ;)

@Lucretius The new feature seems to work, please review it, thanks!

@kirrmann
Copy link
Contributor

kirrmann commented Nov 8, 2022

Can we merge this is? For me snapshot creation didn't work at all. Sometimes got infamous context deadline error, but other times the snapshots were created with only 0 bytes. I just tried this PR to increase the timeout and it does work for me.

Argelbargel added a commit to Argelbargel/vault-raft-snapshot-agent that referenced this pull request Sep 14, 2023
Add Timeout for Vault-Connections (based on Lucretius#21)
@druesendieb
Copy link

@Lucretius The repository is stale since 2021 - are you planning on updating or enhancing it with PR's like this at all?

@Argelbargel
Copy link

HI @druesendieb: see #28 (comment)

This repo is not actively maintained any more. There are some forks with added features including the timeout for vault client (e.g. https://github.com/Boostport/vault_raft_snapshot_agent). I've created a fork too (https://github.com/Argelbargel/vault-raft-snapshot-agent) which includes most of the open prs and some further enhancements. Perhaps one of those might work for you?

@Lucretius
Copy link
Owner

Hello,

No, I built this years ago when I was working at a place that used Vault, which I no longer do. There are others who have forked this repo and have enhanced it. Feel free to do so as well. I will update this repo to reflect its status.

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.

6 participants