Add support for Preservation of Machines and Backing nodes

[thiyyakat]

What this PR does / why we need it:

This PR introduces a feature that allows operators and endusers to preserve a machine/node and the backing VM for diagnostic purposes.

The expected behaviour, use cases and usage are detailed in the proposal that can be found here

Which issue(s) this PR fixes:
Fixes #1008

Special notes for your reviewer:

The following tests were carried out serially with the machine-controller-manager-provider-virtual: #1059 (comment)

Please also take a look at the questions asked here.

Release note:

Introduce support for preservation of machines (both Running and Failed), and the backing node (if it exists). 

[gardener-robot]

@thiyyakat You need rebase this pull request with latest master branch. Please check.

[thiyyakat]

Questions that remain unanswered:

1. On recovery of a preserved machine, it transitions from Failed to Running. However, if the preserve annotation was when-failed, then the node continues to be preserved in Running even though the annotation says when-failed - is that okay? The node needs to be preserved so that pods can get scheduled onto it without CA scaling it down.
   Update: We allow the annotation to stay, but we clear PreserveExpirTime and set the node condition to false. The CA annotation remains until manually removed from node.
2. drain timeout is checked currently by calculating time from LastUpdateTime (from when machine moved to Failed) to now. Is there a better way to do it?
   timeOutOccurred = utiltime.HasTimeOutOccurred(machine.Status.CurrentStatus.LastUpdateTime, timeOutDuration)
   In the normal drain, it is checked wrt DeletionTimestamp
3. In some parts of the code, checks are performed to see if the returned error is due to a Conflict, and ConflictRetry rather than ShortRetry is returned. When should these checks be performed? The preservation flow has a lot of update calls. : Addressed. Use ConflictRetry when appropriate.

[thiyyakat]

Note: A review meeting was held today for this PR. The comments were given during the meeting.

During the meeting, we revisited the decision to move drain to Failed state for preserved machine. The reason discussed previously was that it didn't make sense semantically to move the machine to Terminating and then do the drain, because there is a possibility that the machine may recover. Since Terminating is a final state, the drain (separate from the drain in triggerDeletionFlow) will be performed in Failed phase. There was no change proposed during the meeting. This design decision was only reconfirmed.

[takoverflow]

Have only gone through half of the PR, have some suggestions PTAL.

[aaronfern]

Thanks for the PR @thiyyakat!
A few questions/nits from me, please address them

[elankath]

These condition constants feel like they are in the wrong place as we already have conditions at pkg/apis/machine/types.go. Also, I don't think the Node prefix should be used for the condition constant names as they are used in Machine objects too. @unmarshall should these even be exposed in API ?

[thiyyakat]

I've added them here after seeing the constants for InPlaceUpdates added just above:

machine-controller-manager/pkg/apis/machine/v1alpha1/machine_types.go

Line 226 in 219d435

NodeInPlaceUpdate corev1.NodeConditionType = "InPlaceUpdate"

The NodeCondition for InPlace is named NodeInPlaceUpdate, and I've followed the same.

@elankath , @unmarshall , please let me know what change you would like me to make.

[elankath]

@thiyyakat Ok, but the the reason constants like NodePreservedByMCM, etc should just be PreservedByMCM - that is also the convention followed by in-place update constants.

PreservedNodeDrainSuccessful -> DrainSuccessful

[thiyyakat]

Will make the change to the other constant names and shorten them.

This one: PreservedNodeDrainSuccessful -> DrainSuccessful I am unsure of what to do. DrainSuccessful is used as a Reason for InPlaceUpdate, and the comment indicates the same. Is it okay to re-use it for a Message?
Ref:

machine-controller-manager/pkg/apis/machine/v1alpha1/machine_types.go

Line 237 in 219d435

// DrainSuccessful is a constant for reason in condition that indicates node drain is successful

[r4mek]

shouldn't this be inside

if machineutils.IsMachineFailed(machine) {
    preserve := c.shouldFailedMachineBeTerminated(machine)
	if !preserve {
		staleMachines = append(staleMachines, machine)
	}
}

[thiyyakat]

Yes. Thanks! Please hold off on reviewing further. I'm still testing the code after the rebase. Had to push a commit to include hash in virtual-provider go.mod. Will ping you once I'm done. Thanks, again!

[aaronfern]

Thank you for the changes!
/lgtm

[gardener-prow]

LGTM label has been added.

Details

Git tree hash: 11acccc2ad9b00feb4c11b902b865621a4c3120c

[r4mek]

/lgtm

[gardener-prow]

LGTM label has been added.

Details

Git tree hash: 11acccc2ad9b00feb4c11b902b865621a4c3120c

[takoverflow]

Thanks for the changes and bearing with multiple review rounds
/lgtm

[gardener-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aaronfern, gagan16k, takoverflow

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [aaronfern,gagan16k,takoverflow]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment