diff --git a/pkg/controller/cloud/node_controller.go b/pkg/controller/cloud/node_controller.go index bb470d41bc4..4ee4816d54b 100644 --- a/pkg/controller/cloud/node_controller.go +++ b/pkg/controller/cloud/node_controller.go @@ -284,6 +284,18 @@ func (cnc *CloudNodeController) AddCloudNode(obj interface{}) { return err } + if curNode.Spec.ProviderID == "" { + providerID, err := cloudprovider.GetInstanceProviderID(cnc.cloud, types.NodeName(curNode.Name)) + if err == nil { + curNode.Spec.ProviderID = providerID + } else { + // we should attempt to set providerID on curNode, but + // we can continue if we fail since we will attempt to set + // node addresses given the node name in getNodeAddressesByProviderIDOrName + glog.Errorf("failed to set node provider id: %v", err) + } + } + nodeAddresses, err := getNodeAddressesByProviderIDOrName(instances, curNode) if err != nil { glog.Errorf("%v", err) diff --git a/pkg/controller/cloud/node_controller_test.go b/pkg/controller/cloud/node_controller_test.go index 033dbe90889..6f054432b9f 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -804,3 +804,179 @@ func TestNodeAddressesChangeDetected(t *testing.T) { t.Errorf("Node address changes are not detected correctly") } } + +// This test checks that a node is set with the correct providerID +func TestNodeProviderID(t *testing.T) { + fnh := &testutil.FakeNodeHandler{ + Existing: []*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: "ImproveCoverageTaint", + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: algorithm.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + }, + Clientset: fake.NewSimpleClientset(&v1.PodList{}), + DeleteWaitChan: make(chan struct{}), + } + + factory := informers.NewSharedInformerFactory(fnh, controller.NoResyncPeriodFunc()) + + fakeCloud := &fakecloud.FakeCloud{ + 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", + }, + }, + Provider: "test", + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Err: nil, + } + + eventBroadcaster := record.NewBroadcaster() + cloudNodeController := &CloudNodeController{ + kubeClient: fnh, + nodeInformer: factory.Core().V1().Nodes(), + cloud: fakeCloud, + nodeMonitorPeriod: 5 * time.Second, + nodeStatusUpdateFrequency: 1 * time.Second, + recorder: eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-controller-manager"}), + } + eventBroadcaster.StartLogging(glog.Infof) + + cloudNodeController.AddCloudNode(fnh.Existing[0]) + + if len(fnh.UpdatedNodes) != 1 || fnh.UpdatedNodes[0].Name != "node0" { + t.Errorf("Node was not updated") + } + + if fnh.UpdatedNodes[0].Spec.ProviderID != "test://12345" { + t.Errorf("Node ProviderID not set correctly") + } +} + +// This test checks that a node's provider ID will not be overwritten +func TestNodeProviderIDAlreadySet(t *testing.T) { + fnh := &testutil.FakeNodeHandler{ + Existing: []*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: algorithm.TaintExternalCloudProvider, + Value: "true", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + }, + Clientset: fake.NewSimpleClientset(&v1.PodList{}), + DeleteWaitChan: make(chan struct{}), + } + + factory := informers.NewSharedInformerFactory(fnh, controller.NoResyncPeriodFunc()) + + fakeCloud := &fakecloud.FakeCloud{ + 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", + }, + }, + Provider: "test", + ExtID: map[types.NodeName]string{ + types.NodeName("node0"): "12345", + }, + Err: nil, + } + + eventBroadcaster := record.NewBroadcaster() + cloudNodeController := &CloudNodeController{ + kubeClient: fnh, + nodeInformer: factory.Core().V1().Nodes(), + cloud: fakeCloud, + nodeMonitorPeriod: 5 * time.Second, + nodeStatusUpdateFrequency: 1 * time.Second, + recorder: eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-controller-manager"}), + } + eventBroadcaster.StartLogging(glog.Infof) + + cloudNodeController.AddCloudNode(fnh.Existing[0]) + + if len(fnh.UpdatedNodes) != 1 || fnh.UpdatedNodes[0].Name != "node0" { + t.Errorf("Node was not updated") + } + + // CCM node controller should not overwrite provider if it's already set + if fnh.UpdatedNodes[0].Spec.ProviderID != "test-provider-id" { + t.Errorf("Node ProviderID not set correctly") + } +}