Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Co-authored-by: Himanshu Sharma <[email protected]>
  • Loading branch information
aaronfern and himanshu-kun committed Oct 11, 2023
1 parent 8c9b9c9 commit 60b89e6
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
5 changes: 3 additions & 2 deletions pkg/util/provider/machinecontroller/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,10 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque
// If node label is not present
klog.V(2).Infof("Creating a VM for machine %q, please wait!", machine.Name)
klog.V(2).Infof("The machine creation is triggered with timeout of %s", c.getEffectiveCreationTimeout(createMachineRequest.Machine).Duration)
createMCCtx, cancelFunc := c.getCreationContext(ctx, machine)
createMachineCtx, cancelFunc := c.getCreationContext(ctx, machine)
defer cancelFunc()
createMachineResponse, err := c.driver.CreateMachine(createMCCtx, createMachineRequest)
//TODO Adapt all mcm-providers driver method implementations to use the passed context
createMachineResponse, err := c.driver.CreateMachine(createMachineCtx, createMachineRequest)
if err != nil {
// Create call returned an error.
klog.Errorf("Error while creating machine %s: %s", machine.Name, err.Error())
Expand Down
6 changes: 3 additions & 3 deletions pkg/util/provider/machinecontroller/machine_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,16 +513,16 @@ func (c *controller) machineCreateErrorHandler(ctx context.Context, machine *v1a
return retryRequired, statusUpdErr
}

// adjustCreateRetryRequired adjusts the retry period if needed so that we don't have a case where the machine is reconciled after machineCreationTimeout
// if the retry period is too large so that, it is adjusted so that it causes a reconcile at the machineCreationTimeout
// adjustCreateRetryRequired adjusts the retry period if needed so that we don't have a case where the machine is reconciled long (<=ShortRetry) after machineCreationTimeout
// if the retry period is too large, it is adjusted to cause a reconcile at the machineCreationTimeout
// if the machineCreationTimeout has already passed, return `ShortRetry` so that the machine is immediately reconciled
func (c *controller) adjustCreateRetryRequired(machine *v1alpha1.Machine, retryRequired machineutils.RetryPeriod) machineutils.RetryPeriod {
adjustedRetry := retryRequired
machineCreationDeadline := machine.CreationTimestamp.Time.Add(c.getEffectiveCreationTimeout(machine).Duration)
if time.Now().After(machineCreationDeadline) {
adjustedRetry = machineutils.ShortRetry
} else if time.Now().Add(time.Duration(retryRequired)).After(machineCreationDeadline) {
// Machine will reconcile after create deadline. Adapt RetryPeriod to reconcile machine at deadline
// Machine will reconcile after create deadline, so adapt RetryPeriod to reconcile machine at deadline
adjustedRetry = machineutils.RetryPeriod(machineCreationDeadline.Sub(time.Now()))
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/util/provider/machinecontroller/machine_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2726,7 +2726,7 @@ var _ = Describe("machine_util", func() {
)
})

Describe("#Adjust Machine Create Retry Timeout", func() {
Describe("#Adjust Machine Create Retry Period if near Timeout", func() {
type setup struct {
machine *machinev1.Machine
mcCreateTimeout metav1.Duration
Expand Down Expand Up @@ -2755,6 +2755,7 @@ var _ = Describe("machine_util", func() {
adjustedRetry := c.adjustCreateRetryRequired(data.setup.machine, machineutils.RetryPeriod(data.setup.createOpRetry))

if data.expect.retryAdjusted {
// Considering time.Now() = 0
Expect(adjustedRetry).Should(BeNumerically("<=", machineutils.RetryPeriod(c.getEffectiveCreationTimeout(data.setup.machine).Duration)))
} else {
Expect(adjustedRetry).To(Equal(data.setup.createOpRetry))
Expand All @@ -2775,7 +2776,7 @@ var _ = Describe("machine_util", func() {
retryAdjusted: false,
},
}),
Entry("Should adjust machine create retry period to machine create deadline if retry is set to occur after machine create deadline", &data{
Entry("Should adjust machine create retry period to retry at machine create deadline if retry is set to occur after machine create deadline", &data{
setup: setup{
createOpRetry: machineutils.RetryPeriod(4 * time.Minute),
mcCreateTimeout: metav1.Duration{Duration: 3 * time.Minute},
Expand Down

0 comments on commit 60b89e6

Please sign in to comment.