Add a max failed CNRs threshold to Nodegroups #88
Conversation
vincentportella
changed the title
…to vportella/add-max-failed-cnrs-threshold
…to vportella/add-max-failed-cnrs-threshold
| @@ -30,6 +44,10 @@ type Transitioner struct { | |||
|
|
|||
| CloudProviderInstances []*mock.Node | |||
| KubeNodes []*mock.Node | |||
|
|
|||
| extrakubeObjects []client.Object | |||
There was a problem hiding this comment.
nit: extraKubeObjects
| }, | ||
| } | ||
|
|
||
| // Failed CNR for the same nodegroup in a different namespace |
There was a problem hiding this comment.
Are CNRs intended to be namespaced? I can't see much value in separating them by namespace like this but being able to act on the same set of backing nodes provided by a cloud provider.
There was a problem hiding this comment.
That would go back to when they were designed. I'm not particularly sure, though changing that is out of scope here.
| @@ -536,3 +536,38 @@ func (t *CycleNodeRequestTransitioner) validateInstanceState(validNodeGroupInsta | |||
|
|
|||
| return false, nil | |||
| } | |||
|
|
|||
| // deleteFailedSiblingCNRs finds the CNRs generated for the same nodegroup as | |||
| // the one in the calling transitioner. It filters for deleted CNRs in the same | |||
There was a problem hiding this comment.
nit: it's not a calling transitioner, it's the parent struct technically. I would just say "the one in the transitioner"
pkg/observer/controller.go
Outdated
| } | ||
| } | ||
| if found { | ||
|
|
||
| if dropNodeGroup { | ||
| klog.Warningf("nodegroup %q has an in progress CNR.. skipping this nodegroup", nodeGroup.Name) |
There was a problem hiding this comment.
This log message now doesn't match reality. dropNodeGroup is being set to true when the number of failed CNRs exceeds the threshold, and not when it has an in progress CNR.
There was a problem hiding this comment.
I will add a second log line there because Failed is defined as "in progress" in the existing implementation. I'm adding functionality to skip a certain number of them from counting as "in progress" so I will update for that.
maxFailedCycleNodeRequestsattribute to theNodeGroupspec which defines how many failed CNRs are allowed for a nodegroup before observer will stop generating new ones. The equivalent to the current behaviour ismaxFailedCycleNodeRequests: 0. This is not a breaking change.Successfulphase.cyclops_cycle_node_requests_by_phasemetric has been update to include a newnodegroupattribute which is the name of the nodegroup the CNR was generated from. Manually created CNRs that don't match a nodegroup will include the attribute with an empty string.