OCPBUGS-54805: Handle deleted nodes

pkg/cloudprovider/openstack.go

@@ -318,27 +318,31 @@ func (o *OpenStack) AssignPrivateIP(ip net.IP, node *corev1.Node) error {
 // of nodeToAdd. Additionally, if reservation port is missing MovePrivateIP will attempt to recreate it (this is a
 // corner case and should not happen in normal operation).
 func (o *OpenStack) MovePrivateIP(ip net.IP, nodeToAdd, nodeToDel *corev1.Node) error {
-	if nodeToAdd == nil || nodeToDel == nil {
+	if nodeToAdd == nil {
 		return fmt.Errorf("invalid nil pointer provided for node when trying to move IP %s", ip.String())
 	}
 
-	// List all ports that are attached to this server.
-	serverID, err := getNovaServerIDFromProviderID(nodeToDel.Spec.ProviderID)
-	if err != nil {
-		return err
-	}
-	serverPorts, err := o.listNovaServerPorts(serverID)
-	if err != nil {
-		return err
-	}
+	if nodeToDel != nil {
+		// List all ports that are attached to this server.
+		serverID, err := getNovaServerIDFromProviderID(nodeToDel.Spec.ProviderID)
+		if err != nil {
+			return err
+		}
+		serverPorts, err := o.listNovaServerPorts(serverID)
+		if err != nil {
+			return err
+		}
 
-	// Loop over all ports that are attached to this nova instance.
-	for _, serverPort := range serverPorts {
-		if isIPAddressAllowedOnNeutronPort(serverPort, ip) {
-			if err = o.unallowIPAddressOnNeutronPort(serverPort.ID, ip); err != nil {
-				return err
+		// Loop over all ports that are attached to this nova instance.
+		for _, serverPort := range serverPorts {
+			if isIPAddressAllowedOnNeutronPort(serverPort, ip) {
+				if err = o.unallowIPAddressOnNeutronPort(serverPort.ID, ip); err != nil {
+					return err
+				}
 			}
 		}
+	} else {
+		klog.Infof("Source node not provided, not modifying existing reservations")
 	}
 
 	subnet, port, err := o.findAssignSubnetAndPort(ip, nodeToAdd)
@@ -348,7 +352,7 @@ func (o *OpenStack) MovePrivateIP(ip net.IP, nodeToAdd, nodeToDel *corev1.Node)
 
 	// This call is to double-check if the reservation port exists and update its DeviceID. If reservation port is
 	// missing it will be recreated.
-	serverID, err = getNovaServerIDFromProviderID(nodeToAdd.Spec.ProviderID) // got to use new node's ProviderID now
+	serverID, err := getNovaServerIDFromProviderID(nodeToAdd.Spec.ProviderID) // got to use new node's ProviderID now
 	if err != nil {
 		return err
 	}

pkg/controller/cloudprivateipconfig/cloudprivateipconfig_controller.go

@@ -186,11 +186,34 @@ func (c *CloudPrivateIPConfigController) SyncHandler(key string) error {
 	case nodeNameToAdd != "" && nodeNameToDel != "":
 		klog.Infof("CloudPrivateIPConfig: %q will be moved from node %q to node %q", key, nodeNameToDel, nodeNameToAdd)
 		nodeToDel, err := c.nodesLister.Get(nodeNameToDel)
-		if err != nil {
+		if err != nil && apierrors.IsNotFound(err) {
+			klog.Infof("Source node: %s no longer exists for CloudPrivateIPConfig: %q", nodeNameToDel, key)
+		} else if err != nil {
 			return err
 		}
+
 		nodeToAdd, err := c.nodesLister.Get(nodeNameToAdd)
 		if err != nil {
+			if apierrors.IsNotFound(err) {
+				klog.Errorf("Target node: %s does not exist for CloudPrivateIPConfig: %q", nodeNameToAdd, key)
+				status = &cloudnetworkv1.CloudPrivateIPConfigStatus{
+					Node: nodeNameToDel,
+					Conditions: []metav1.Condition{
+						{
+							Type:               string(cloudnetworkv1.Assigned),
+							Status:             metav1.ConditionFalse,
+							ObservedGeneration: cloudPrivateIPConfig.Generation,
+							LastTransitionTime: metav1.Now(),
+							Reason:             cloudResponseReasonError,
+							Message:            fmt.Sprintf("Target node %q does not exist", nodeNameToAdd),
+						},
+					},
+				}
+				if _, err = c.updateCloudPrivateIPConfigStatus(cloudPrivateIPConfig, status); err != nil {
+					return fmt.Errorf("error updating CloudPrivateIPConfig: %q status for non-existent target node, err: %v", key, err)
+				}
+				return nil
+			}
 			return err
 		}
 
@@ -213,6 +236,7 @@ func (c *CloudPrivateIPConfigController) SyncHandler(key string) error {
 
 		// This is a blocking call. If the IP is not assigned then don't treat
 		// it as an error.
+		// If nodeToDel is nil (source node was deleted), we can still proceed with the move
 		withMover, ok := c.cloudProviderClient.(cloudprovider.CloudProviderWithMoveIntf)
 		if !ok {
 			return fmt.Errorf("cannot convert driver to the interface with move abilities, this should never happen")
@@ -389,6 +413,26 @@ func (c *CloudPrivateIPConfigController) SyncHandler(key string) error {
 
 		node, err := c.nodesLister.Get(nodeNameToAdd)
 		if err != nil {
+			if apierrors.IsNotFound(err) {
+				klog.Errorf("Node: %s does not exist for CloudPrivateIPConfig: %q", nodeNameToAdd, key)
+				status = &cloudnetworkv1.CloudPrivateIPConfigStatus{
+					Node: nodeNameToAdd,
+					Conditions: []metav1.Condition{
+						{
+							Type:               string(cloudnetworkv1.Assigned),
+							Status:             metav1.ConditionFalse,
+							ObservedGeneration: cloudPrivateIPConfig.Generation,
+							LastTransitionTime: metav1.Now(),
+							Reason:             cloudResponseReasonError,
+							Message:            fmt.Sprintf("Node %q does not exist", nodeNameToAdd),
+						},
+					},
+				}
+				if _, err = c.updateCloudPrivateIPConfigStatus(cloudPrivateIPConfig, status); err != nil {
+					return fmt.Errorf("error updating CloudPrivateIPConfig: %q status for non-existent node, err: %v", key, err)
+				}
+				return nil
+			}
 			return err
 		}
 

pkg/controller/cloudprivateipconfig/cloudprivateipconfig_controller_test.go

@@ -525,6 +525,39 @@ func TestSyncAddCloudPrivateIPConfig(t *testing.T) {
 				fmt.Sprintf("assign-%s-%s", cloudPrivateIPConfigName, nodeNameA),
 			},
 		},
+		{
+			name: "Should set error condition when node does not exist",
+			testObject: &cloudnetworkv1.CloudPrivateIPConfig{
+				ObjectMeta: v1.ObjectMeta{
+					Name: cloudPrivateIPConfigName,
+				},
+				Spec: cloudnetworkv1.CloudPrivateIPConfigSpec{
+					Node: "nonExistentNode",
+				},
+			},
+			expectedObject: &cloudnetworkv1.CloudPrivateIPConfig{
+				ObjectMeta: v1.ObjectMeta{
+					Name: cloudPrivateIPConfigName,
+					Finalizers: []string{
+						cloudPrivateIPConfigFinalizer,
+					},
+				},
+				Spec: cloudnetworkv1.CloudPrivateIPConfigSpec{
+					Node: "nonExistentNode",
+				},
+				Status: cloudnetworkv1.CloudPrivateIPConfigStatus{
+					Node: "nonExistentNode",
+					Conditions: []v1.Condition{
+						{
+							Type:   string(cloudnetworkv1.Assigned),
+							Status: v1.ConditionFalse,
+							Reason: cloudResponseReasonError,
+						},
+					},
+				},
+			},
+			expectedTrackedState: []string{},
+		},
 	}
 	runTests(t, tests)
 }
@@ -1046,6 +1079,55 @@ func TestSyncUpdateCloudPrivateIPConfig(t *testing.T) {
 			mockCloudAssignError:    true,
 			expectErrorOnAssignSync: true,
 		},
+		{
+			name: "Should set error condition when target node does not exist during move",
+			testObject: &cloudnetworkv1.CloudPrivateIPConfig{
+				ObjectMeta: v1.ObjectMeta{
+					Name: cloudPrivateIPConfigName,
+					Finalizers: []string{
+						cloudPrivateIPConfigFinalizer,
+					},
+				},
+				Spec: cloudnetworkv1.CloudPrivateIPConfigSpec{
+					Node: "nonExistentNode",
+				},
+				Status: cloudnetworkv1.CloudPrivateIPConfigStatus{
+					Node: nodeNameA,
+					Conditions: []v1.Condition{
+						{
+							Type:   string(cloudnetworkv1.Assigned),
+							Status: v1.ConditionTrue,
+							Reason: cloudResponseReasonSuccess,
+						},
+					},
+				},
+			},
+			expectedObject: &cloudnetworkv1.CloudPrivateIPConfig{
+				ObjectMeta: v1.ObjectMeta{
+					Name: cloudPrivateIPConfigName,
+					Finalizers: []string{
+						cloudPrivateIPConfigFinalizer,
+					},
+				},
+				Spec: cloudnetworkv1.CloudPrivateIPConfigSpec{
+					Node: "nonExistentNode",
+				},
+				Status: cloudnetworkv1.CloudPrivateIPConfigStatus{
+					Node: "nonExistentNode",
+					Conditions: []v1.Condition{
+						{
+							Type:   string(cloudnetworkv1.Assigned),
+							Status: v1.ConditionFalse,
+							Reason: cloudResponseReasonError,
+						},
+					},
+				},
+			},
+			expectedTrackedState: []string{
+				fmt.Sprintf("release-%s-%s", cloudPrivateIPConfigName, nodeNameA),
+			},
+			isUpdate: true,
+		},
 	}
 	runTests(t, tests)
 }

pkg/controller/node/node_controller.go

@@ -8,6 +8,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	coreinformers "k8s.io/client-go/informers/core/v1"
@@ -147,6 +148,10 @@ func (n *NodeController) SetNodeEgressIPConfigAnnotation(node *corev1.Node, node
 		// See: updateCloudPrivateIPConfigStatus
 		nodeLatest, err := n.kubeClient.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{})
 		if err != nil {
+			if apierrors.IsNotFound(err) {
+				klog.Infof("Node: %s no longer exists, stopping annotation update", node.Name)
+				return nil
+			}
 			return err
 		}
 		existingAnnotations := nodeLatest.Annotations