From be1aa4b22076c80e877ab619d3db0c8da1cc91a1 Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Mon, 27 Jul 2020 14:25:28 +0000 Subject: [PATCH] Remove nodesToDelete functionality As the nodesToDelete does not describe an end result of the CR, this will be removed with this patch. To remove a specific node or nodes the following process needs to be followed: 1. annotate the machine/machines object corresponding to the node which should be removed 2. lower the worker number of the CR of the machineset --- README.md | 16 ++-- api/v1alpha1/computenodeopenstack_types.go | 12 --- api/v1alpha1/zz_generated.deepcopy.go | 25 ------ ...e.openstack.org_computenodeopenstacks.yaml | 32 ------- .../computenodeopenstack_controller.go | 90 +------------------ 5 files changed, 7 insertions(+), 168 deletions(-) diff --git a/README.md b/README.md index 20cd95c..c98863f 100644 --- a/README.md +++ b/README.md @@ -174,19 +174,13 @@ The following command will show which compute worker node got disabled to be rem #### Remove a specific compute worker node -Edit the computenodeopenstack CR, lower the workers number and add a `nodesToDelete` section in the spec with the details of the worker node which should be removed: +Annotate the machine object corresponding to the node to be removed: - oc -n openstack edit computenodeopenstacks.compute-node.openstack.org example-computenodeopenstack - -Modify the number of `workers` and add the `nodesToDelete` section to the spec: + oc get -n openshift-machine-api machine -o wide + oc annotate -n openshift-machine-api machines machine.openshift.io/cluster-api-delete-machine=1 - nodesToDelete: - - name: - # enable disable live migration for this node. If not specified the global setting from the - # `drain:` section is used. If not specified there, the default is `false` - drain: true/false - -Save and exit. +Edit the computenodeopenstack CR, lower the workers number, save and exit: + oc -n openstack edit computenodeopenstacks.compute-node.openstack.org example-computenodeopenstack ## Cleanup diff --git a/api/v1alpha1/computenodeopenstack_types.go b/api/v1alpha1/computenodeopenstack_types.go index 61819c2..915a6cf 100644 --- a/api/v1alpha1/computenodeopenstack_types.go +++ b/api/v1alpha1/computenodeopenstack_types.go @@ -36,8 +36,6 @@ type ComputeNodeOpenStackSpec struct { Dedicated bool `json:"dedicated,omitempty"` // Infra DaemonSets needed InfraDaemonSets []InfraDaemonSet `json:"infraDaemonSets,omitempty"` - // Nodes to delete upon scale down - NodesToDelete []NodeToDelete `json:"nodesToDelete,omitempty"` // openstackclient configmap which holds information to connect to OpenStack API OpenStackClientConfigMap string `json:"openStackClientConfigMap"` // user secrets used to connect to OpenStack API via openstackclient @@ -99,14 +97,6 @@ type SriovConfig struct { DevName string `json:"devName"` } -// NodeToDelete defines the name of the node to delete and if automatic drain is needed -type NodeToDelete struct { - // Node Name - Name string `json:"name"` - // Automatic draining of the node - Drain bool `json:"drain,omitempty"` -} - // Node defines the status of the associated nodes type Node struct { // Node name @@ -129,8 +119,6 @@ type ComputeNodeOpenStackStatus struct { SpecMDS string `json:"specMDS"` // Nodes information Nodes []Node `json:"nodes,omitempty"` - // Nodes to delete upon scale down - NodesToDelete []NodeToDelete `json:"nodesToDelete,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8f21b21..2d26c0a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -91,11 +91,6 @@ func (in *ComputeNodeOpenStackSpec) DeepCopyInto(out *ComputeNodeOpenStackSpec) *out = make([]InfraDaemonSet, len(*in)) copy(*out, *in) } - if in.NodesToDelete != nil { - in, out := &in.NodesToDelete, &out.NodesToDelete - *out = make([]NodeToDelete, len(*in)) - copy(*out, *in) - } out.Drain = in.Drain out.Compute = in.Compute out.Network = in.Network @@ -124,11 +119,6 @@ func (in *ComputeNodeOpenStackStatus) DeepCopyInto(out *ComputeNodeOpenStackStat *out = make([]Node, len(*in)) copy(*out, *in) } - if in.NodesToDelete != nil { - in, out := &in.NodesToDelete, &out.NodesToDelete - *out = make([]NodeToDelete, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeNodeOpenStackStatus. @@ -202,21 +192,6 @@ func (in *Node) DeepCopy() *Node { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *NodeToDelete) DeepCopyInto(out *NodeToDelete) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeToDelete. -func (in *NodeToDelete) DeepCopy() *NodeToDelete { - if in == nil { - return nil - } - out := new(NodeToDelete) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NovaCompute) DeepCopyInto(out *NovaCompute) { *out = *in diff --git a/config/crd/bases/compute-node.openstack.org_computenodeopenstacks.yaml b/config/crd/bases/compute-node.openstack.org_computenodeopenstacks.yaml index 6b0a98a..cb37f69 100644 --- a/config/crd/bases/compute-node.openstack.org_computenodeopenstacks.yaml +++ b/config/crd/bases/compute-node.openstack.org_computenodeopenstacks.yaml @@ -125,22 +125,6 @@ spec: required: - nic type: object - nodesToDelete: - description: Nodes to delete upon scale down - items: - description: NodeToDelete defines the name of the node to delete and - if automatic drain is needed - properties: - drain: - description: Automatic draining of the node - type: boolean - name: - description: Node Name - type: string - required: - - name - type: object - type: array openStackClientAdminSecret: description: user secrets used to connect to OpenStack API via openstackclient type: string @@ -205,22 +189,6 @@ spec: - status type: object type: array - nodesToDelete: - description: Nodes to delete upon scale down - items: - description: NodeToDelete defines the name of the node to delete and - if automatic drain is needed - properties: - drain: - description: Automatic draining of the node - type: boolean - name: - description: Node Name - type: string - required: - - name - type: object - type: array readyWorkers: description: Number of ready workers format: int32 diff --git a/controllers/computenodeopenstack_controller.go b/controllers/computenodeopenstack_controller.go index 36d3ec9..dbf8c74 100644 --- a/controllers/computenodeopenstack_controller.go +++ b/controllers/computenodeopenstack_controller.go @@ -190,12 +190,6 @@ func (r *ComputeNodeOpenStackReconciler) Reconcile(req ctrl.Request) (ctrl.Resul if err != nil { return ctrl.Result{}, err } - } else if !reflect.DeepEqual(instance.Spec.NodesToDelete, instance.Status.NodesToDelete) || instance.Spec.Workers < instance.Status.Workers { - // Check if nodes to delete information has changed - err := updateMachineDeletionSelection(r.Client, instance) - if err != nil { - return ctrl.Result{}, err - } } // Need to reapply the spec @@ -273,7 +267,6 @@ func (r *ComputeNodeOpenStackReconciler) Reconcile(req ctrl.Request) (ctrl.Resul // Update Status instance.Status.Workers = instance.Spec.Workers instance.Status.InfraDaemonSets = instance.Spec.InfraDaemonSets - instance.Status.NodesToDelete = instance.Spec.NodesToDelete instance.Status.SpecMDS = specMDS err = r.Client.Status().Update(context.TODO(), instance) if err != nil { @@ -571,7 +564,7 @@ func (r *ComputeNodeOpenStackReconciler) updateNodesStatus(instance *computenode /* Steps to delete the drain the node 1. Predraining: disable nova service, (optional) migrate VMs, wait until there is no VMs 2. Postdraining: taint the node, remove nova-compute from nova services and placement - 3. Remove cleanup jobs, blocker pod finalizer, and update nodesToDelete status information + 3. Remove cleanup jobs, blocker pod finalizer */ // 1. NodePreDrain nodePreDrained, err := ensureNodePreDrain(r.Client, node.Name, instance, openstackClientAdmin, openstackClient) @@ -629,22 +622,6 @@ func (r *ComputeNodeOpenStackReconciler) updateNodesStatus(instance *computenode if err != nil { return err } - - log.Info(fmt.Sprintf("Updating nodeToDelete status")) - for i, nodeToDelete := range instance.Spec.NodesToDelete { - if nodeToDelete.Name == node.Name { - if len(instance.Spec.NodesToDelete) == 1 { - instance.Spec.NodesToDelete = []computenodev1alpha1.NodeToDelete{} - } else { - instance.Spec.NodesToDelete = removeNode(instance.Spec.NodesToDelete, i) - } - err = r.Client.Update(context.TODO(), instance) - if err != nil { - return err - } - break - } - } } } @@ -659,11 +636,6 @@ func (r *ComputeNodeOpenStackReconciler) updateNodesStatus(instance *computenode return nil } -func removeNode(nodes []computenodev1alpha1.NodeToDelete, i int) []computenodev1alpha1.NodeToDelete { - nodes[i] = nodes[len(nodes)-1] - return nodes[:len(nodes)-1] -} - func getNodeStatus(c client.Client, node *corev1.Node) string { if node.Spec.Unschedulable { return "SchedulingDisabled" @@ -851,15 +823,6 @@ func _triggerNodeDrain(c client.Client, nodeName string, instance *computenodev1 enableLiveMigration = fmt.Sprintf("%v", instance.Spec.Drain.Enabled) } - - for _, nodeToDelete := range instance.Spec.NodesToDelete { - if nodeToDelete.Name == nodeName { - if nodeToDelete.Drain { - enableLiveMigration = fmt.Sprintf("%v", nodeToDelete.Drain) - } - break - } - } envVars = append(envVars, corev1.EnvVar{Name: "LIVE_MIGRATION_ENABLED", Value: enableLiveMigration}) volumes := []corev1.Volume{ @@ -971,55 +934,6 @@ func isNodeDrained(c client.Client, nodeName string, jobName string, instance *c return false, fmt.Errorf("nodeDrain job type %v, reason: %v", conditionsType, conditionsReason) } -func updateMachineDeletionSelection(c client.Client, instance *computenodev1alpha1.ComputeNodeOpenStack) error { - // We need to delete the old cluster-api-delete-machine labels and add the new ones - nodesToDeleteInfo := make(map[string]string) - for _, newNode := range instance.Spec.NodesToDelete { - nodesToDeleteInfo[newNode.Name] = "Add" - } - for _, oldNode := range instance.Status.NodesToDelete { - _, exists := nodesToDeleteInfo[oldNode.Name] - if exists { - delete(nodesToDeleteInfo, oldNode.Name) - } else { - nodesToDeleteInfo[oldNode.Name] = "Remove" - } - } - machineList := &machinev1beta1.MachineList{} - listOpts := []client.ListOption{ - client.InNamespace("openshift-machine-api"), - client.MatchingLabels{"machine.openshift.io/cluster-api-machine-role": instance.Spec.RoleName}, - } - err := c.List(context.TODO(), machineList, listOpts...) - if err != nil { - return err - } - annotations := map[string]string{} - for _, machine := range machineList.Items { - if machine.Status.NodeRef == nil { - continue - } - - machineNode := machine.Status.NodeRef.Name - action, exists := nodesToDeleteInfo[machineNode] - if !exists { - continue - } - - if action == "Add" { - annotations["machine.openshift.io/cluster-api-delete-machine"] = "1" - machine.SetAnnotations(annotations) - } else if action == "Remove" { - annotations["machine.openshift.io/cluster-api-delete-machine"] = "0" - machine.SetAnnotations(annotations) - } - if err := c.Update(context.TODO(), &machine); err != nil { - return err - } - } - return nil -} - func ensureMachineSetSync(c client.Client, instance *computenodev1alpha1.ComputeNodeOpenStack) error { // get replicas at the openshift-machine-api machineset workerMachineSet := &machinev1beta1.MachineSet{} @@ -1202,7 +1116,7 @@ func deleteAllBlockerPodFinalizer(r *ComputeNodeOpenStackReconciler, instance *c /* Steps to delete the drain the node 1. Predraining: disable nova service, (optional) migrate VMs, wait until there is no VMs 2. Postdraining: taint the node, remove nova-compute from nova services and placement - 3. Remove cleanup jobs, blocker pod finalizer, and update nodesToDelete status information + 3. Remove cleanup jobs, blocker pod finalizer */ // 1. NodePreDrain allNodesPreDrained := true