Implement exponential backoff and do not update MCD, MS on same error

[elankath]

How to categorize this issue?

/area control-plane
/kind bug
/priority 1

What happened:

We had a live, scalability issue where due to invalid credentials, the etcd database was filled up.
The machine-controller-manager was continuing updating MachineDeployments and MachineSets. The MachineDeployment status contains entry for each Machine and its lastError.

(issues-canary/issues/7190 internally)

What you expected to happen:

• machine-controller-manager should adhere to controller best practices such as exponential backoff and skipping no-op (status) updates if there is no change in the status.

How to reproduce it (as minimally and precisely as possible):

• Use the virtual mcm provider and local api-server and etcd to simulate credential failure for large number of machines (> 1000)
• Check size of etcd db.

Anything else we need to know?:

[aaronfern]

Tasks after Discussion

1. Update controller queues
   a. The queues defined here and here use the deprecated NewNamedRateLimitingQueue. These should be updated to use the recommended alternative NewRateLimitingQueueWithConfig.
   b. Do not have configurable base and limit values. Instead the limit should dynamically change based on maxNodeProvisionTimeout.
2. Avoid redundant calls to update machine status
   a. The function machineCreateErrorHandler is responsible for updating the machine status when an error occurs. This should be updated this to only trigger an update when there is a change in the status to avoid unnecessary updates.
   b. Use the following fields to determine if a status update is necessary: ErrorCode, Type, State, CurrentStatus.Phase
   NOTE: The above list as indicative and not final. Please check all relevant fields to ensure an effective decision on status updates
3. Leverage error from reconcileClusterMachine for rate limiting
   a. Currently, the error returned by reconcileClusterMachine is only logged. We should leverage this error to decide whether the machine should be directly requeued or added to the queue with rate limiting
   b. There are 2 approaches one can take here. The first is to pass the error to the enqueueMachine method where the decision about rate limiting will be made. The second approach would be to create a new method that would directly add the machine to the queue with rate limiting. This decision is left to the implementer.
4. Check for "RetryAfter" field in cloud service provider responses
   a. If the cloud service provider includes a RetryAfter field in its response, this value should take precedence over rate limiting. Ensure that this field is respected wherever applicable.
5. Update machineDeployment.Status only when there is a change
   a. The function syncMachineDeploymentStatus() currently uses reflect.DeepEqual to check if an update is required for machineDeployment.Status. This check should be improved to avoid unnecessary updates
   b. Introduce a new method ShouldUpdate, which takes the old and new status as inputs. This method should leverage go-cmp to compute the diff and determine if a status update is necessary. If no update is needed, the method should log a message instead