diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go index e6b60e59205..b7559f16acc 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller.go @@ -146,12 +146,6 @@ func (cnc *CloudNodeController) Run(stopCh <-chan struct{}) { // UpdateNodeStatus updates the node status, such as node addresses func (cnc *CloudNodeController) UpdateNodeStatus(ctx context.Context) { - instances, ok := cnc.cloud.Instances() - if !ok { - utilruntime.HandleError(fmt.Errorf("failed to get instances from cloud provider")) - return - } - nodes, err := cnc.kubeClient.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{ResourceVersion: "0"}) if err != nil { klog.Errorf("Error monitoring node status: %v", err) @@ -159,7 +153,7 @@ func (cnc *CloudNodeController) UpdateNodeStatus(ctx context.Context) { } for i := range nodes.Items { - cnc.updateNodeAddress(ctx, &nodes.Items[i], instances) + cnc.updateNodeAddress(ctx, &nodes.Items[i]) } for _, node := range nodes.Items { @@ -221,29 +215,51 @@ func (cnc *CloudNodeController) reconcileNodeLabels(nodeName string) error { } // UpdateNodeAddress updates the nodeAddress of a single node -func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.Node, instances cloudprovider.Instances) { +func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1.Node) { // Do not process nodes that are still tainted cloudTaint := getCloudTaint(node.Spec.Taints) if cloudTaint != nil { klog.V(5).Infof("This node %s is still tainted. Will not process.", node.Name) return } - // Node that isn't present according to the cloud provider shouldn't have its address updated - exists, err := ensureNodeExistsByProviderID(ctx, instances, node) + + instanceMetadataGetter := func(providerID string, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { + if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok { + return instancesV2.InstanceMetadataByProviderID(ctx, providerID) + } + + // If InstancesV2 not implement, use Instances. + instances, ok := cnc.cloud.Instances() + if !ok { + return nil, fmt.Errorf("failed to get instances from cloud provider") + } + // Node that isn't present according to the cloud provider shouldn't have its address updated + exists, err := ensureNodeExistsByProviderID(ctx, instances, node) + if err != nil { + // Continue to update node address when not sure the node is not exists + klog.Errorf("%v", err) + } else if !exists { + klog.V(4).Infof("The node %s is no longer present according to the cloud provider, do not process.", node.Name) + return nil, fmt.Errorf("node %q not exist", node.Name) + } + + nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, node.Spec.ProviderID, node.Name) + if err != nil { + klog.Errorf("Error getting node addresses for node %q: %v", node.Name, err) + return nil, err + } + return &cloudprovider.InstanceMetadata{ + NodeAddresses: nodeAddresses, + }, nil + } + + instanceMeta, err := instanceMetadataGetter(node.Spec.ProviderID, node) if err != nil { - // Continue to update node address when not sure the node is not exists - klog.Errorf("%v", err) - } else if !exists { - klog.V(4).Infof("The node %s is no longer present according to the cloud provider, do not process.", node.Name) - return - } - - nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, node.Spec.ProviderID, node.Name) - if err != nil { - klog.Errorf("Error getting node addresses for node %q: %v", node.Name, err) + utilruntime.HandleError(err) return } + nodeAddresses := instanceMeta.NodeAddresses if len(nodeAddresses) == 0 { klog.V(5).Infof("Skipping node address update for node %q since cloud provider did not return any", node.Name) return @@ -268,19 +284,16 @@ func (cnc *CloudNodeController) updateNodeAddress(ctx context.Context, node *v1. } // If nodeIP was suggested by user, ensure that // it can be found in the cloud as well (consistent with the behaviour in kubelet) - if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok { - if nodeIP == nil { - klog.Errorf("Specified Node IP not found in cloudprovider for node %q", node.Name) - return - } + if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok && nodeIP == nil { + klog.Errorf("Specified Node IP not found in cloudprovider for node %q", node.Name) + return } if !nodeAddressesChangeDetected(node.Status.Addresses, nodeAddresses) { return } newNode := node.DeepCopy() newNode.Status.Addresses = nodeAddresses - _, _, err = cloudnodeutil.PatchNodeStatus(cnc.kubeClient.CoreV1(), types.NodeName(node.Name), node, newNode) - if err != nil { + if _, _, err := cloudnodeutil.PatchNodeStatus(cnc.kubeClient.CoreV1(), types.NodeName(node.Name), node, newNode); err != nil { klog.Errorf("Error patching node with cloud ip addresses = [%v]", err) } } @@ -322,12 +335,6 @@ func (cnc *CloudNodeController) AddCloudNode(ctx context.Context, obj interface{ func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Node) { klog.Infof("Initializing node %s with cloud provider", node.Name) - instances, ok := cnc.cloud.Instances() - if !ok { - utilruntime.HandleError(fmt.Errorf("failed to get instances from cloud provider")) - return - } - err := clientretry.RetryOnConflict(UpdateNodeSpecBackoff, func() error { // TODO(wlan0): Move this logic to the route controller using the node taint instead of condition // Since there are node taints, do we still need this? @@ -363,7 +370,9 @@ func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Nod return } - nodeModifiers, err := cnc.getNodeModifiersFromCloudProvider(ctx, curNode, instances) + // TODO: getNodeModifiersFromCloudProvider and updateNodeAddress both call cloud api to get instanceMetadata, + // get instanceMetadata and pass it to getNodeModifiersFromCloudProvider and updateNodeAddress which reduces api calls. + nodeModifiers, err := cnc.getNodeModifiersFromCloudProvider(ctx, curNode) if err != nil { utilruntime.HandleError(fmt.Errorf("failed to initialize node %s at cloudprovider: %v", node.Name, err)) return @@ -390,7 +399,7 @@ func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Nod // After adding, call UpdateNodeAddress to set the CloudProvider provided IPAddresses // So that users do not see any significant delay in IP addresses being filled into the node - cnc.updateNodeAddress(ctx, curNode, instances) + cnc.updateNodeAddress(ctx, curNode) klog.Infof("Successfully initialized node %s with cloud provider", node.Name) return nil @@ -405,7 +414,7 @@ func (cnc *CloudNodeController) initializeNode(ctx context.Context, node *v1.Nod // a node object with provider-specific information. // All of the returned functions are idempotent, because they are used in a retry-if-conflict // loop, meaning they could get called multiple times. -func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Context, node *v1.Node, instances cloudprovider.Instances) ([]nodeModifier, error) { +func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Context, node *v1.Node) ([]nodeModifier, error) { var ( nodeModifiers []nodeModifier providerID string @@ -435,30 +444,50 @@ func (cnc *CloudNodeController) getNodeModifiersFromCloudProvider(ctx context.Co providerID = node.Spec.ProviderID } - nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, providerID, node.Name) + instanceMetadataGetter := func(providerID string, nodeName string) (*cloudprovider.InstanceMetadata, error) { + if instancesV2, ok := cnc.cloud.InstancesV2(); instancesV2 != nil && ok { + return instancesV2.InstanceMetadataByProviderID(ctx, providerID) + } + + // If InstancesV2 not implement, use Instances. + instances, ok := cnc.cloud.Instances() + if !ok { + return nil, fmt.Errorf("failed to get instances from cloud provider") + } + nodeAddresses, err := getNodeAddressesByProviderIDOrName(ctx, instances, providerID, nodeName) + if err != nil { + return nil, err + } + instanceType, err := getInstanceTypeByProviderIDOrName(ctx, instances, providerID, nodeName) + if err != nil { + return nil, err + } + return &cloudprovider.InstanceMetadata{ + Type: instanceType, + NodeAddresses: nodeAddresses, + }, nil + } + + instanceMeta, err := instanceMetadataGetter(providerID, node.Name) if err != nil { return nil, err } // If user provided an IP address, ensure that IP address is found // in the cloud provider before removing the taint on the node - if nodeIP, ok := ensureNodeProvidedIPExists(node, nodeAddresses); ok { - if nodeIP == nil { - return nil, errors.New("failed to find kubelet node IP from cloud provider") - } + if nodeIP, ok := ensureNodeProvidedIPExists(node, instanceMeta.NodeAddresses); ok && nodeIP == nil { + return nil, errors.New("failed to find kubelet node IP from cloud provider") } - if instanceType, err := getInstanceTypeByProviderIDOrName(ctx, instances, providerID, node.Name); err != nil { - return nil, err - } else if instanceType != "" { - klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceType) - klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceType) + if instanceMeta.Type != "" { + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceType, instanceMeta.Type) + klog.V(2).Infof("Adding node label from cloud provider: %s=%s", v1.LabelInstanceTypeStable, instanceMeta.Type) nodeModifiers = append(nodeModifiers, func(n *v1.Node) { if n.Labels == nil { n.Labels = map[string]string{} } - n.Labels[v1.LabelInstanceType] = instanceType - n.Labels[v1.LabelInstanceTypeStable] = instanceType + n.Labels[v1.LabelInstanceType] = instanceMeta.Type + n.Labels[v1.LabelInstanceTypeStable] = instanceMeta.Type }) } diff --git a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go index d2c0beefaa7..3f3f402b944 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/node/node_controller_test.go @@ -171,6 +171,7 @@ func Test_AddCloudNode(t *testing.T) { { name: "node initialized with provider ID", fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: false, InstanceTypes: map[types.NodeName]string{ types.NodeName("node0"): "t1.micro", }, @@ -223,6 +224,7 @@ func Test_AddCloudNode(t *testing.T) { { name: "node ignored", fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: false, InstanceTypes: map[types.NodeName]string{ types.NodeName("node0"): "t1.micro", types.NodeName("fake://12345"): "t1.micro", @@ -262,6 +264,7 @@ func Test_AddCloudNode(t *testing.T) { { name: "zone/region topology labels added", fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: false, InstanceTypes: map[types.NodeName]string{ types.NodeName("node0"): "t1.micro", }, @@ -345,7 +348,8 @@ func Test_AddCloudNode(t *testing.T) { { name: "node addresses", fakeCloud: &fakecloud.Cloud{ - InstanceTypes: map[types.NodeName]string{}, + EnableInstancesV2: false, + InstanceTypes: map[types.NodeName]string{}, ExtID: map[types.NodeName]string{ types.NodeName("node0"): "12345", }, @@ -441,6 +445,7 @@ func Test_AddCloudNode(t *testing.T) { { name: "provided node IP address", fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: false, Addresses: []v1.NodeAddress{ { Type: v1.NodeInternalIP, @@ -541,7 +546,8 @@ func Test_AddCloudNode(t *testing.T) { { name: "provider ID already set", fakeCloud: &fakecloud.Cloud{ - InstanceTypes: map[types.NodeName]string{}, + EnableInstancesV2: false, + InstanceTypes: map[types.NodeName]string{}, Addresses: []v1.NodeAddress{ { Type: v1.NodeHostName, @@ -622,9 +628,565 @@ func Test_AddCloudNode(t *testing.T) { { name: "provider ID not implemented", fakeCloud: &fakecloud.Cloud{ - InstanceTypes: map[types.NodeName]string{}, - Provider: "test", - ExtID: map[types.NodeName]string{}, + EnableInstancesV2: false, + InstanceTypes: map[types.NodeName]string{}, + Provider: "test", + ExtID: map[types.NodeName]string{}, + ExtIDErr: map[types.NodeName]error{ + types.NodeName("node0"): cloudprovider.NotImplemented, + }, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + }, + }, + { + name: "[instanceV2] node initialized with provider ID", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{ + types.NodeName("node0"): "t1.micro", + }, + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + ErrByProviderID: nil, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "fake://12345", + }, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + }, + }, + }, + { + name: "[instanceV2] node ignored", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{ + types.NodeName("node0"): "t1.micro", + types.NodeName("fake://12345"): "t1.micro", + }, + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + }, + }, + { + name: "[instanceV2] zone/region topology labels added", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{ + types.NodeName("node0"): "t1.micro", + }, + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + Provider: "aws", + Zone: cloudprovider.Zone{ + FailureDomain: "us-west-1a", + Region: "us-west", + }, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{ + "failure-domain.beta.kubernetes.io/region": "us-west", + "failure-domain.beta.kubernetes.io/zone": "us-west-1a", + "topology.kubernetes.io/region": "us-west", + "topology.kubernetes.io/zone": "us-west-1a", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + }, + Spec: v1.NodeSpec{ + ProviderID: "aws://12345", + }, + }, + }, + { + name: "[instanceV2] node addresses", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{}, + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + ExistsByProviderID: true, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{}, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "fake://12345", + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + }, + { + name: "[instanceV2] provided node IP address", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + ExistsByProviderID: true, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Annotations: map[string]string{ + cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + ProviderID: "node0.aws.12345", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Annotations: map[string]string{ + cloudproviderapi.AnnotationAlphaProvidedIPAddr: "10.0.0.1", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + ProviderID: "node0.aws.12345", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + }, + }, + }, + }, + { + name: "[instanceV2] provider ID already set", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{}, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + ExistsByProviderID: false, + Err: nil, + }, + existingNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + ProviderID: "test-provider-id", + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + updatedNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + }, + Spec: v1.NodeSpec{ + ProviderID: "test-provider-id", + Taints: []v1.Taint{ + { + Key: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + }, + { + name: "[instanceV2] provider ID not implemented", + fakeCloud: &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{}, + Provider: "test", + ExtID: map[types.NodeName]string{}, ExtIDErr: map[types.NodeName]error{ types.NodeName("node0"): cloudprovider.NotImplemented, }, @@ -706,6 +1268,87 @@ func Test_AddCloudNode(t *testing.T) { } } +// test AddCloudNode with instanceV2, same test case with TestGCECondition. +func TestGCEConditionV2(t *testing.T) { + existingNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + } + + fakeCloud := &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{ + types.NodeName("node0"): "t1.micro", + }, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + Provider: "gce", + Err: nil, + } + + clientset := fake.NewSimpleClientset(existingNode) + factory := informers.NewSharedInformerFactory(clientset, 0) + + eventBroadcaster := record.NewBroadcaster() + cloudNodeController := &CloudNodeController{ + kubeClient: clientset, + nodeInformer: factory.Core().V1().Nodes(), + cloud: fakeCloud, + recorder: eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-node-controller"}), + nodeStatusUpdateFrequency: 1 * time.Second, + } + eventBroadcaster.StartLogging(t.Logf) + + cloudNodeController.AddCloudNode(context.TODO(), existingNode) + + updatedNode, err := clientset.CoreV1().Nodes().Get(context.TODO(), existingNode.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("error getting updated nodes: %v", err) + } + + conditionAdded := false + for _, cond := range updatedNode.Status.Conditions { + if cond.Status == "True" && cond.Type == "NetworkUnavailable" && cond.Reason == "NoRouteCreated" { + conditionAdded = true + } + } + + assert.True(t, conditionAdded, "Network Route Condition for GCE not added by external cloud initializer") +} + // This test checks that a node with the external cloud provider taint is cloudprovider initialized and // the GCE route condition is added if cloudprovider is GCE func TestGCECondition(t *testing.T) { @@ -736,6 +1379,7 @@ func TestGCECondition(t *testing.T) { } fakeCloud := &fakecloud.Cloud{ + EnableInstancesV2: false, InstanceTypes: map[types.NodeName]string{ types.NodeName("node0"): "t1.micro", }, @@ -1028,6 +1672,77 @@ func TestNodeAddressesChangeDetected(t *testing.T) { "Node address changes are not detected correctly") } +// Test updateNodeAddress with instanceV2, same test case with TestNodeAddressesNotUpdate. +func TestNodeAddressesNotUpdateV2(t *testing.T) { + existingNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionUnknown, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: cloudproviderapi.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + } + + clientset := fake.NewSimpleClientset(existingNode) + factory := informers.NewSharedInformerFactory(clientset, 0) + + fakeCloud := &fakecloud.Cloud{ + EnableInstancesV2: true, + InstanceTypes: map[types.NodeName]string{}, + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeHostName, + Address: "node0.cloud.internal", + }, + { + Type: v1.NodeInternalIP, + Address: "10.0.0.1", + }, + { + Type: v1.NodeExternalIP, + Address: "132.143.154.163", + }, + }, + ExistsByProviderID: false, + Err: nil, + } + + cloudNodeController := &CloudNodeController{ + kubeClient: clientset, + nodeInformer: factory.Core().V1().Nodes(), + cloud: fakeCloud, + } + + cloudNodeController.updateNodeAddress(context.TODO(), existingNode) + + updatedNode, err := clientset.CoreV1().Nodes().Get(context.TODO(), existingNode.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("error getting updated nodes: %v", err) + } + + if len(updatedNode.Status.Addresses) > 0 { + t.Errorf("Node addresses should not be updated") + } +} + // This test checks that a node with the external cloud provider taint is cloudprovider initialized and // and node addresses will not be updated when node isn't present according to the cloudprovider func TestNodeAddressesNotUpdate(t *testing.T) { @@ -1062,7 +1777,8 @@ func TestNodeAddressesNotUpdate(t *testing.T) { factory := informers.NewSharedInformerFactory(clientset, 0) fakeCloud := &fakecloud.Cloud{ - InstanceTypes: map[types.NodeName]string{}, + EnableInstancesV2: false, + InstanceTypes: map[types.NodeName]string{}, Addresses: []v1.NodeAddress{ { Type: v1.NodeHostName, @@ -1087,7 +1803,7 @@ func TestNodeAddressesNotUpdate(t *testing.T) { cloud: fakeCloud, } - cloudNodeController.updateNodeAddress(context.TODO(), existingNode, fakeCloud) + cloudNodeController.updateNodeAddress(context.TODO(), existingNode) updatedNode, err := clientset.CoreV1().Nodes().Get(context.TODO(), existingNode.Name, metav1.GetOptions{}) if err != nil {