diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 9ca91ebf317..1acd7328752 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -148,6 +148,8 @@ type Instances interface { // InstanceExistsByProviderID returns true if the instance for the given provider id still is running. // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager. InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) + // InstanceShutdownByProviderID returns true if the instance is shutdown in cloudprovider + InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) } // Route is a representation of an advanced routing rule. diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 419e3884b28..6e9b5ac718a 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -1337,6 +1337,11 @@ func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID strin return true, nil } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the node with the specified nodeName. func (c *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) { // In the future it is possible to also return an endpoint as: diff --git a/pkg/cloudprovider/providers/azure/azure_instances.go b/pkg/cloudprovider/providers/azure/azure_instances.go index 1b85334e1aa..2f69f381794 100644 --- a/pkg/cloudprovider/providers/azure/azure_instances.go +++ b/pkg/cloudprovider/providers/azure/azure_instances.go @@ -103,6 +103,11 @@ func (az *Cloud) isCurrentInstance(name types.NodeName) (bool, error) { return (metadataName == nodeName), err } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (az *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the specified instance. // Note that if the instance does not exist or is no longer running, we must return ("", cloudprovider.InstanceNotFound) func (az *Cloud) InstanceID(ctx context.Context, name types.NodeName) (string, error) { diff --git a/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go b/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go index cf8559a2b0b..20f98a31e25 100644 --- a/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go +++ b/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go @@ -158,3 +158,8 @@ func (cs *CSCloud) InstanceExistsByProviderID(ctx context.Context, providerID st return true, nil } + +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (cs *CSCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} diff --git a/pkg/cloudprovider/providers/cloudstack/metadata.go b/pkg/cloudprovider/providers/cloudstack/metadata.go index bffeb49cf9e..69b5bd13517 100644 --- a/pkg/cloudprovider/providers/cloudstack/metadata.go +++ b/pkg/cloudprovider/providers/cloudstack/metadata.go @@ -119,6 +119,11 @@ func (m *metadata) InstanceExistsByProviderID(ctx context.Context, providerID st return false, errors.New("InstanceExistsByProviderID not implemented") } +// InstanceShutdownByProviderID returns if the instance is shutdown. +func (m *metadata) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // GetZone returns the Zone containing the region that the program is running in. func (m *metadata) GetZone(ctx context.Context) (cloudprovider.Zone, error) { zone := cloudprovider.Zone{} diff --git a/pkg/cloudprovider/providers/fake/fake.go b/pkg/cloudprovider/providers/fake/fake.go index 6b4ee682562..582bb7c5d42 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -50,8 +50,10 @@ type FakeCloud struct { Exists bool Err error - ExistsByProviderID bool - ErrByProviderID error + ExistsByProviderID bool + ErrByProviderID error + NodeShutdown bool + ErrShutdownByProviderID error Calls []string Addresses []v1.NodeAddress @@ -241,6 +243,12 @@ func (f *FakeCloud) InstanceExistsByProviderID(ctx context.Context, providerID s return f.ExistsByProviderID, f.ErrByProviderID } +// InstanceShutdownByProviderID returns true if the instances is in safe state to detach volumes +func (f *FakeCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + f.addCall("instance-shutdown-by-provider-id") + return f.NodeShutdown, f.ErrShutdownByProviderID +} + // List is a test-spy implementation of Instances.List. // It adds an entry "list" into the internal method call record. func (f *FakeCloud) List(filter string) ([]types.NodeName, error) { diff --git a/pkg/cloudprovider/providers/gce/gce_instances.go b/pkg/cloudprovider/providers/gce/gce_instances.go index aa31f5e3a3e..d52fe06414f 100644 --- a/pkg/cloudprovider/providers/gce/gce_instances.go +++ b/pkg/cloudprovider/providers/gce/gce_instances.go @@ -190,6 +190,11 @@ func (gce *GCECloud) InstanceExistsByProviderID(ctx context.Context, providerID return true, nil } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (gce *GCECloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the node with the specified NodeName. func (gce *GCECloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) { instanceName := mapNodeNameToInstanceName(nodeName) diff --git a/pkg/cloudprovider/providers/openstack/openstack_instances.go b/pkg/cloudprovider/providers/openstack/openstack_instances.go index b8ea557e674..a6ef7f434c1 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_instances.go +++ b/pkg/cloudprovider/providers/openstack/openstack_instances.go @@ -141,6 +141,11 @@ func (i *Instances) InstanceExistsByProviderID(ctx context.Context, providerID s return true, nil } +// InstanceShutdownByProviderID returns true if the instances is in safe state to detach volumes +func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the kubelet's cloud provider ID. func (os *OpenStack) InstanceID() (string, error) { if len(os.localInstanceID) == 0 { @@ -155,7 +160,8 @@ func (os *OpenStack) InstanceID() (string, error) { // InstanceID returns the cloud provider ID of the specified instance. func (i *Instances) InstanceID(ctx context.Context, name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name, true) + // we should fetch instanceid from all states instead of ACTIVE + srv, err := getServerByName(i.compute, name, false) if err != nil { if err == ErrNotFound { return "", cloudprovider.InstanceNotFound diff --git a/pkg/cloudprovider/providers/ovirt/ovirt.go b/pkg/cloudprovider/providers/ovirt/ovirt.go index 708358eec6d..2e4e35c62b4 100644 --- a/pkg/cloudprovider/providers/ovirt/ovirt.go +++ b/pkg/cloudprovider/providers/ovirt/ovirt.go @@ -212,6 +212,11 @@ func (v *OVirtCloud) InstanceExistsByProviderID(ctx context.Context, providerID return false, cloudprovider.NotImplemented } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (v *OVirtCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the node with the specified NodeName. func (v *OVirtCloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) { name := mapNodeNameToInstanceName(nodeName) diff --git a/pkg/cloudprovider/providers/photon/photon.go b/pkg/cloudprovider/providers/photon/photon.go index 424b6f91af6..2fa2951a1b5 100644 --- a/pkg/cloudprovider/providers/photon/photon.go +++ b/pkg/cloudprovider/providers/photon/photon.go @@ -477,6 +477,11 @@ func (pc *PCCloud) InstanceExistsByProviderID(ctx context.Context, providerID st return false, cloudprovider.NotImplemented } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (pc *PCCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the specified instance. func (pc *PCCloud) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) (string, error) { name := string(nodeName) diff --git a/pkg/cloudprovider/providers/vsphere/vsphere.go b/pkg/cloudprovider/providers/vsphere/vsphere.go index 4835306d156..d4482e395c4 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -609,6 +609,11 @@ func (vs *VSphere) InstanceExistsByProviderID(ctx context.Context, providerID st return false, err } +// InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes +func (vs *VSphere) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, cloudprovider.NotImplemented +} + // InstanceID returns the cloud provider ID of the node with the specified Name. func (vs *VSphere) InstanceID(ctx context.Context, nodeName k8stypes.NodeName) (string, error) { diff --git a/pkg/controller/cloud/node_controller.go b/pkg/controller/cloud/node_controller.go index 9f1c86e7b94..74c09d53db3 100644 --- a/pkg/controller/cloud/node_controller.go +++ b/pkg/controller/cloud/node_controller.go @@ -37,16 +37,23 @@ import ( clientretry "k8s.io/client-go/util/retry" nodeutilv1 "k8s.io/kubernetes/pkg/api/v1/node" "k8s.io/kubernetes/pkg/cloudprovider" + "k8s.io/kubernetes/pkg/controller" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" "k8s.io/kubernetes/pkg/scheduler/algorithm" nodeutil "k8s.io/kubernetes/pkg/util/node" ) -var UpdateNodeSpecBackoff = wait.Backoff{ - Steps: 20, - Duration: 50 * time.Millisecond, - Jitter: 1.0, -} +var ( + UpdateNodeSpecBackoff = wait.Backoff{ + Steps: 20, + Duration: 50 * time.Millisecond, + Jitter: 1.0} + + ShutDownTaint = &v1.Taint{ + Key: algorithm.TaintNodeShutdown, + Effect: v1.TaintEffectNoSchedule, + } +) type CloudNodeController struct { nodeInformer coreinformers.NodeInformer @@ -240,9 +247,28 @@ func (cnc *CloudNodeController) MonitorNode() { // from the cloud provider. If node cannot be found in cloudprovider, then delete the node immediately if currentReadyCondition != nil { if currentReadyCondition.Status != v1.ConditionTrue { + // we need to check this first to get taint working in similar in all cloudproviders + // current problem is that shutdown nodes are not working in similar way ie. all cloudproviders + // does not delete node from kubernetes cluster when instance it is shutdown see issue #46442 + exists, err := instances.InstanceShutdownByProviderID(context.TODO(), node.Spec.ProviderID) + if err != nil && err != cloudprovider.NotImplemented { + glog.Errorf("Error getting data for node %s from cloud: %v", node.Name, err) + continue + } + + if exists { + // if node is shutdown add shutdown taint + err = controller.AddOrUpdateTaintOnNode(cnc.kubeClient, node.Name, ShutDownTaint) + if err != nil { + glog.Errorf("Error patching node taints: %v", err) + } + // Continue checking the remaining nodes since the current one is fine. + continue + } + // Check with the cloud provider to see if the node still exists. If it // doesn't, delete the node immediately. - exists, err := ensureNodeExistsByProviderIDOrExternalID(instances, node) + exists, err = ensureNodeExistsByProviderIDOrExternalID(instances, node) if err != nil { glog.Errorf("Error getting data for node %s from cloud: %v", node.Name, err) continue @@ -272,6 +298,12 @@ func (cnc *CloudNodeController) MonitorNode() { } }(node.Name) + } else { + // if taint exist remove taint + err = controller.RemoveTaintOffNode(cnc.kubeClient, node.Name, node, ShutDownTaint) + if err != nil { + glog.Errorf("Error patching node taints: %v", err) + } } } } diff --git a/pkg/controller/cloud/node_controller_test.go b/pkg/controller/cloud/node_controller_test.go index a8faf8352f4..805e7373fe9 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -148,6 +148,115 @@ func TestEnsureNodeExistsByProviderIDOrNodeName(t *testing.T) { } +func TestNodeShutdown(t *testing.T) { + + testCases := []struct { + testName string + node *v1.Node + existsByProviderID bool + shutdown bool + }{ + { + testName: "node shutdowned add taint", + existsByProviderID: true, + shutdown: true, + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + 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), + }, + }, + }, + }, + }, + { + testName: "node started after shutdown remove taint", + existsByProviderID: true, + shutdown: false, + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + Taints: []v1.Taint{ + { + Key: algorithm.TaintNodeShutdown, + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + fc := &fakecloud.FakeCloud{ + ExistsByProviderID: tc.existsByProviderID, + NodeShutdown: tc.shutdown, + } + fnh := &testutil.FakeNodeHandler{ + Existing: []*v1.Node{tc.node}, + Clientset: fake.NewSimpleClientset(), + PatchWaitChan: make(chan struct{}), + } + + factory := informers.NewSharedInformerFactory(fnh, controller.NoResyncPeriodFunc()) + + eventBroadcaster := record.NewBroadcaster() + cloudNodeController := &CloudNodeController{ + kubeClient: fnh, + nodeInformer: factory.Core().V1().Nodes(), + cloud: fc, + nodeMonitorPeriod: 1 * time.Second, + recorder: eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-node-controller"}), + nodeStatusUpdateFrequency: 1 * time.Second, + } + eventBroadcaster.StartLogging(glog.Infof) + + cloudNodeController.Run() + + select { + case <-fnh.PatchWaitChan: + case <-time.After(1 * time.Second): + t.Errorf("Timed out waiting %v for node to be updated", wait.ForeverTestTimeout) + } + + assert.Equal(t, 1, len(fnh.UpdatedNodes), "Node was not updated") + if tc.shutdown { + assert.Equal(t, 1, len(fnh.UpdatedNodes[0].Spec.Taints), "Node Taint was not added") + assert.Equal(t, "node.cloudprovider.kubernetes.io/shutdown", fnh.UpdatedNodes[0].Spec.Taints[0].Key, "Node Taint key is not correct") + } else { + assert.Equal(t, 0, len(fnh.UpdatedNodes[0].Spec.Taints), "Node Taint was not removed after node is back in ready state") + } + + }) + } + +} + // This test checks that the node is deleted when kubelet stops reporting // and cloud provider says node is gone func TestNodeDeleted(t *testing.T) { diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller.go b/pkg/controller/nodelifecycle/node_lifecycle_controller.go index 953d9246c33..6de3fd14a65 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller.go @@ -79,6 +79,11 @@ var ( Effect: v1.TaintEffectNoExecute, } + shutDownTaint = &v1.Taint{ + Key: algorithm.TaintNodeShutdown, + Effect: v1.TaintEffectNoSchedule, + } + nodeConditionToTaintKeyMap = map[v1.NodeConditionType]string{ v1.NodeMemoryPressure: algorithm.TaintNodeMemoryPressure, v1.NodeOutOfDisk: algorithm.TaintNodeOutOfDisk, @@ -151,9 +156,10 @@ type Controller struct { daemonSetStore extensionslisters.DaemonSetLister daemonSetInformerSynced cache.InformerSynced - nodeLister corelisters.NodeLister - nodeInformerSynced cache.InformerSynced - nodeExistsInCloudProvider func(types.NodeName) (bool, error) + nodeLister corelisters.NodeLister + nodeInformerSynced cache.InformerSynced + nodeExistsInCloudProvider func(types.NodeName) (bool, error) + nodeShutdownInCloudProvider func(types.NodeName) (bool, error) recorder record.EventRecorder @@ -239,6 +245,9 @@ func NewNodeLifecycleController(podInformer coreinformers.PodInformer, nodeExistsInCloudProvider: func(nodeName types.NodeName) (bool, error) { return nodeutil.ExistsInCloudProvider(cloud, nodeName) }, + nodeShutdownInCloudProvider: func(nodeName types.NodeName) (bool, error) { + return nodeutil.ShutdownInCloudProvider(cloud, nodeName) + }, recorder: recorder, nodeMonitorPeriod: nodeMonitorPeriod, nodeStartupGracePeriod: nodeStartupGracePeriod, @@ -653,6 +662,11 @@ func (nc *Controller) monitorNodeStatus() error { glog.V(2).Infof("Node %s is ready again, cancelled pod eviction", node.Name) } } + // remove shutdown taint this is needed always depending do we use taintbased or not + err := nc.markNodeAsNotShutdown(node) + if err != nil { + glog.Errorf("Failed to remove taints from node %v. Will retry in next iteration.", node.Name) + } } // Report node event. @@ -666,7 +680,21 @@ func (nc *Controller) monitorNodeStatus() error { // Check with the cloud provider to see if the node still exists. If it // doesn't, delete the node immediately. if currentReadyCondition.Status != v1.ConditionTrue && nc.cloud != nil { - exists, err := nc.nodeExistsInCloudProvider(types.NodeName(node.Name)) + // check is node shutdowned, if yes do not deleted it. Instead add taint + exists, err := nc.nodeShutdownInCloudProvider(types.NodeName(node.Name)) + if err != nil && err != cloudprovider.NotImplemented { + glog.Errorf("Error determining if node %v shutdown in cloud: %v", node.Name, err) + continue + } + // node shutdown + if exists { + err = controller.AddOrUpdateTaintOnNode(nc.kubeClient, node.Name, shutDownTaint) + if err != nil { + glog.Errorf("Error patching node taints: %v", err) + } + continue + } + exists, err = nc.nodeExistsInCloudProvider(types.NodeName(node.Name)) if err != nil { glog.Errorf("Error determining if node %v exists in cloud: %v", node.Name, err) continue @@ -1102,6 +1130,17 @@ func (nc *Controller) markNodeAsReachable(node *v1.Node) (bool, error) { return nc.zoneNoExecuteTainter[utilnode.GetZoneKey(node)].Remove(node.Name), nil } +func (nc *Controller) markNodeAsNotShutdown(node *v1.Node) error { + nc.evictorLock.Lock() + defer nc.evictorLock.Unlock() + err := controller.RemoveTaintOffNode(nc.kubeClient, node.Name, node, shutDownTaint) + if err != nil { + glog.Errorf("Failed to remove taint from node %v: %v", node.Name, err) + return err + } + return nil +} + // ComputeZoneState returns a slice of NodeReadyConditions for all Nodes in a given zone. // The zone is considered: // - fullyDisrupted if there're no Ready Nodes, diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go index 365f31dc998..6cba4870f5c 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -1360,6 +1360,118 @@ func TestMonitorNodeStatusEvictPodsWithDisruption(t *testing.T) { } } +func TestCloudProviderNodeShutdown(t *testing.T) { + + testCases := []struct { + testName string + node *v1.Node + shutdown bool + }{ + { + testName: "node shutdowned add taint", + shutdown: true, + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + }, + 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), + }, + }, + }, + }, + }, + { + testName: "node started after shutdown remove taint", + shutdown: false, + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node0", + CreationTimestamp: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), + }, + Spec: v1.NodeSpec{ + ProviderID: "node0", + Taints: []v1.Taint{ + { + Key: algorithm.TaintNodeShutdown, + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC), + }, + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + fnh := &testutil.FakeNodeHandler{ + Existing: []*v1.Node{tc.node}, + Clientset: fake.NewSimpleClientset(), + } + nodeController, _ := newNodeLifecycleControllerFromClient( + nil, + fnh, + 10*time.Minute, + testRateLimiterQPS, + testRateLimiterQPS, + testLargeClusterThreshold, + testUnhealthyThreshold, + testNodeMonitorGracePeriod, + testNodeStartupGracePeriod, + testNodeMonitorPeriod, + false) + nodeController.cloud = &fakecloud.FakeCloud{} + nodeController.now = func() metav1.Time { return metav1.Date(2016, 1, 1, 12, 0, 0, 0, time.UTC) } + nodeController.recorder = testutil.NewFakeRecorder() + nodeController.nodeShutdownInCloudProvider = func(nodeName types.NodeName) (bool, error) { + return tc.shutdown, nil + } + + if err := nodeController.syncNodeStore(fnh); err != nil { + t.Errorf("unexpected error: %v", err) + } + if err := nodeController.monitorNodeStatus(); err != nil { + t.Errorf("unexpected error: %v", err) + } + + if len(fnh.UpdatedNodes) != 1 { + t.Errorf("Node was not updated") + } + if tc.shutdown { + if len(fnh.UpdatedNodes[0].Spec.Taints) != 1 { + t.Errorf("Node Taint was not added") + } + if fnh.UpdatedNodes[0].Spec.Taints[0].Key != "node.cloudprovider.kubernetes.io/shutdown" { + t.Errorf("Node Taint key is not correct") + } + } else { + if len(fnh.UpdatedNodes[0].Spec.Taints) != 0 { + t.Errorf("Node Taint was not removed after node is back in ready state") + } + } + }) + } + +} + // TestCloudProviderNoRateLimit tests that monitorNodes() immediately deletes // pods and the node when kubelet has not reported, and the cloudprovider says // the node is gone. @@ -1404,6 +1516,9 @@ func TestCloudProviderNoRateLimit(t *testing.T) { nodeController.nodeExistsInCloudProvider = func(nodeName types.NodeName) (bool, error) { return false, nil } + nodeController.nodeShutdownInCloudProvider = func(nodeName types.NodeName) (bool, error) { + return false, nil + } // monitorNodeStatus should allow this node to be immediately deleted if err := nodeController.syncNodeStore(fnh); err != nil { t.Errorf("unexpected error: %v", err) @@ -2242,6 +2357,9 @@ func TestNodeEventGeneration(t *testing.T) { nodeController.nodeExistsInCloudProvider = func(nodeName types.NodeName) (bool, error) { return false, nil } + nodeController.nodeShutdownInCloudProvider = func(nodeName types.NodeName) (bool, error) { + return false, nil + } nodeController.now = func() metav1.Time { return fakeNow } fakeRecorder := testutil.NewFakeRecorder() nodeController.recorder = fakeRecorder diff --git a/pkg/controller/testutil/test_utils.go b/pkg/controller/testutil/test_utils.go index 1ecada1f6f2..11bfb66366c 100644 --- a/pkg/controller/testutil/test_utils.go +++ b/pkg/controller/testutil/test_utils.go @@ -67,6 +67,7 @@ type FakeNodeHandler struct { // Synchronization lock sync.Mutex DeleteWaitChan chan struct{} + PatchWaitChan chan struct{} } // FakeLegacyHandler is a fake implemtation of CoreV1Interface. @@ -270,6 +271,9 @@ func (m *FakeNodeHandler) Patch(name string, pt types.PatchType, data []byte, su m.lock.Lock() defer func() { m.RequestCount++ + if m.PatchWaitChan != nil { + m.PatchWaitChan <- struct{}{} + } m.lock.Unlock() }() var nodeCopy v1.Node diff --git a/pkg/controller/util/node/controller_utils.go b/pkg/controller/util/node/controller_utils.go index b2c0ec23cf5..a286cdc4de3 100644 --- a/pkg/controller/util/node/controller_utils.go +++ b/pkg/controller/util/node/controller_utils.go @@ -187,6 +187,21 @@ func ExistsInCloudProvider(cloud cloudprovider.Interface, nodeName types.NodeNam return true, nil } +// ShutdownInCloudProvider returns true if the node is shutdowned in +// cloud provider. +func ShutdownInCloudProvider(cloud cloudprovider.Interface, nodeName types.NodeName) (bool, error) { + instances, ok := cloud.Instances() + if !ok { + return false, fmt.Errorf("%v", ErrCloudInstance) + } + providerID, err := cloudprovider.GetInstanceProviderID(context.TODO(), cloud, nodeName) + if err != nil { + return false, err + } + shutdown, err := instances.InstanceShutdownByProviderID(context.TODO(), providerID) + return shutdown, err +} + // RecordNodeEvent records a event related to a node. func RecordNodeEvent(recorder record.EventRecorder, nodeName, nodeUID, eventtype, reason, event string) { ref := &v1.ObjectReference{ diff --git a/pkg/scheduler/algorithm/well_known_labels.go b/pkg/scheduler/algorithm/well_known_labels.go index 578cafb7e66..6a246bdcaf3 100644 --- a/pkg/scheduler/algorithm/well_known_labels.go +++ b/pkg/scheduler/algorithm/well_known_labels.go @@ -61,4 +61,7 @@ const ( // from the cloud-controller-manager intitializes this node, and then removes // the taint TaintExternalCloudProvider = "node.cloudprovider.kubernetes.io/uninitialized" + + // TaintNodeShutdown when node is shutdown in external cloud provider + TaintNodeShutdown = "node.cloudprovider.kubernetes.io/shutdown" ) diff --git a/pkg/volume/cinder/attacher_test.go b/pkg/volume/cinder/attacher_test.go index b75fdef561d..bef42dcaa24 100644 --- a/pkg/volume/cinder/attacher_test.go +++ b/pkg/volume/cinder/attacher_test.go @@ -732,6 +732,10 @@ func (instances *instances) InstanceExistsByProviderID(ctx context.Context, prov return false, errors.New("unimplemented") } +func (instances *instances) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { + return false, errors.New("unimplemented") +} + func (instances *instances) List(filter string) ([]types.NodeName, error) { return []types.NodeName{}, errors.New("Not implemented") }