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

feat: sleep at renewal #1657

Merged
merged 4 commits into from
Jun 15, 2022
Merged

feat: sleep at renewal #1657

merged 4 commits into from
Jun 15, 2022

Conversation

ldez
Copy link
Member

@ldez ldez commented Jun 15, 2022

Add a sleep between 0 to 8 minutes during the call of the renew command like certbot.

Fixes #1656

@ldez ldez added this to the v4.8 milestone Jun 15, 2022
@ldez ldez requested a review from dmke June 15, 2022 01:38
Copy link
Member

@dmke dmke left a comment

Choose a reason for hiding this comment

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

I've found some defects :)

cmd/cmd_renew.go Outdated Show resolved Hide resolved
cmd/cmd_renew.go Outdated Show resolved Hide resolved
ldez and others added 2 commits June 15, 2022 08:52
Co-authored-by: Dominik Menke <git@dmke.org>
@ldez ldez enabled auto-merge (squash) June 15, 2022 07:01
@ldez ldez merged commit 257dfa7 into go-acme:master Jun 15, 2022
@ldez ldez deleted the feat/sleep-at-renewal branch June 15, 2022 07:08
@dmke
Copy link
Member

dmke commented Jun 17, 2022

@ldez, I have another remark:

@jsha asked for a randomized sleep for non-interactive renewal. The current implementation however requires an extra flag to skip it. Users might be surprised when they run renew manually and don't know of that flag.

Wouldn't it be better, if we (additionally) check (github.com/mattn/isatty).IsTerminal, i.e.

if !isatty.IsTerminal(os.Stdout.Fd()) && !ctx.Bool("no-random-sleep") {
    // ...
}

(N.B.: This is the same approach certbot uses)

isatty is already an indirect dependency, and works under various GOOS, however it might delay moving to Go 1.18: mattn/go-isatty#72

If we don't want to use that, the log.Infof call should also print something like "add --no-random-sleep to skip".

What do you think, shall I open a PR?

@ldez
Copy link
Member Author

ldez commented Jun 17, 2022

@dmke you can open a PR.

For the problem with go1.18 we already use an updated version of golang.org/x/sys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

small random sleep at renewal to avoid load spikes
2 participants