E2E: ensure periodic test can't fail due to cron conflicts (#20300)

The E2E test for periodic dispatch jobs has a `cron` trigger for once a
minute. If the test happens to run at the top of the minute, it's possible for
the forced dispatch to run from the test code, then the periodic timer triggers
and leaves a running child job. This fails the test because it expects only a
single job in the "dead" state.

Make it so that the `cron` expression is implausible to run during our test
window, and migrate the test off the old framework while we're at it.

e2e/periodic/doc.go

@@ -0,0 +1,5 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+// Package periodic provides tests around periodic dispatch behavior
+package periodic

e2e/periodic/input/simple.nomad

@@ -2,8 +2,7 @@
 # SPDX-License-Identifier: BUSL-1.1
 
 job "periodic" {
-  datacenters = ["dc1"]
-  type        = "batch"
+  type = "batch"
 
   constraint {
     attribute = "${attr.kernel.name}"
@@ -11,10 +10,10 @@ job "periodic" {
     value     = "darwin,linux"
   }
 
-
-
   periodic {
-    cron             = "* * * * *"
+    # run on Jan 31st at 13:13, only if it's Sunday, to ensure no collisions
+    # with our test forcing a dispatch
+    cron             = "13 13 31 1 7"
     prohibit_overlap = true
   }
 

e2e/periodic/periodic.go

@@ -1,82 +0,0 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: BUSL-1.1
-
-package periodic
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/nomad/e2e/e2eutil"
-	"github.com/hashicorp/nomad/e2e/framework"
-	"github.com/hashicorp/nomad/helper/uuid"
-	"github.com/hashicorp/nomad/testutil"
-	"github.com/stretchr/testify/require"
-)
-
-type PeriodicTest struct {
-	framework.TC
-	jobIDs []string
-}
-
-func init() {
-	framework.AddSuites(&framework.TestSuite{
-		Component:   "Periodic",
-		CanRunLocal: true,
-		Cases: []framework.TestCase{
-			new(PeriodicTest),
-		},
-	})
-}
-
-func (tc *PeriodicTest) BeforeAll(f *framework.F) {
-	e2eutil.WaitForLeader(f.T(), tc.Nomad())
-}
-
-func (tc *PeriodicTest) AfterEach(f *framework.F) {
-	nomadClient := tc.Nomad()
-	j := nomadClient.Jobs()
-
-	for _, id := range tc.jobIDs {
-		j.Deregister(id, true, nil)
-	}
-	_, err := e2eutil.Command("nomad", "system", "gc")
-	f.NoError(err)
-}
-
-func (tc *PeriodicTest) TestPeriodicDispatch_Basic(f *framework.F) {
-	t := f.T()
-
-	uuid := uuid.Generate()
-	jobID := fmt.Sprintf("periodicjob-%s", uuid[0:8])
-	tc.jobIDs = append(tc.jobIDs, jobID)
-
-	// register job
-	require.NoError(t, e2eutil.Register(jobID, "periodic/input/simple.nomad"))
-
-	// force dispatch
-	require.NoError(t, e2eutil.PeriodicForce(jobID))
-
-	testutil.WaitForResult(func() (bool, error) {
-		children, err := e2eutil.PreviouslyLaunched(jobID)
-		if err != nil {
-			return false, err
-		}
-
-		for _, c := range children {
-			if c["Status"] == "dead" {
-				return true, nil
-			}
-		}
-		return false, fmt.Errorf("expected periodic job to be dead")
-	}, func(err error) {
-		require.NoError(t, err)
-	})
-
-	// Assert there are no pending children
-	summary, err := e2eutil.ChildrenJobSummary(jobID)
-	require.NoError(t, err)
-	require.Len(t, summary, 1)
-	require.Equal(t, summary[0]["Pending"], "0")
-	require.Equal(t, summary[0]["Running"], "0")
-	require.Equal(t, summary[0]["Dead"], "1")
-}

e2e/periodic/periodic_test.go

@@ -0,0 +1,50 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package periodic
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/nomad/e2e/e2eutil"
+	"github.com/hashicorp/nomad/e2e/v3/jobs3"
+	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
+)
+
+func TestPeriodicDispatch_Basic(t *testing.T) {
+
+	sub, cleanup := jobs3.Submit(t, "input/simple.nomad", jobs3.Dispatcher())
+	t.Cleanup(cleanup)
+
+	// force dispatch and wait for the dispatched job to finish
+	must.NoError(t, e2eutil.PeriodicForce(sub.JobID()))
+	must.Wait(t, wait.InitialSuccess(
+		wait.ErrorFunc(func() error {
+			children, err := e2eutil.PreviouslyLaunched(sub.JobID())
+			if err != nil {
+				return err
+			}
+
+			for _, c := range children {
+				if c["Status"] == "dead" {
+					return nil
+				}
+			}
+			return fmt.Errorf("expected periodic job to be dead")
+
+		}),
+		wait.Timeout(30*time.Second),
+		wait.Gap(time.Second),
+	))
+
+	// Assert there are no pending children
+	summary, err := e2eutil.ChildrenJobSummary(sub.JobID())
+	must.NoError(t, err)
+	must.Len(t, 1, summary)
+	must.Eq(t, "0", summary[0]["Pending"])
+	must.Eq(t, "0", summary[0]["Running"])
+	must.Eq(t, "1", summary[0]["Dead"])
+}

e2e/v3/jobs3/jobs3.go

@@ -37,6 +37,7 @@ type Submission struct {
 	timeout       time.Duration
 	verbose       bool
 	detach        bool
+	dispatcher    bool
 
 	// jobspec mutator funcs
 	mutators []func(string) string
@@ -327,6 +328,10 @@ func (sub *Submission) run() {
 		})
 	}
 
+	if sub.dispatcher {
+		return
+	}
+
 	evalID := regResp.EvalID
 
 	queryOpts := &nomadapi.QueryOptions{
@@ -593,6 +598,14 @@ func PreCleanup(cb func(*Submission)) Option {
 	}
 }
 
+// Dispatcher indicates the job is the parent for dispatched jobs, so we
+// shouldn't wait for evals or deployments
+func Dispatcher() Option {
+	return func(sub *Submission) {
+		sub.dispatcher = true
+	}
+}
+
 // defaultPreCleanup looks for blocked evals, alloc errors, and task events
 // only when the test has failed.
 func defaultPreCleanup(job *Submission) {