From 2d54ba3e0f93a1dafce21f3f1c209711b7a3c0e0 Mon Sep 17 00:00:00 2001 From: Aleksandra Malinowska Date: Fri, 16 Feb 2018 12:24:27 +0100 Subject: [PATCH] Revert "add node shutdown taint" --- pkg/cloudprovider/cloud.go | 2 - pkg/cloudprovider/providers/aws/aws.go | 5 - .../providers/azure/azure_instances.go | 5 - .../cloudstack/cloudstack_instances.go | 5 - .../providers/cloudstack/metadata.go | 5 - pkg/cloudprovider/providers/fake/fake.go | 12 +- .../providers/gce/gce_instances.go | 5 - .../openstack/openstack_instances.go | 8 +- pkg/cloudprovider/providers/ovirt/ovirt.go | 5 - pkg/cloudprovider/providers/photon/photon.go | 5 - .../providers/vsphere/vsphere.go | 5 - pkg/controller/cloud/node_controller.go | 44 +------ pkg/controller/cloud/node_controller_test.go | 109 ---------------- .../node_lifecycle_controller.go | 47 +------ .../node_lifecycle_controller_test.go | 118 ------------------ pkg/controller/testutil/test_utils.go | 4 - pkg/controller/util/node/controller_utils.go | 15 --- pkg/scheduler/algorithm/well_known_labels.go | 3 - pkg/volume/cinder/attacher_test.go | 4 - 19 files changed, 13 insertions(+), 393 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index 1acd7328752..9ca91ebf317 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -148,8 +148,6 @@ 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 6e9b5ac718a..419e3884b28 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -1337,11 +1337,6 @@ 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 2f69f381794..1b85334e1aa 100644 --- a/pkg/cloudprovider/providers/azure/azure_instances.go +++ b/pkg/cloudprovider/providers/azure/azure_instances.go @@ -103,11 +103,6 @@ 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 20f98a31e25..cf8559a2b0b 100644 --- a/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go +++ b/pkg/cloudprovider/providers/cloudstack/cloudstack_instances.go @@ -158,8 +158,3 @@ 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 69b5bd13517..bffeb49cf9e 100644 --- a/pkg/cloudprovider/providers/cloudstack/metadata.go +++ b/pkg/cloudprovider/providers/cloudstack/metadata.go @@ -119,11 +119,6 @@ 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 582bb7c5d42..6b4ee682562 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -50,10 +50,8 @@ type FakeCloud struct { Exists bool Err error - ExistsByProviderID bool - ErrByProviderID error - NodeShutdown bool - ErrShutdownByProviderID error + ExistsByProviderID bool + ErrByProviderID error Calls []string Addresses []v1.NodeAddress @@ -243,12 +241,6 @@ 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 d52fe06414f..aa31f5e3a3e 100644 --- a/pkg/cloudprovider/providers/gce/gce_instances.go +++ b/pkg/cloudprovider/providers/gce/gce_instances.go @@ -190,11 +190,6 @@ 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 a6ef7f434c1..b8ea557e674 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_instances.go +++ b/pkg/cloudprovider/providers/openstack/openstack_instances.go @@ -141,11 +141,6 @@ 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 { @@ -160,8 +155,7 @@ 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) { - // we should fetch instanceid from all states instead of ACTIVE - srv, err := getServerByName(i.compute, name, false) + srv, err := getServerByName(i.compute, name, true) 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 2e4e35c62b4..708358eec6d 100644 --- a/pkg/cloudprovider/providers/ovirt/ovirt.go +++ b/pkg/cloudprovider/providers/ovirt/ovirt.go @@ -212,11 +212,6 @@ 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 2fa2951a1b5..424b6f91af6 100644 --- a/pkg/cloudprovider/providers/photon/photon.go +++ b/pkg/cloudprovider/providers/photon/photon.go @@ -477,11 +477,6 @@ 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 d4482e395c4..4835306d156 100644 --- a/pkg/cloudprovider/providers/vsphere/vsphere.go +++ b/pkg/cloudprovider/providers/vsphere/vsphere.go @@ -609,11 +609,6 @@ 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 e1c3bf70e0d..de20921bc9e 100644 --- a/pkg/controller/cloud/node_controller.go +++ b/pkg/controller/cloud/node_controller.go @@ -37,23 +37,16 @@ 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} - - ShutDownTaint = &v1.Taint{ - Key: algorithm.TaintNodeShutdown, - Effect: v1.TaintEffectNoSchedule, - } -) +var UpdateNodeSpecBackoff = wait.Backoff{ + Steps: 20, + Duration: 50 * time.Millisecond, + Jitter: 1.0, +} type CloudNodeController struct { nodeInformer coreinformers.NodeInformer @@ -250,28 +243,9 @@ 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 @@ -301,12 +275,6 @@ 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 805e7373fe9..a8faf8352f4 100644 --- a/pkg/controller/cloud/node_controller_test.go +++ b/pkg/controller/cloud/node_controller_test.go @@ -148,115 +148,6 @@ 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 6de3fd14a65..953d9246c33 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller.go @@ -79,11 +79,6 @@ 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, @@ -156,10 +151,9 @@ type Controller struct { daemonSetStore extensionslisters.DaemonSetLister daemonSetInformerSynced cache.InformerSynced - nodeLister corelisters.NodeLister - nodeInformerSynced cache.InformerSynced - nodeExistsInCloudProvider func(types.NodeName) (bool, error) - nodeShutdownInCloudProvider func(types.NodeName) (bool, error) + nodeLister corelisters.NodeLister + nodeInformerSynced cache.InformerSynced + nodeExistsInCloudProvider func(types.NodeName) (bool, error) recorder record.EventRecorder @@ -245,9 +239,6 @@ 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, @@ -662,11 +653,6 @@ 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. @@ -680,21 +666,7 @@ 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 { - // 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)) + 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 @@ -1130,17 +1102,6 @@ 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 6cba4870f5c..365f31dc998 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -1360,118 +1360,6 @@ 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. @@ -1516,9 +1404,6 @@ 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) @@ -2357,9 +2242,6 @@ 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 11bfb66366c..1ecada1f6f2 100644 --- a/pkg/controller/testutil/test_utils.go +++ b/pkg/controller/testutil/test_utils.go @@ -67,7 +67,6 @@ type FakeNodeHandler struct { // Synchronization lock sync.Mutex DeleteWaitChan chan struct{} - PatchWaitChan chan struct{} } // FakeLegacyHandler is a fake implemtation of CoreV1Interface. @@ -271,9 +270,6 @@ 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 a286cdc4de3..b2c0ec23cf5 100644 --- a/pkg/controller/util/node/controller_utils.go +++ b/pkg/controller/util/node/controller_utils.go @@ -187,21 +187,6 @@ 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 6a246bdcaf3..578cafb7e66 100644 --- a/pkg/scheduler/algorithm/well_known_labels.go +++ b/pkg/scheduler/algorithm/well_known_labels.go @@ -61,7 +61,4 @@ 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 bef42dcaa24..b75fdef561d 100644 --- a/pkg/volume/cinder/attacher_test.go +++ b/pkg/volume/cinder/attacher_test.go @@ -732,10 +732,6 @@ 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") }