From fac281073f80988fc0eb43eae3ee9f801e40c8f9 Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Thu, 5 Oct 2023 15:13:33 +0300 Subject: [PATCH] NodeReload controller was refactored --- .../v1beta1/nodereload_types.go | 26 ++-- .../v1beta1/zz_generated.deepcopy.go | 39 +++++- ...resources.instaclustr.com_nodereloads.yaml | 27 ++++ .../clusterresources/nodereload_controller.go | 120 ++++++++++++------ pkg/instaclustr/client.go | 4 + 5 files changed, 162 insertions(+), 54 deletions(-) diff --git a/apis/clusterresources/v1beta1/nodereload_types.go b/apis/clusterresources/v1beta1/nodereload_types.go index fcbb47bcf..1a75c130d 100644 --- a/apis/clusterresources/v1beta1/nodereload_types.go +++ b/apis/clusterresources/v1beta1/nodereload_types.go @@ -30,8 +30,11 @@ type NodeReloadSpec struct { // NodeReloadStatus defines the observed state of NodeReload type NodeReloadStatus struct { - NodeInProgress Node `json:"nodeInProgress,omitempty"` + NodeInProgress *Node `json:"nodeInProgress,omitempty"` CurrentOperationStatus *Operation `json:"currentOperationStatus,omitempty"` + PendingNodes []*Node `json:"pendingNodes,omitempty"` + CompletedNodes []*Node `json:"completedNodes,omitempty"` + FailedNodes []*Node `json:"failedNodes,omitempty"` } type Node struct { @@ -76,19 +79,12 @@ func init() { SchemeBuilder.Register(&NodeReload{}, &NodeReloadList{}) } -func (nr *NodeReloadStatus) FromInstAPI(status *models.NodeReloadStatus) *NodeReloadStatus { - var nrStatus = &NodeReloadStatus{ - NodeInProgress: Node{ - ID: status.NodeID, - }, - CurrentOperationStatus: &Operation{ - OperationID: status.OperationID, - TimeCreated: status.TimeCreated, - TimeModified: status.TimeModified, - Status: status.Status, - Message: status.Message, - }, +func (nr *NodeReloadStatus) UpdateCurrentOperationStatus(status *models.NodeReloadStatus) { + nr.CurrentOperationStatus = &Operation{ + OperationID: status.OperationID, + TimeCreated: status.TimeCreated, + TimeModified: status.TimeModified, + Status: status.Status, + Message: status.Message, } - - return nrStatus } diff --git a/apis/clusterresources/v1beta1/zz_generated.deepcopy.go b/apis/clusterresources/v1beta1/zz_generated.deepcopy.go index db442acec..c9c23a015 100644 --- a/apis/clusterresources/v1beta1/zz_generated.deepcopy.go +++ b/apis/clusterresources/v1beta1/zz_generated.deepcopy.go @@ -1281,12 +1281,49 @@ func (in *NodeReloadSpec) DeepCopy() *NodeReloadSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeReloadStatus) DeepCopyInto(out *NodeReloadStatus) { *out = *in - out.NodeInProgress = in.NodeInProgress + if in.NodeInProgress != nil { + in, out := &in.NodeInProgress, &out.NodeInProgress + *out = new(Node) + **out = **in + } if in.CurrentOperationStatus != nil { in, out := &in.CurrentOperationStatus, &out.CurrentOperationStatus *out = new(Operation) **out = **in } + if in.PendingNodes != nil { + in, out := &in.PendingNodes, &out.PendingNodes + *out = make([]*Node, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Node) + **out = **in + } + } + } + if in.CompletedNodes != nil { + in, out := &in.CompletedNodes, &out.CompletedNodes + *out = make([]*Node, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Node) + **out = **in + } + } + } + if in.FailedNodes != nil { + in, out := &in.FailedNodes, &out.FailedNodes + *out = make([]*Node, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Node) + **out = **in + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeReloadStatus. diff --git a/config/crd/bases/clusterresources.instaclustr.com_nodereloads.yaml b/config/crd/bases/clusterresources.instaclustr.com_nodereloads.yaml index 49e44db04..141679c0a 100644 --- a/config/crd/bases/clusterresources.instaclustr.com_nodereloads.yaml +++ b/config/crd/bases/clusterresources.instaclustr.com_nodereloads.yaml @@ -50,6 +50,15 @@ spec: status: description: NodeReloadStatus defines the observed state of NodeReload properties: + completedNodes: + items: + properties: + nodeID: + type: string + required: + - nodeID + type: object + type: array currentOperationStatus: properties: message: @@ -68,6 +77,15 @@ spec: - timeCreated - timeModified type: object + failedNodes: + items: + properties: + nodeID: + type: string + required: + - nodeID + type: object + type: array nodeInProgress: properties: nodeID: @@ -75,6 +93,15 @@ spec: required: - nodeID type: object + pendingNodes: + items: + properties: + nodeID: + type: string + required: + - nodeID + type: object + type: array type: object type: object served: true diff --git a/controllers/clusterresources/nodereload_controller.go b/controllers/clusterresources/nodereload_controller.go index 43865122d..2ba40dc52 100644 --- a/controllers/clusterresources/nodereload_controller.go +++ b/controllers/clusterresources/nodereload_controller.go @@ -18,6 +18,7 @@ package clusterresources import ( "context" + "errors" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -28,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/instaclustr/operator/apis/clusterresources/v1beta1" "github.com/instaclustr/operator/pkg/instaclustr" @@ -42,6 +44,10 @@ type NodeReloadReconciler struct { EventRecorder record.EventRecorder } +const ( + NodeReloadOperationStatusCompleted = "COMPLETED" +) + //+kubebuilder:rbac:groups=clusterresources.instaclustr.com,resources=nodereloads,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=clusterresources.instaclustr.com,resources=nodereloads/status,verbs=get;update;patch //+kubebuilder:rbac:groups=clusterresources.instaclustr.com,resources=nodereloads/finalizers,verbs=update @@ -66,47 +72,50 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return models.ReconcileRequeue, err } - if len(nrs.Spec.Nodes) == 0 { - err = r.Client.Delete(ctx, nrs) + patch := nrs.NewPatch() + if len(nrs.Status.PendingNodes) == 0 && len(nrs.Status.CompletedNodes) == 0 && len(nrs.Status.FailedNodes) == 0 { + nrs.Status.PendingNodes = nrs.Spec.Nodes + err = r.Status().Patch(ctx, nrs, patch) if err != nil { - l.Error(err, - "Cannot delete Node Reload resource from K8s cluster", - "Node Reload spec", nrs.Spec, - ) - r.EventRecorder.Eventf( - nrs, models.Warning, models.DeletionFailed, - "Resource deletion is failed. Reason: %v", - err, + l.Error(err, "Cannot set pending nodes to the status") + r.EventRecorder.Event(nrs, models.Warning, models.PatchFailed, + "Cannot set pending nodes to the resource status", ) + return models.ReconcileRequeue, nil } + } + + if len(nrs.Status.PendingNodes) == 0 && nrs.Status.NodeInProgress == nil { r.EventRecorder.Eventf( - nrs, models.Normal, models.DeletionStarted, - "Resource is deleted.", + nrs, models.Normal, models.UpdatedEvent, + "The controller has finished working", ) l.Info( - "Nodes were reloaded, resource was deleted", - "Node Reload spec", nrs.Spec, + "The controller has finished working", + "completed nodes", nrs.Status.CompletedNodes, + "failed nodes", nrs.Status.FailedNodes, ) + return models.ExitReconcile, nil } - patch := nrs.NewPatch() - if nrs.Status.NodeInProgress.ID == "" { - nodeInProgress := &v1beta1.Node{ - ID: nrs.Spec.Nodes[len(nrs.Spec.Nodes)-1].ID, - } - nrs.Status.NodeInProgress.ID = nodeInProgress.ID + if nrs.Status.NodeInProgress == nil { + nodeInProgress := nrs.Status.PendingNodes[len(nrs.Status.PendingNodes)-1] err = r.API.CreateNodeReload(nodeInProgress) if err != nil { + if errors.Is(err, instaclustr.NotFound) { + return r.handleNodeNotFound(ctx, nodeInProgress, nrs) + } + l.Error(err, "Cannot start Node Reload process", "nodeID", nodeInProgress.ID, ) r.EventRecorder.Eventf( nrs, models.Warning, models.CreationFailed, - "Resource creation on the Instaclustr is failed. Reason: %v", + "Sending node reload request to the Instaclustr is failed. Reason: %v", err, ) return models.ReconcileRequeue, nil @@ -114,15 +123,17 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) r.EventRecorder.Eventf( nrs, models.Normal, models.Created, - "Resource creation request is sent. Node ID: %s", - nrs.Status.NodeInProgress.ID, + "Node reload request is sent. Node ID: %s", + nodeInProgress.ID, ) + nrs.Status.PendingNodes = nrs.Status.PendingNodes[:len(nrs.Status.PendingNodes)-1] + nrs.Status.NodeInProgress = nodeInProgress err = r.Status().Patch(ctx, nrs, patch) if err != nil { l.Error(err, - "Cannot patch Node Reload status", - "nodeID", nrs.Status.NodeInProgress, + "Cannot patch NodeInProgress and PendingNodes status", + "nodeID", nodeInProgress.ID, ) r.EventRecorder.Eventf( nrs, models.Warning, models.PatchFailed, @@ -135,6 +146,10 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) nodeReloadStatus, err := r.API.GetNodeReloadStatus(nrs.Status.NodeInProgress.ID) if err != nil { + if errors.Is(err, instaclustr.NotFound) { + return r.handleNodeNotFound(ctx, nrs.Status.NodeInProgress, nrs) + } + l.Error(err, "Cannot get Node Reload status", "nodeID", nrs.Status.NodeInProgress, @@ -147,7 +162,7 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return models.ReconcileRequeue, nil } - nrs.Status = *nrs.Status.FromInstAPI(nodeReloadStatus) + nrs.Status.UpdateCurrentOperationStatus(nodeReloadStatus) err = r.Status().Patch(ctx, nrs, patch) if err != nil { l.Error(err, @@ -162,7 +177,7 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return models.ReconcileRequeue, nil } - if nrs.Status.CurrentOperationStatus.Status != "COMPLETED" { + if nrs.Status.CurrentOperationStatus.Status != NodeReloadOperationStatusCompleted { l.Info("Node Reload operation is not completed yet, please wait a few minutes", "nodeID", nrs.Status.NodeInProgress, "status", nrs.Status, @@ -170,7 +185,17 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return models.ReconcileRequeue, nil } - nrs.Status.NodeInProgress.ID = "" + l.Info("The node has been successfully reloaded", + "Node ID", nrs.Status.NodeInProgress.ID, + ) + r.EventRecorder.Eventf(nrs, models.Normal, models.UpdatedEvent, + "Node %s has been successfully reloaded", nrs.Status.NodeInProgress.ID, + ) + + nrs.Status.CompletedNodes = append(nrs.Status.CompletedNodes, nrs.Status.NodeInProgress) + nrs.Status.CurrentOperationStatus = nil + nrs.Status.NodeInProgress = nil + err = r.Status().Patch(ctx, nrs, patch) if err != nil { l.Error(err, @@ -185,21 +210,40 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return models.ReconcileRequeue, nil } - nrs.Spec.Nodes = nrs.Spec.Nodes[:len(nrs.Spec.Nodes)-1] - err = r.Patch(ctx, nrs, patch) + return reconcile.Result{Requeue: true}, nil +} + +func (r *NodeReloadReconciler) handleNodeNotFound(ctx context.Context, node *v1beta1.Node, nrs *v1beta1.NodeReload) (reconcile.Result, error) { + l := log.FromContext(ctx) + + patch := nrs.NewPatch() + + if len(nrs.Status.PendingNodes) > 0 && nrs.Status.NodeInProgress == nil { + nrs.Status.PendingNodes = nrs.Status.PendingNodes[:len(nrs.Status.PendingNodes)-1] + } + + nrs.Status.FailedNodes = append(nrs.Status.FailedNodes, node) + nrs.Status.CurrentOperationStatus = nil + nrs.Status.NodeInProgress = nil + + err := r.Status().Patch(ctx, nrs, patch) if err != nil { - l.Error(err, "Cannot patch Node Reload cluster", - "spec", nrs.Spec, - ) - r.EventRecorder.Eventf( - nrs, models.Warning, models.PatchFailed, - "Resource patch is failed. Reason: %v", - err, + l.Error(err, "Cannot patch failed node") + r.EventRecorder.Event(nrs, models.Warning, models.PatchFailed, + "Cannot patch failed node", ) + return models.ReconcileRequeue, nil } - return models.ExitReconcile, nil + l.Error(err, "Node is not found on Instaclustr", + "Node ID", node.ID, + ) + r.EventRecorder.Eventf(nrs, models.Warning, models.FetchFailed, + "Node %s is not found on Instaclustr", node.ID, + ) + + return reconcile.Result{Requeue: true}, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/pkg/instaclustr/client.go b/pkg/instaclustr/client.go index bfa9ac122..82f8fe97b 100644 --- a/pkg/instaclustr/client.go +++ b/pkg/instaclustr/client.go @@ -1476,6 +1476,10 @@ func (c *Client) CreateNodeReload(nr *clusterresourcesv1beta1.Node) error { return err } + if resp.StatusCode == http.StatusNotFound { + return NotFound + } + if resp.StatusCode != http.StatusAccepted { return fmt.Errorf("status code: %d, message: %s", resp.StatusCode, body) }