Skip to content

Commit

Permalink
Do not dalete unknown uninitialized nodes
Browse files Browse the repository at this point in the history
And wait for 5 minutes uptime before process it.
  • Loading branch information
sergelogvinov committed Jun 6, 2022
1 parent c6f724f commit 8468b74
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
29 changes: 28 additions & 1 deletion controllers/nodelifecycle/node_lifecycle_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ package cloud
import (
"context"
"errors"
"strings"
"time"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -127,6 +128,11 @@ func (c *CloudNodeLifecycleController) MonitorNodes(ctx context.Context) {
}

for _, node := range nodes {
// Skip foreign node
if node.Spec.ProviderID != "" && !strings.HasPrefix(node.Spec.ProviderID, c.cloud.ProviderName()) {
continue
}

// Default NodeReady status to v1.ConditionUnknown
status := v1.ConditionUnknown
if _, c := nodeutil.GetNodeCondition(&node.Status, v1.NodeReady); c != nil {
Expand All @@ -142,6 +148,27 @@ func (c *CloudNodeLifecycleController) MonitorNodes(ctx context.Context) {
continue
}

// Wait uninitialized unknown node
if node.Spec.ProviderID == "" {
uninitialized := false

for _, taint := range node.Spec.Taints {
if taint.Key == cloudproviderapi.TaintExternalCloudProvider {
uninitialized = true
}
}

// Do not delete node for 5 minutes after creation
if uninitialized {
delay := time.Since(node.ObjectMeta.CreationTimestamp.Time)
if delay < time.Duration(5*time.Minute) {
klog.V(2).Infof("Wait uninitialized node %s for %s", node.Name, delay.String())

continue
}
}
}

// At this point the node has NotReady status, we need to check if the node has been removed
// from the cloud provider. If node cannot be found in cloudprovider, then delete the node
exists, err := ensureNodeExistsByProviderID(ctx, c.cloud, node)
Expand Down
60 changes: 60 additions & 0 deletions controllers/nodelifecycle/node_lifecycle_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/record"
cloudproviderapi "k8s.io/cloud-provider/api"
fakecloud "k8s.io/cloud-provider/fake"
"k8s.io/klog/v2"
)
Expand Down Expand Up @@ -67,6 +68,63 @@ func Test_NodesDeleted(t *testing.T) {
ExistsByProviderID: false,
},
},
{
name: "node is not ready and uninitialized",
existingNode: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node-uninitialized",
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,
},
},
},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{
Type: v1.NodeReady,
Status: v1.ConditionFalse,
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
},
},
// expectedNode: &v1.Node{
// ObjectMeta: metav1.ObjectMeta{
// Name: "node-uninitialized",
// 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,
// },
// },
// },
// Status: v1.NodeStatus{
// Conditions: []v1.NodeCondition{
// {
// Type: v1.NodeReady,
// Status: v1.ConditionFalse,
// LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
// LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
// },
// },
// },
// },
expectedDeleted: true,
fakeCloud: &fakecloud.Cloud{
ExistsByProviderID: false,
},
},
{
name: "node is not ready and provider returns err",
existingNode: &v1.Node{
Expand Down Expand Up @@ -155,6 +213,7 @@ func Test_NodesDeleted(t *testing.T) {
},
expectedDeleted: false,
fakeCloud: &fakecloud.Cloud{
Provider: "node0",
ExistsByProviderID: true,
},
},
Expand Down Expand Up @@ -595,6 +654,7 @@ func Test_NodesShutdown(t *testing.T) {
fakeCloud: &fakecloud.Cloud{
NodeShutdown: true,
ExistsByProviderID: true,
Provider: "node0",
ErrShutdownByProviderID: nil,
},
},
Expand Down

0 comments on commit 8468b74

Please sign in to comment.