Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1733474: Use upstream drain library instead of downstream #464

Merged
merged 3 commits into from
Feb 6, 2020
Merged

Bug 1733474: Use upstream drain library instead of downstream #464

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57a3204
Vendor dependencies
Jan 16, 2020
f0c52c1
Use upstream drain library
Jan 16, 2020
4e9ca8a
Add skip wait for delete timeout
Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/controller/machine/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ const (

// Machine has a deletion timestamp
phaseDeleting = "Deleting"

skipWaitForDeleteTimeoutSeconds = 60 * 5
)

var DefaultActuator Actuator
Expand Down Expand Up @@ -344,6 +346,10 @@ func (r *ReconcileMachine) drainNode(machine *machinev1.Machine) error {
DryRun: false,
}

if nodeIsUnreachable(node) {
drainer.SkipWaitForDeleteTimeoutSeconds = skipWaitForDeleteTimeoutSeconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include a logging message here e.g "This node is unreachable, draining will wait for pod deletion during skipWaitForDeleteTimeoutSeconds after the request and will skip otherwise."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, can we do it after master opens?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@enxebre Why not skip the drain entirely we know the node to be unreachable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that kubelet unreachability might be temporary. The intention is to tolerate unreachability during a reasonable timeframe i.e 5 min before considering the node dead and deleting the underlying infra, therefore disrupting app intent for graceful shutdowns and PDB policies.

}

if err := drain.RunCordonOrUncordon(drainer, node, true); err != nil {
// Can't cordon a node
klog.Warningf("cordon failed for node %q: %v", node.Name, err)
Expand Down Expand Up @@ -429,6 +435,16 @@ func machineIsFailed(machine *machinev1.Machine) bool {
return false
}

func nodeIsUnreachable(node *corev1.Node) bool {
for _, condition := range node.Status.Conditions {
if condition.Type == corev1.NodeReady && condition.Status == corev1.ConditionUnknown {
return true
}
}

return false
}

// writer implements io.Writer interface as a pass-through for klog.
type writer struct {
logFunc func(args ...interface{})
Expand Down
53 changes: 53 additions & 0 deletions pkg/controller/machine/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,3 +497,56 @@ func TestMachineIsFailed(t *testing.T) {
}
}
}

func TestNodeIsUnreachable(t *testing.T) {
testCases := []struct {
name string
node *corev1.Node
expected bool
}{
{
name: "Node should be unreachable",
node: &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "Node",
Namespace: "test",
},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{
corev1.NodeCondition{
Type: corev1.NodeReady,
Status: corev1.ConditionUnknown,
},
},
},
},
expected: true,
},
{
name: "Node should not be unreachable",
node: &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "Node",
Namespace: "test",
},
Status: corev1.NodeStatus{
Conditions: []corev1.NodeCondition{
corev1.NodeCondition{
Type: corev1.NodeReady,
Status: corev1.ConditionTrue,
},
},
},
},
expected: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if actual := nodeIsUnreachable(tc.node); actual != tc.expected {
t.Errorf("Expected: %v, got: %v", actual, tc.expected)
}
})
}
}