Skip to content

Commit

Permalink
drop TODO comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Yuvaraj Kakaraparthi committed May 14, 2023
1 parent 9bb25e4 commit 14ee7f0
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 9 deletions.
8 changes: 0 additions & 8 deletions internal/controllers/topology/cluster/reconcile_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,6 @@ func (r *Reconciler) reconcileMachineDeployments(ctx context.Context, s *scope.S
// Create MachineDeployments.
for _, mdTopologyName := range diff.toCreate {
md := s.Desired.MachineDeployments[mdTopologyName]
// Note: We don't need to hold reconcile while creating an MD here because of 2 reasons:
// 1) New MDs never get marked as "pending upgrade" as "upgrade" does not apply for new MDs, so HoldReconcile will always return false.
// 2) The MS preflight checks will stop the MD machines from actually coming up so that it does not hit the kubeadm join issue.
// Another option would be to stop creation if ControlPlane.HoldReconcile() is true. But it is probably better for the user to see that the
// MD is created and waiting for pre-flight checks to pass to create the machine than not see a MD created at all.
// TODO(ykakarap): Validate the better option here.
// TODO(ykakarap): Drop the above comment.
if err := r.createMachineDeployment(ctx, s.Current.Cluster, md); err != nil {
return err
}
Expand All @@ -477,7 +470,6 @@ func (r *Reconciler) reconcileMachineDeployments(ctx context.Context, s *scope.S
currentMD := s.Current.MachineDeployments[mdTopologyName]
desiredMD := s.Desired.MachineDeployments[mdTopologyName]
if s.UpgradeTracker.MachineDeployments.HoldReconcile(currentMD.Object.Name) {
// TODO: This will also hold reconciliation of the MHC. Is that okay? Probably.
continue
}
if err := r.updateMachineDeployment(ctx, s.Current.Cluster, mdTopologyName, currentMD, desiredMD); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ func (m *MachineDeploymentUpgradeTracker) DeferredUpgrade() bool {

// HoldReconcile returns true if the MachineDeployment reconciliation needs to be held back.
// Example: MachineDeployment reconciliation needs to be held back if it is pending an upgrade.
// TODO(ykakarap): What about deferred? -> We should not hold reconcile for deferred MDs as that is held back by the user on purpose. <- double check this.
func (m *MachineDeploymentUpgradeTracker) HoldReconcile(name string) bool {
return m.pendingNames.Has(name)
}

0 comments on commit 14ee7f0

Please sign in to comment.