Merge pull request #268 from davidvossel/smooth-over-internal-ip

Only set internal ip on node when default IP is present on VMI

pkg/provider/instances_v2.go

@@ -88,7 +88,7 @@ func (i *instancesV2) InstanceMetadata(ctx context.Context, node *corev1.Node) (
 	if err != nil {
 		return nil, err
 	}
-	addrs := i.getNodeAddresses(instance.Status.Interfaces)
+	addrs := i.getNodeAddresses(instance.Status.Interfaces, node.Status.Addresses)
 
 	instanceType := ""
 	if val, ok := instance.Annotations[kubevirtv1.InstancetypeAnnotation]; ok {
@@ -132,20 +132,36 @@ func (i *instancesV2) findInstance(ctx context.Context, fetchers ...InstanceGett
 	return instance, nil
 }
 
-func (i *instancesV2) getNodeAddresses(ifs []kubevirtv1.VirtualMachineInstanceNetworkInterface) []corev1.NodeAddress {
+func (i *instancesV2) getNodeAddresses(ifs []kubevirtv1.VirtualMachineInstanceNetworkInterface, prevAddrs []corev1.NodeAddress) []corev1.NodeAddress {
 	var addrs []corev1.NodeAddress
 
+	foundInternalIP := false
 	// TODO: detect type of all addresses, right now pick only the default
 	for _, i := range ifs {
-		if i.Name == "default" {
+		// Only change the IP if it is known, not if it is empty
+		if i.Name == "default" && i.IP != "" {
 			v1helper.AddToNodeAddresses(&addrs, corev1.NodeAddress{
 				Type:    corev1.NodeInternalIP,
 				Address: i.IP,
 			})
+			foundInternalIP = true
 			break
 		}
 	}
 
+	// fall back to the previously known internal IP on the node
+	// if the default IP on the vmi.status.interfaces is not present.
+	// This smooths over issues where the vmi.status.interfaces field is
+	// not reporting results due to an internal reboot or other issues when
+	// contacting the qemu guest agent.
+	if !foundInternalIP {
+		for _, prevAddr := range prevAddrs {
+			if prevAddr.Type == corev1.NodeInternalIP {
+				v1helper.AddToNodeAddresses(&addrs, prevAddr)
+			}
+		}
+	}
+
 	return addrs
 }
 

pkg/provider/instances_v2_test.go

@@ -114,6 +114,95 @@ var _ = Describe("Instances V2", func() {
 
 		Context("With a node name name value equal to a vmi name", func() {
 
+			It("Should default to last known internal IP when no vmi.status.interfaces default ip exists", func() {
+				vmiName := "test-vm"
+				namespace := "cluster-qwedas"
+				i := instancesV2{
+					namespace: namespace,
+					client:    mockClient,
+					config: &InstancesV2Config{
+						Enabled:              true,
+						ZoneAndRegionEnabled: true,
+					},
+				}
+
+				infraNode := corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "infra-node",
+						Labels: map[string]string{
+							corev1.LabelTopologyRegion: "region-a",
+							corev1.LabelTopologyZone:   "zone-1",
+						},
+					},
+				}
+
+				vmi := kubevirtv1.VirtualMachineInstance{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      vmiName,
+						Namespace: namespace,
+						Annotations: map[string]string{
+							kubevirtv1.InstancetypeAnnotation: "highPerformance",
+						},
+					},
+					Status: kubevirtv1.VirtualMachineInstanceStatus{
+						Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{
+							{
+								IP:   "10.245.0.1",
+								Name: "unknown",
+							},
+							{
+								IP: "10.246.0.1",
+							},
+						},
+						NodeName: infraNode.Name,
+					},
+				}
+
+				tenantNode := corev1.Node{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: vmiName,
+					},
+					Status: corev1.NodeStatus{
+						Addresses: []corev1.NodeAddress{
+							{
+								Type:    corev1.NodeInternalIP,
+								Address: "10.200.100.1",
+							},
+						},
+					},
+				}
+
+				gomock.InOrder(
+					mockClient.EXPECT().
+						Get(ctx, types.NamespacedName{Name: vmiName, Namespace: namespace}, gomock.AssignableToTypeOf(&kubevirtv1.VirtualMachineInstance{})).
+						SetArg(2, vmi).
+						Times(1),
+					mockClient.EXPECT().
+						Get(ctx, client.ObjectKey{Name: infraNode.Name}, gomock.AssignableToTypeOf(&corev1.Node{})).
+						SetArg(2, infraNode).
+						Times(1),
+				)
+
+				metadata, err := i.InstanceMetadata(ctx, &tenantNode)
+				Expect(err).To(BeNil())
+
+				idFn := func(index int, element interface{}) string {
+					return strconv.Itoa(index)
+				}
+				Expect(*metadata).To(MatchAllFields(Fields{
+					"ProviderID": Equal("kubevirt://test-vm"),
+					"NodeAddresses": MatchAllElementsWithIndex(idFn, Elements{
+						"0": MatchAllFields(Fields{
+							"Address": Equal("10.200.100.1"),
+							"Type":    Equal(corev1.NodeInternalIP),
+						}),
+					}),
+					"InstanceType": Equal("highPerformance"),
+					"Region":       Equal("region-a"),
+					"Zone":         Equal("zone-1"),
+				}))
+			})
+
 			It("Should fetch a vmi by node name and return a complete metadata object", func() {
 				vmiName := "test-vm"
 				namespace := "cluster-qwedas"
@@ -166,6 +255,14 @@ var _ = Describe("Instances V2", func() {
 					ObjectMeta: metav1.ObjectMeta{
 						Name: vmiName,
 					},
+					Status: corev1.NodeStatus{
+						Addresses: []corev1.NodeAddress{
+							{
+								Type:    corev1.NodeInternalIP,
+								Address: "10.200.100.1",
+							},
+						},
+					},
 				}
 
 				gomock.InOrder(