-
Notifications
You must be signed in to change notification settings - Fork 120
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
Fix machine creation bootstrap tokens expiration #773
Conversation
@schrodit Thank you for your contribution. |
Thank you @schrodit for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a serious bug , Thanks for the PR Tim!
expiration := c.safetyOptions.MachineCreationTimeout.Duration | ||
if machine.Spec.MachineConfiguration != nil && machine.Spec.MachineConfiguration.MachineCreationTimeout != nil { | ||
expiration = machine.Spec.MachineConfiguration.MachineCreationTimeout.Duration | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use the helper function getEffectiveCreationTimeout
for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah didn't know that helper function exists.
Updated the code PTAL.
f59217c
to
f8b3aee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
What this PR does / why we need it:
The expiration of bootstrap tokens is hardcoded in the machine controller deployment. (depending on the provider but mostly approx. 20min).
There is an option to overwrite this default per worker group with
machineCreationTimeout
.This option is considered during the machine creation but not in the bootstrap token.
So this means that if the machine takes more time than 20 minutes to be created, the bootstrap token is already expired and the needed configs cannot be fetched form the cluster.
This PR makes the bootstrap token respect the machines
machineCreationTimeout
option.Which issue(s) this PR fixes:
Fixes #772
Special notes for your reviewer:
Tested with the equinix provider
Release note: