MachinePool: MinReadySeconds does not work

[sbueringer]

What steps did you take and what happened?

MachinePool has a MinReadySeconds field just like MachineDeployment. In MachinePools however it is not implemented.

What did you expect to happen?

I would expect it to behave as documented:

	// Minimum number of seconds for which a newly created machine instances should
	// be ready.
	// Defaults to 0 (machine instance will be considered available as soon as it
	// is ready)
	// +optional
	MinReadySeconds *int32 `json:"minReadySeconds,omitempty"`

Cluster API version

main

Kubernetes version

No response

Anything else you would like to add?

No response

Label(s) to be applied

/kind bug
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

[sbueringer]

(cc @CecileRobertMichon, just fyi)

[killianmuldoon]

/triage accepted

Should we add a note to the field for now to communicate that it's not implemented?

[sbueringer]

Sounds like a good idea!

[enxebre]

Related tiny one #8956

[Dhairya-Arora01]

@sbueringer will it be implemented somewhere here

[sbueringer]

That is definitely the place where it's "not implemented" today :).

Not sure how it should be implemented, maybe similar to MachineDeployments. I would leave this to folks more familiar with MachinePools, they can probably provide better guidance

(cc @CecileRobertMichon @Jont828)

[Dhairya-Arora01]

hey @CecileRobertMichon @Jont828 should we create a IsNodeAvailable function like here and, then use it and refractor the code a bit in machinepool_controller_noderef.go

[Jont828]

@sbueringer I'd need to poke around a bit to see how we could implement this.

@Dhairya-Arora01 Would we be able to just call the IsNodeAvailable in the MachinePool controller? It seems like that function wouldn't be able to tell if the node was coming from a MachinePool or a MachineDeployment.

[Dhairya-Arora01]

@Jont828 yes, you are right. We can use IsNodeAvailable in getNodeReferences here
and modify getNodeReferences here
where we pass an additional parameter minReadySeconds which would be used in IsNodeAvailable
Right?
If its the right approach, I'm happy to assign myself

[Jont828]

I think that seems reasonable. I'm making changes to the Docker implementation in #8842, so I think you can implement that once it merges. That way, we can test out your changes with Docker MachinePool Machines.

[Dhairya-Arora01]

/assign

[fabriziopandini]

/priority important-soon