backport of commit 44ca8ee (#13162)

This pull request was automerged via backport-assistant
hashicorp · Jun 2, 2022 · 259f4e4 · 259f4e4
1 parent 1ed01f8
commit 259f4e4
Show file tree

Hide file tree

Showing 6 changed files with 139 additions and 4 deletions.
diff --git a/.changelog/13205.txt b/.changelog/13205.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+core: Fixed a bug where an evicted batch job would not be rescheduled
+```
diff --git a/go.work b/go.work
@@ -0,0 +1,4 @@
+go 1.18
+
+use .
+use ../netlog
diff --git a/go.work.sum b/go.work.sum
@@ -0,0 +1,13 @@
+github.com/apparentlymart/go-textseg v1.0.0 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2wFoYVvnCs0=
+github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=
+github.com/cheggaaa/pb v1.0.27 h1:wIkZHkNfC7R6GI5w7l/PdAdzXzlrbcI3p8OAlnkTsnc=
+github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e h1:Wf6HqHfScWJN9/ZjdUKyjop4mf3Qdd+1TvvltAvM3m8=
+github.com/godbus/dbus v0.0.0-20190422162347-ade71ed3457e h1:BWhy2j3IXJhjCbC68FptL43tDKIq8FladmaTs3Xs7Z8=
+github.com/shirou/gopsutil v0.0.0-20181107111621-48177ef5f880 h1:1Ge4j/3uB2rxzPWD3TC+daeCw+w91z8UCUL/7WH5gn8=
+github.com/vmihailenco/msgpack v3.3.3+incompatible h1:wapg9xDUZDzGCNFlwc5SqI1rvcciqcxEHac4CYj89xI=
+github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
+golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
+golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
+golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
+golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
diff --git a/scheduler/reconcile_util.go b/scheduler/reconcile_util.go
@@ -292,7 +292,8 @@ func (a allocSet) filterByRescheduleable(isBatch bool, now time.Time, evalID str
 	return
 }
 
-// shouldFilter returns whether the alloc should be ignored or considered untainted
+// shouldFilter returns whether the alloc should be ignored or considered untainted.
+//
 // Ignored allocs are filtered out.
 // Untainted allocs count against the desired total.
 // Filtering logic for batch jobs:
@@ -309,11 +310,13 @@ func shouldFilter(alloc *structs.Allocation, isBatch bool) (untainted, ignore bo
 	// complete but not failed, they shouldn't be replaced.
 	if isBatch {
 		switch alloc.DesiredStatus {
-		case structs.AllocDesiredStatusStop, structs.AllocDesiredStatusEvict:
+		case structs.AllocDesiredStatusStop:
 			if alloc.RanSuccessfully() {
 				return true, false
 			}
 			return false, true
+		case structs.AllocDesiredStatusEvict:
+			return false, true
 		default:
 		}
 

diff --git a/scheduler/reconcile_util_test.go b/scheduler/reconcile_util_test.go
@@ -2,7 +2,7 @@ package scheduler
 
 import (
 	"testing"
-
+	"time"
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/nomad/mock"
@@ -134,3 +134,110 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 	require.Contains(lost, "lost1")
 	require.Contains(lost, "lost2")
 }
+
+func TestReconcile_shouldFilter(t *testing.T) {
+	testCases := []struct {
+		description   string
+		batch         bool
+		failed        bool
+		desiredStatus string
+		clientStatus  string
+
+		untainted bool
+		ignore    bool
+	}{
+		{
+			description:   "batch running",
+			batch:         true,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusRun,
+			clientStatus:  structs.AllocClientStatusRunning,
+			untainted:     true,
+			ignore:        false,
+		},
+		{
+			description:   "batch stopped success",
+			batch:         true,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusStop,
+			clientStatus:  structs.AllocClientStatusRunning,
+			untainted:     true,
+			ignore:        false,
+		},
+		{
+			description:   "batch stopped failed",
+			batch:         true,
+			failed:        true,
+			desiredStatus: structs.AllocDesiredStatusStop,
+			clientStatus:  structs.AllocClientStatusComplete,
+			untainted:     false,
+			ignore:        true,
+		},
+		{
+			description:   "batch evicted",
+			batch:         true,
+			desiredStatus: structs.AllocDesiredStatusEvict,
+			clientStatus:  structs.AllocClientStatusComplete,
+			untainted:     false,
+			ignore:        true,
+		},
+		{
+			description:   "batch failed",
+			batch:         true,
+			desiredStatus: structs.AllocDesiredStatusRun,
+			clientStatus:  structs.AllocClientStatusFailed,
+			untainted:     false,
+			ignore:        false,
+		},
+		{
+			description:   "service running",
+			batch:         false,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusRun,
+			clientStatus:  structs.AllocClientStatusRunning,
+			untainted:     false,
+			ignore:        false,
+		},
+		{
+			description:   "service stopped",
+			batch:         false,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusStop,
+			clientStatus:  structs.AllocClientStatusComplete,
+			untainted:     false,
+			ignore:        true,
+		},
+		{
+			description:   "service evicted",
+			batch:         false,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusEvict,
+			clientStatus:  structs.AllocClientStatusComplete,
+			untainted:     false,
+			ignore:        true,
+		},
+		{
+			description:   "service client complete",
+			batch:         false,
+			failed:        false,
+			desiredStatus: structs.AllocDesiredStatusRun,
+			clientStatus:  structs.AllocClientStatusComplete,
+			untainted:     false,
+			ignore:        true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			alloc := &structs.Allocation{
+				DesiredStatus: tc.desiredStatus,
+				TaskStates:    map[string]*structs.TaskState{"task": {State: structs.TaskStateDead, Failed: tc.failed}},
+				ClientStatus:  tc.clientStatus,
+			}
+
+			untainted, ignore := shouldFilter(alloc, tc.batch)
+			require.Equal(t, tc.untainted, untainted)
+			require.Equal(t, tc.ignore, ignore)
+		})
+	}
+}
diff --git a/website/content/docs/drivers/docker.mdx b/website/content/docs/drivers/docker.mdx
@@ -199,11 +199,16 @@ config {
   ```
 
 - `load` - (Optional) Load an image from a `tar` archive file instead of from a
-  remote repository. Equivalent to the `docker load -i <filename>` command.
+  remote repository. Equivalent to the `docker load -i <filename>` command. If
+  you're using an `artifact` block to fetch the archive file, you'll need to
+  ensure that Nomad keeps the archive intact after download.
 
   ```hcl
   artifact {
     source = "http://path.to/redis.tar"
+    options {
+      archive = false
+    }
   }
   config {
     load = "redis.tar"