NodeReload refactor #586
NodeReload refactor #586
Conversation
worryg0d
force-pushed
the
nodereload-refactor
branch
2 times, most recently
from
a84dc49 to
fac2810
Compare
worryg0d
force-pushed
the
nodereload-refactor
branch
3 times, most recently
from
5f6f38e to
2c6f78a
Compare
worryg0d
requested review from
testisnullus,
ribaraka,
RostislavPorohnya,
tengu-alt and
OleksiienkoMykyta
and removed request for
testisnullus and
ribaraka
worryg0d
force-pushed
the
nodereload-refactor
branch
3 times, most recently
from
d1464a2 to
c8d53fc
Compare
worryg0d
force-pushed
the
nodereload-refactor
branch
from
c8d53fc to
af21b12
Compare
| } | ||
| nrs.Status.NodeInProgress.ID = nodeInProgress.ID | ||
| if nrs.Status.NodeInProgress == nil { | ||
| nodeInProgress := nrs.Status.PendingNodes[0] |
There was a problem hiding this comment.
Let's keep the previous indexing, like nrs.Status.PendingNodes[len(nrs.Status.PendingNodes)-1], wdyt?
There was a problem hiding this comment.
I think we have to reload nodes in the order they are presented in the spec.
| } | ||
|
|
||
| return models.ExitReconcile, nil | ||
| l.Error(err, "Node is not found on Instaclustr", |
There was a problem hiding this comment.
Node is not found on the Instaclustr side, something like that
There was a problem hiding this comment.
Changed to Node is not found on the Instaclustr side
worryg0d
force-pushed
the
nodereload-refactor
branch
from
af21b12 to
3f4bb22
Compare
| @@ -60,151 +68,188 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) | |||
| if err != nil { | |||
| if k8serrors.IsNotFound(err) { | |||
| l.Error(err, "Node Reload resource is not found", "request", req) | |||
| return models.ExitReconcile, nil | |||
| return reconcile.Result{}, err | |||
There was a problem hiding this comment.
it's better to return nil error here
| if nrs.Status.NodeInProgress == nil { | ||
| nodeInProgress := nrs.Status.PendingNodes[0] |
There was a problem hiding this comment.
I think it'd be safer to add a check for not nil of nrs.Status.PendingNodes[0]
There was a problem hiding this comment.
It doesn't make any sense because it doesn't pass inside if there is a node in progress. If there is no it triggers node reloads, sets the node in progress, and pops it from the pending nodes. Then it starts checking node reload status and when it finishes it sets node in progress to nil.
worryg0d
force-pushed
the
nodereload-refactor
branch
from
3f4bb22 to
e894f46
Compare
worryg0d
added
enhancement
The NodeReload controller was refactored. Now it doesn't update the resource spec while it reloads nodes.
Added:
PendingNodesshows which nodes awaiting reloadCompletedNodesshows which nodes are successfully reloadedFailedNodesshows which nodes cannot be reloaded because they are not foundAlso, in the previous implementation of the controller, it deletes resources after a successful reload of all nodes. In this implementation, it doesn't delete the resources but writes events to it.