Annotate machine-deployment if machine-types are not available in cloud #454
Conversation
gardener-robot-ci-2
added
the
reviewed/ok-to-test
gardener-robot-ci-3
added
needs/ok-to-test
hardikdr
force-pushed
the
feature/oor-annotation
branch
from
40a10a1 to
50d3104
Compare
gardener-robot-ci-3
added
reviewed/ok-to-test
hardikdr
changed the title
hardikdr
force-pushed
the
feature/oor-annotation
branch
from
50d3104 to
5574a53
Compare
gardener-robot-ci-2
added
the
reviewed/ok-to-test
gardener-robot-ci-1
removed
the
reviewed/ok-to-test
|
/test |
ghost
added
the
hardikdr
added
reviewed/ok-to-test
gardener-robot-ci-2
removed
the
reviewed/ok-to-test
|
|
||
| _, errUpdate := c.controlMachineClient.MachineDeployments(machine.Namespace).Update(mdCopy) | ||
| if errUpdate != nil { | ||
| klog.Errorf("Error updating the machine-deployment %+v", errUpdate) |
There was a problem hiding this comment.
Not returning the error here, to keep the error the intact from the cloud-provider, or should we return the errUpdate ?
| @@ -428,6 +429,27 @@ func (c *controller) machineCreate(machine *v1alpha1.Machine, driver driver.Driv | |||
| klog.Warning(err) | |||
| } | |||
|
|
|||
| // Check if the creation-error logs contain the specific pattern, and annotate the machine-deployment if found. | |||
| contains, _ := c.containsSpecialPattern(err.Error(), MachineTypeNotAvailableAzure, MachineTypeNotAvailableAWS) | |||
There was a problem hiding this comment.
I think we should move this logic into the machine deployment controller. We shouldn't be updating machine deployments in machine reconciliation. The machine deployment controller can look up for these patterns in failedMachines field?
There was a problem hiding this comment.
Hm, that's a valid point.
I had actually started implementing this into the deployment controller. But later moved it here. First of all the log-patterns are extremely cloud-provider specific, may keep changing, and providers may want to add more such patterns in the future. I felt the best place for this logic is to be at provider-specific machine-controllers.
There was a problem hiding this comment.
My concern being that, the machine controller on OOT will not have access to machineDeployments. It will not be able to update this. One alternate approach could be this - gardener/autoscaler#37 (comment)?
|
Update: We hold the development here unless we see the need for improvement again. Let's re-open later if required. |
What this PR does / why we need it:
This PR adds the annotation at the nodeSpec of the MachineDeployment, whenever a specific pattern is found in the creation-logs.
It is also mainly motivated due to the resource-scarcity at Azure, where certain machine-types are not available. MCM adds the annotation on such MachineDeployment, which becomes the hint for the autoscaler to keep trying to scale-up it up.
Related to this: gardener/autoscaler#35
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: