Skip to content

Commit

Permalink
Do not try to evict mirror pods (#1058)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ole Markus With authored Dec 30, 2021
1 parent 77017ac commit b63346c
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 4 deletions.
43 changes: 43 additions & 0 deletions pkg/controllers/termination/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
. "knative.dev/pkg/logging/testing"
Expand Down Expand Up @@ -196,6 +197,48 @@ var _ = Describe("Termination", func() {
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
ExpectNotFound(ctx, env.Client, node)
})
It("should not evict static pods", func() {
podEvict := test.Pod(test.PodOptions{NodeName: node.Name})
ExpectCreated(ctx, env.Client, node, podEvict)

podNoEvict := test.Pod(test.PodOptions{
NodeName: node.Name,
OwnerReferences: []metav1.OwnerReference{{
APIVersion: "v1",
Kind: "Node",
Name: node.Name,
UID: node.UID,
}},
})
ExpectCreated(ctx, env.Client, podNoEvict)

Expect(env.Client.Delete(ctx, node)).To(Succeed())
node = ExpectNodeExists(ctx, env.Client, node.Name)
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))

// Expect mirror pod to not be queued for enviction
ExpectNotEnqueuedForEviction(evictionQueue, podNoEvict)

// Expect podEvict to be enqueued for eviction then be successful
ExpectEnqueuedForEviction(evictionQueue, podEvict)
ExpectEvicted(env.Client, podEvict)

// Expect node to exist and be draining
ExpectNodeDraining(env.Client, node.Name)

// Reconcile node to evict pod
node = ExpectNodeExists(ctx, env.Client, node.Name)
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))

// Delete pod to simulate successful eviction
ExpectDeleted(ctx, env.Client, podEvict)

// Reconcile to delete node
node = ExpectNodeExists(ctx, env.Client, node.Name)
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(node))
ExpectNotFound(ctx, env.Client, node)

})
It("should not delete nodes until all pods are deleted", func() {
pods := []*v1.Pod{test.Pod(test.PodOptions{NodeName: node.Name}), test.Pod(test.PodOptions{NodeName: node.Name})}
ExpectCreated(ctx, env.Client, node, pods[0], pods[1])
Expand Down
13 changes: 9 additions & 4 deletions pkg/controllers/termination/terminate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/aws/karpenter/pkg/cloudprovider"
"github.com/aws/karpenter/pkg/utils/functional"
"github.com/aws/karpenter/pkg/utils/injectabletime"
"github.com/aws/karpenter/pkg/utils/pod"
"github.com/aws/karpenter/pkg/utils/ptr"
)

Expand Down Expand Up @@ -110,16 +111,20 @@ func (t *Terminator) getPods(ctx context.Context, node *v1.Node) ([]*v1.Pod, err

func (t *Terminator) getEvictablePods(pods []*v1.Pod) []*v1.Pod {
evictable := []*v1.Pod{}
for _, pod := range pods {
for _, p := range pods {
// Ignore if unschedulable is tolerated, since they will reschedule
if (v1alpha5.Taints{{Key: v1.TaintNodeUnschedulable, Effect: v1.TaintEffectNoSchedule}}).Tolerates(pod) == nil {
if (v1alpha5.Taints{{Key: v1.TaintNodeUnschedulable, Effect: v1.TaintEffectNoSchedule}}).Tolerates(p) == nil {
continue
}
// Ignore if kubelet is partitioned and pods are beyond graceful termination window
if IsStuckTerminating(pod) {
if IsStuckTerminating(p) {
continue
}
// Ignore static mirror pods
if pod.IsOwnedByNode(p) {
continue
}
evictable = append(evictable, pod)
evictable = append(evictable, p)
}
return evictable
}
Expand Down

1 comment on commit b63346c

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on b63346c Dec 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@check-spelling-bot Report

🔴 Please review

See the action log for details.

Unrecognized words (1)

enviction

Available dictionaries could cover words not in the dictionary

cspell:cpp/cpp.txt (104293) covers 72 of them
cspell:aws/aws.txt (1485) covers 30 of them
cspell:golang/go.txt (7745) covers 18 of them
cspell:django/django.txt (2342) covers 17 of them
cspell:filetypes/filetypes.txt (337) covers 11 of them
cspell:html/html.txt (542) covers 9 of them
cspell:css/css.txt (993) covers 9 of them
cspell:java/java.txt (33524) covers 8 of them
cspell:fullstack/fullstack.txt (181) covers 7 of them
cspell:python/python.txt (364) covers 6 of them
cspell:lua/lua.txt (391) covers 6 of them
cspell:scala/scala.txt (2752) covers 5 of them
cspell:npm/npm.txt (671) covers 5 of them
cspell:csharp/csharp.txt (123) covers 3 of them
cspell:rust/rust.txt (112) covers 2 of them
cspell:ruby/ruby.txt (354) covers 2 of them
cspell:node/node.txt (9611) covers 2 of them
cspell:bash/bash-words.txt (22) covers 2 of them
cspell:php/php.txt (9785) covers 1 of them
cspell:dotnet/dotnet.txt (9824) covers 1 of them
cspell:ada/ada.txt (72) covers 1 of them

Consider adding them using:

      with:
        extra_dictionaries:
          cspell:cpp/cpp.txt
          cspell:aws/aws.txt
          cspell:golang/go.txt
          cspell:django/django.txt
          cspell:filetypes/filetypes.txt
          cspell:html/html.txt
          cspell:css/css.txt
          cspell:java/java.txt
          cspell:fullstack/fullstack.txt
          cspell:python/python.txt
          cspell:lua/lua.txt
          cspell:scala/scala.txt
          cspell:npm/npm.txt
          cspell:csharp/csharp.txt
          cspell:rust/rust.txt
          cspell:ruby/ruby.txt
          cspell:node/node.txt
          cspell:bash/bash-words.txt
          cspell:php/php.txt
          cspell:dotnet/dotnet.txt
          cspell:ada/ada.txt

To stop checking additional dictionaries, add:

      with:
        check_extra_dictionaries: ''
To accept these unrecognized words as correct, run the following commands

... in a clone of the [email protected]:aws/karpenter.git repository
on the main branch:

update_files() {
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/aws/karpenter/comments/62705876" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u
If the flagged items do not appear to be text

If items relate to a ...

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

  • binary file.

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

Please sign in to comment.