fix(frontend): peek and restart take <array-id>[k] and written job IDs
- apiserver peek (parseJobIDParam): <array-id>[k] now resolves to the element; it used to drop [k] and look up the array identity itself (and panicked in parseArrayName on some inputs). A job number still names that job exactly (an element by its own XCE_JOBID); ranges and invalid selections are rejected. - apiserver restart: the decimal-only pre-check (isValidJobId, now unused and removed) rejected the written form c-a-ts-seq[k]; ParseIdentitySelection, already used for the lookup, validates it. - xcecli jobs restart converts its argument with ToAPIJobID like the control commands, so written IDs work and invalid ones fail locally. - docs: the peek/restart caveat in the Job arrays section. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a65146a9ae
commit
c15580a7f3
7 changed files with 126 additions and 35 deletions
|
|
@ -91,9 +91,10 @@ supports a subset of the LSF array syntax.
|
||||||
- In listing and control commands, a bare element job number (for example
|
- In listing and control commands, a bare element job number (for example
|
||||||
the value of `XCE_JOBID`) is rejected: name the element as
|
the value of `XCE_JOBID`) is rejected: name the element as
|
||||||
`<array-id>[k]`.
|
`<array-id>[k]`.
|
||||||
- `xcecli jobs peek` and `xcecli jobs restart` do not accept
|
- `xcecli jobs peek` and `xcecli jobs restart` take one job: a job ID,
|
||||||
`<array-id>[k]` yet: peek an element by its own job number
|
or one element as `<array-id>[k]` (not a range). `peek` also accepts
|
||||||
(`XCE_JOBID`); restart takes job IDs as before.
|
an element's own job number (`XCE_JOBID`). Restarting a bare array
|
||||||
|
identity is refused: restart one element at a time.
|
||||||
- **Element number in the job's environment.** A running element finds its
|
- **Element number in the job's environment.** A running element finds its
|
||||||
own number in `XCE_JOBINDEX` (1..N). An ordinary job gets
|
own number in `XCE_JOBINDEX` (1..N). An ordinary job gets
|
||||||
`XCE_JOBINDEX=0`. A user-supplied `XCE_JOBINDEX` is overridden, with a
|
`XCE_JOBINDEX=0`. A user-supplied `XCE_JOBINDEX` is overridden, with a
|
||||||
|
|
|
||||||
|
|
@ -839,12 +839,6 @@ func (j JobController) RestartJobByID(ctx *gin.Context) {
|
||||||
logger.Debug("Receive job restart request:",
|
logger.Debug("Receive job restart request:",
|
||||||
zap.String("jobId", jobIDStr), zap.String("restarter", restarter))
|
zap.String("jobId", jobIDStr), zap.String("restarter", restarter))
|
||||||
|
|
||||||
if err = isValidJobId(jobIDStr); err != nil {
|
|
||||||
logger.Error(err.Error())
|
|
||||||
ReturnError(ctx, common.INVALID_PARAMS_CODE, common.INVALID_PARAMS_MSG+": "+err.Error(), nil)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
// check post body whether correct
|
// check post body whether correct
|
||||||
var req models.JobRestartRequest
|
var req models.JobRestartRequest
|
||||||
if err := utils.BindJSONStrict(ctx, &req); err != nil {
|
if err := utils.BindJSONStrict(ctx, &req); err != nil {
|
||||||
|
|
@ -1775,15 +1769,6 @@ func isValidArrayName(arrayName string) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func isValidJobId(jobID string) error {
|
|
||||||
matched, _ := regexp.MatchString(`^\d+$`, jobID)
|
|
||||||
matchedIndex, _ := regexp.MatchString(`^\d+\[\d+\]$`, jobID)
|
|
||||||
if !matched && !matchedIndex {
|
|
||||||
return fmt.Errorf("jobid '%s' contains invalid characters. Allowed characters are 0-9 or with index value", jobID)
|
|
||||||
}
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
func validateAskedHosts(askedHosts string) error {
|
func validateAskedHosts(askedHosts string) error {
|
||||||
if askedHosts == "" {
|
if askedHosts == "" {
|
||||||
return nil // Nothing to validate
|
return nil // Nothing to validate
|
||||||
|
|
|
||||||
|
|
@ -1470,6 +1470,8 @@ func TestJobHandler_RestartNamedJob(t *testing.T) {
|
||||||
}{
|
}{
|
||||||
{"ordinary job by decimal id", strconv.FormatUint(ord, 10), "ordinary"},
|
{"ordinary job by decimal id", strconv.FormatUint(ord, 10), "ordinary"},
|
||||||
{"array element by <array-id>[k]", strconv.FormatUint(arr, 10) + "[2]", "elem-2"},
|
{"array element by <array-id>[k]", strconv.FormatUint(arr, 10) + "[2]", "elem-2"},
|
||||||
|
{"array element by written <array-id>[k]", backend.FormatJobID(int64(arr)) + "[2]", "elem-2"},
|
||||||
|
{"ordinary job by written id", backend.FormatJobID(int64(ord)), "ordinary"},
|
||||||
}
|
}
|
||||||
for _, c := range cases {
|
for _, c := range cases {
|
||||||
before := len(f.submitted)
|
before := len(f.submitted)
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,6 @@ import (
|
||||||
"apiserver/common"
|
"apiserver/common"
|
||||||
"apiserver/models"
|
"apiserver/models"
|
||||||
"apiserver/store"
|
"apiserver/store"
|
||||||
"apiserver/utils"
|
|
||||||
|
|
||||||
logger "apiserver/utils"
|
logger "apiserver/utils"
|
||||||
|
|
||||||
|
|
@ -220,25 +219,29 @@ func resolveOutputPath(path string, job models.JobInfo) string {
|
||||||
return path
|
return path
|
||||||
}
|
}
|
||||||
|
|
||||||
// parseJobIDParam parses a job ID string and returns the corresponding JobInfo.
|
// parseJobIDParam parses the job peek names and returns the corresponding
|
||||||
|
// JobInfo: a job number (an ordinary job, or an element by its own number,
|
||||||
|
// XCE_JOBID) names that job exactly; <array-id>[k] names element k. A range
|
||||||
|
// or an invalid selection is an error -- peek shows one job.
|
||||||
func parseJobIDParam(jobidStr string) (models.JobInfo, error) {
|
func parseJobIDParam(jobidStr string) (models.JobInfo, error) {
|
||||||
// Use the same parsing logic as GetJobByIDs
|
raw := strings.TrimSpace(jobidStr)
|
||||||
jobArrInfo, err := parseArrayName(jobidStr)
|
if n, err := strconv.ParseUint(raw, 10, 64); err == nil {
|
||||||
if err != nil {
|
return models.JobInfo{Unique_JobID: int64(n), ArrayIdx: backend.ExtractArrayIdx(n)}, nil
|
||||||
return models.JobInfo{}, err
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if !utils.IsNumeric(jobArrInfo.KeyID) {
|
sel, selErr := ParseIdentitySelection(raw)
|
||||||
return models.JobInfo{}, fmt.Errorf("bad jobid: %s", jobArrInfo.KeyID)
|
if selErr != nil {
|
||||||
|
return models.JobInfo{}, selErr
|
||||||
}
|
}
|
||||||
|
switch sel.Form {
|
||||||
uniqueJobID, err := strconv.ParseInt(jobArrInfo.KeyID, 10, 64)
|
case SelSingle:
|
||||||
if err != nil {
|
return models.JobInfo{
|
||||||
return models.JobInfo{}, fmt.Errorf("bad jobid: %s", jobArrInfo.KeyID)
|
Unique_JobID: int64(backend.MakeElementID(sel.Key, sel.Lo)),
|
||||||
|
ArrayIdx: sel.Lo,
|
||||||
|
}, nil
|
||||||
|
case SelRange:
|
||||||
|
return models.JobInfo{}, fmt.Errorf("'%s' names several jobs; peek one element as <array-id>[k]", raw)
|
||||||
|
default:
|
||||||
|
return models.JobInfo{Unique_JobID: int64(sel.Key)}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return models.JobInfo{
|
|
||||||
Unique_JobID: uniqueJobID,
|
|
||||||
ArrayIdx: 0, // peek defaults to base job; array idx can be added later
|
|
||||||
}, nil
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
50
src/frontend/apiserver/controllers/output_jobid_test.go
Normal file
50
src/frontend/apiserver/controllers/output_jobid_test.go
Normal file
|
|
@ -0,0 +1,50 @@
|
||||||
|
package controllers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strconv"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"apiserver/backend"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Peek names one job: an ordinary job or element by its own job number
|
||||||
|
// (XCE_JOBID), or an element as <array-id>[k]; parseJobIDParam used to drop
|
||||||
|
// the [k] and look up the array identity itself.
|
||||||
|
func TestParseJobIDParam(t *testing.T) {
|
||||||
|
arr := handlerTestArrayBase + 50<<15
|
||||||
|
elem2 := backend.MakeElementID(arr, 2)
|
||||||
|
ord := handlerTestOrdinaryBase + 51<<15
|
||||||
|
human := backend.FormatJobID(int64(arr))
|
||||||
|
|
||||||
|
ok := []struct {
|
||||||
|
in string
|
||||||
|
want uint64
|
||||||
|
}{
|
||||||
|
{strconv.FormatUint(ord, 10), ord},
|
||||||
|
{strconv.FormatUint(elem2, 10), elem2}, // the element's own number
|
||||||
|
{strconv.FormatUint(arr, 10) + "[2]", elem2},
|
||||||
|
{human + "[2]", elem2},
|
||||||
|
{" " + strconv.FormatUint(arr, 10) + "[2] ", elem2},
|
||||||
|
}
|
||||||
|
for _, c := range ok {
|
||||||
|
got, err := parseJobIDParam(c.in)
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("%q: unexpected error %v", c.in, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if uint64(got.Unique_JobID) != c.want {
|
||||||
|
t.Errorf("%q: job %d, want %d", c.in, got.Unique_JobID, c.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, in := range []string{
|
||||||
|
strconv.FormatUint(arr, 10) + "[1-3]", // a range is several jobs
|
||||||
|
strconv.FormatUint(arr, 10) + "[40000]",
|
||||||
|
"garbage",
|
||||||
|
"",
|
||||||
|
} {
|
||||||
|
if _, err := parseJobIDParam(in); err == nil {
|
||||||
|
t.Errorf("%q: want an error", in)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -3,6 +3,7 @@ package jobs
|
||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"xcecli/common/format"
|
||||||
"xcecli/common/request"
|
"xcecli/common/request"
|
||||||
"xcecli/config"
|
"xcecli/config"
|
||||||
"xcecli/models"
|
"xcecli/models"
|
||||||
|
|
@ -35,6 +36,13 @@ func (c *JobRestart) ParseCmdArgs(cmd *cobra.Command, args []string) error {
|
||||||
// if no job IDs or filter conditions are specified, default to processing all jobs of the current user
|
// if no job IDs or filter conditions are specified, default to processing all jobs of the current user
|
||||||
return fmt.Errorf("no job ID is specified")
|
return fmt.Errorf("no job ID is specified")
|
||||||
}
|
}
|
||||||
|
// the API server takes the decimal key, or <key>[k] for an element;
|
||||||
|
// restart names one job, so no range
|
||||||
|
apiID, err := format.ToAPIJobID(c.jobID, false)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
c.jobID = apiID
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
42
src/frontend/xcecli/cmd/jobs/restart_test.go
Normal file
42
src/frontend/xcecli/cmd/jobs/restart_test.go
Normal file
|
|
@ -0,0 +1,42 @@
|
||||||
|
package jobs
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"xcecli/common/format"
|
||||||
|
)
|
||||||
|
|
||||||
|
// restart sends the API server the decimal job key (or <key>[k]); a job ID
|
||||||
|
// in its written form c-a-ts-seq used to be sent unconverted and rejected.
|
||||||
|
func TestRestartParseCmdArgsConvertsJobID(t *testing.T) {
|
||||||
|
written := format.FormatJobID(int64(testArrayKey))
|
||||||
|
key := strconv.FormatUint(testArrayKey, 10)
|
||||||
|
cases := []struct{ in, want string }{
|
||||||
|
{key, key},
|
||||||
|
{written, key},
|
||||||
|
{written + "[2]", key + "[2]"},
|
||||||
|
{key + "[2]", key + "[2]"},
|
||||||
|
}
|
||||||
|
for _, c := range cases {
|
||||||
|
r := NewJobRestart(nil)
|
||||||
|
if err := r.ParseCmdArgs(nil, []string{c.in}); err != nil {
|
||||||
|
t.Errorf("%q: unexpected error %v", c.in, err)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if r.jobID != c.want {
|
||||||
|
t.Errorf("%q: sends %q, want %q", c.in, r.jobID, c.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, in := range []string{written + "[1-3]", "garbage"} {
|
||||||
|
r := NewJobRestart(nil)
|
||||||
|
err := r.ParseCmdArgs(nil, []string{in})
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("%q: want an error", in)
|
||||||
|
} else if !strings.Contains(err.Error(), in) {
|
||||||
|
t.Errorf("%q: error %q does not name the selection", in, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Add a link
Reference in a new issue