Skip to content

Commit

Permalink
Move job utils to temporary legacy package (#3457)
Browse files Browse the repository at this point in the history
Currently the V1 CLI commands that we want to move to V2 (docker run,
wasm run) make use of the `pkg/job` package to build a job ready for
submission. The files here make use of pkg/model.

To make it clearer what needs to change in the CLI this PR moves
`pkg/job` to `pkg/legacyjob`. This leaves pkg/job open to
re-implementation following a similar but not identical approach
(builders with options).

This aims to make it easier to move over the job creation commands
_without_ forcing the implementor to change both, and all the tests in
the same PR. Now it should be possible for docker run to move over to v2
independently of the wasm run command.

## Summary by CodeRabbit

- **Refactor**
- Updated various components and tests to use `legacy_job` instead of
`jobutils` for job verification and handling, enhancing clarity and
consistency across the application.
- **Chores**
- Renamed the `job` package to `legacy_job` across multiple files to
better reflect its purpose and usage.
- **Tests**
- Adjusted tests to align with the renaming from `job` to `legacy_job`,
ensuring consistent test execution and results.
  • Loading branch information
rossjones committed Feb 29, 2024
1 parent 14f8f89 commit 9f9ae95
Show file tree
Hide file tree
Showing 49 changed files with 376 additions and 363 deletions.
4 changes: 2 additions & 2 deletions cmd/cli/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags"
"github.com/bacalhau-project/bacalhau/cmd/util/hook"
"github.com/bacalhau-project/bacalhau/cmd/util/printer"
jobutils "github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/userstrings"
"github.com/bacalhau-project/bacalhau/pkg/util/templates"
Expand Down Expand Up @@ -212,7 +212,7 @@ func create(cmd *cobra.Command, cmdArgs []string, OC *CreateOptions) error { //n
cmd.Printf("WARNING: The following fields have data in them and will be ignored on creation: %s\n", strings.Join(unusedFieldList, ", "))
}

err = jobutils.VerifyJob(ctx, j)
err = legacy_job.VerifyJob(ctx, j)
if err != nil {
return fmt.Errorf("error verifying job: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/cli/describe/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
cmdtesting "github.com/bacalhau-project/bacalhau/cmd/testing"
"github.com/bacalhau-project/bacalhau/cmd/util"
"github.com/bacalhau-project/bacalhau/pkg/bacerrors"
jobutils "github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
testutils "github.com/bacalhau-project/bacalhau/pkg/test/utils"
)
Expand Down Expand Up @@ -54,7 +54,7 @@ func (s *DescribeSuite) TestDescribeJob() {
for i := 0; i < tc.numberOfAcceptNodes; i++ {
for k := 0; k < n.numOfJobs; k++ {
j := testutils.MakeJobWithOpts(s.T(),
jobutils.WithEngineSpec(
legacy_job.WithEngineSpec(
model.NewEngineBuilder().
WithType(strings.ToLower(model.EngineNoop.String())).
WithParam("Entrypoint-Unique-Array", uuid.NewString()).
Expand Down Expand Up @@ -206,7 +206,7 @@ func (s *DescribeSuite) TestDescribeJobEdgeCases() {

for i := 0; i < n.numOfJobs; i++ {
j := testutils.MakeJobWithOpts(s.T(),
jobutils.WithEngineSpec(
legacy_job.WithEngineSpec(
model.NewEngineBuilder().
WithType(strings.ToLower(model.EngineNoop.String())).
WithParam("Entrypoint-Unique-Array", uuid.NewString()).
Expand Down
22 changes: 11 additions & 11 deletions cmd/cli/docker/docker_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/bacalhau-project/bacalhau/cmd/util/parse"
"github.com/bacalhau-project/bacalhau/cmd/util/printer"
"github.com/bacalhau-project/bacalhau/pkg/bacerrors"
jobutils "github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/util/templates"
)
Expand Down Expand Up @@ -146,7 +146,7 @@ func dockerRun(cmd *cobra.Command, cmdArgs []string, opts *DockerRunOptions) err
return fmt.Errorf("creating job: %w", err)
}

if err := jobutils.VerifyJob(ctx, j); err != nil {
if err := legacy_job.VerifyJob(ctx, j); err != nil {
if _, ok := err.(*bacerrors.ImageNotFound); ok {
return fmt.Errorf("docker image '%s' not found in the registry, or needs authorization", image)
} else {
Expand Down Expand Up @@ -198,24 +198,24 @@ func CreateJob(ctx context.Context, image string, parameters []string, opts *Doc
return nil, err
}

spec, err := jobutils.MakeDockerSpec(
spec, err := legacy_job.MakeDockerSpec(
image, opts.WorkingDirectory, opts.Entrypoint, opts.SpecSettings.EnvVar, parameters,
jobutils.WithResources(
legacy_job.WithResources(
opts.ResourceSettings.CPU,
opts.ResourceSettings.Memory,
opts.ResourceSettings.Disk,
opts.ResourceSettings.GPU,
),
jobutils.WithNetwork(
legacy_job.WithNetwork(
opts.NetworkingSettings.Network,
opts.NetworkingSettings.Domains,
),
jobutils.WithTimeout(opts.SpecSettings.Timeout),
jobutils.WithInputs(opts.SpecSettings.Inputs.Values()...),
jobutils.WithOutputs(outputs...),
jobutils.WithAnnotations(labels...),
jobutils.WithNodeSelector(nodeSelectorRequirements),
jobutils.WithDeal(
legacy_job.WithTimeout(opts.SpecSettings.Timeout),
legacy_job.WithInputs(opts.SpecSettings.Inputs.Values()...),
legacy_job.WithOutputs(outputs...),
legacy_job.WithAnnotations(labels...),
legacy_job.WithNodeSelector(nodeSelectorRequirements),
legacy_job.WithDeal(
opts.DealSettings.TargetingMode,
opts.DealSettings.Concurrency,
),
Expand Down
6 changes: 3 additions & 3 deletions cmd/cli/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

"github.com/bacalhau-project/bacalhau/cmd/util"
"github.com/bacalhau-project/bacalhau/cmd/util/output"
"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/util/templates"
)
Expand Down Expand Up @@ -165,11 +165,11 @@ var listColumns = []output.TableColumn[*model.JobWithInfo]{
},
{
ColumnConfig: table.ColumnConfig{Name: "state", WidthMax: 20, WidthMaxEnforcer: text.WrapText},
Value: func(jwi *model.JobWithInfo) string { return job.ComputeStateSummary(jwi.State) },
Value: func(jwi *model.JobWithInfo) string { return legacy_job.ComputeStateSummary(jwi.State) },
},
{
ColumnConfig: table.ColumnConfig{Name: "published"},
Value: job.ComputeResultsSummary,
Value: legacy_job.ComputeResultsSummary,
},
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/cli/list/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

"github.com/bacalhau-project/bacalhau/cmd/cli/list"
cmdtesting "github.com/bacalhau-project/bacalhau/cmd/testing"
jobutils "github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/system"
testutils "github.com/bacalhau-project/bacalhau/pkg/test/utils"
Expand Down Expand Up @@ -173,7 +173,7 @@ func (suite *ListSuite) TestList_AnnotationFilter() {
suite.setupRun()

testJob := testutils.MakeJobWithOpts(suite.T(),
jobutils.WithAnnotations(tc.JobLabels...),
legacy_job.WithAnnotations(tc.JobLabels...),
)
j, err := suite.Client.Submit(ctx, &testJob)
require.NoError(suite.T(), err)
Expand Down
8 changes: 4 additions & 4 deletions cmd/cli/serve/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"time"

"github.com/bacalhau-project/bacalhau/pkg/docker"
"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/publicapi/client"
clientv2 "github.com/bacalhau-project/bacalhau/pkg/publicapi/client/v2"
Expand Down Expand Up @@ -59,11 +59,11 @@ func (s *ServeSuite) TestNoTimeoutSetOrApplied() {
s.Require().NoError(apitest.WaitForAlive(s.ctx, clientV2))

testJob := model.NewJob()
specOpts := []job.SpecOpt{}
specOpts := []legacy_job.SpecOpt{}
if tc.timeoutSpecified != nil {
specOpts = append(specOpts, job.WithTimeout(int64(tc.timeoutSpecified.Seconds())))
specOpts = append(specOpts, legacy_job.WithTimeout(int64(tc.timeoutSpecified.Seconds())))
}
testJob.Spec, err = job.MakeSpec(specOpts...)
testJob.Spec, err = legacy_job.MakeSpec(specOpts...)
s.Require().NoError(err)

returnedJob, err := client.Submit(s.ctx, testJob)
Expand Down
22 changes: 11 additions & 11 deletions cmd/cli/wasm/wasm_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"

legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/models/migration/legacy"
"github.com/ipfs/go-cid"
"github.com/pkg/errors"
Expand All @@ -23,7 +24,6 @@ import (
"github.com/bacalhau-project/bacalhau/cmd/util/parse"
"github.com/bacalhau-project/bacalhau/cmd/util/printer"
"github.com/bacalhau-project/bacalhau/pkg/executor/wasm"
"github.com/bacalhau-project/bacalhau/pkg/job"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/storage/inline"
"github.com/bacalhau-project/bacalhau/pkg/util/closer"
Expand Down Expand Up @@ -146,7 +146,7 @@ func runWasm(cmd *cobra.Command, args []string, opts *WasmRunOptions) error {
return fmt.Errorf("creating job: %w", err)
}

if err := job.VerifyJob(ctx, j); err != nil {
if err := legacy_job.VerifyJob(ctx, j); err != nil {
return fmt.Errorf("verifying job: %w", err)
}

Expand Down Expand Up @@ -197,24 +197,24 @@ func CreateJob(ctx context.Context, cmdArgs []string, opts *WasmRunOptions) (*mo
return nil, fmt.Errorf("wasm env vars invalid: %w", err)
}

spec, err := job.MakeWasmSpec(
spec, err := legacy_job.MakeWasmSpec(
*entryModule, opts.Entrypoint, parameters, wasmEnvvar, opts.ImportModules,
job.WithResources(
legacy_job.WithResources(
opts.ResourceSettings.CPU,
opts.ResourceSettings.Memory,
opts.ResourceSettings.Disk,
opts.ResourceSettings.GPU,
),
job.WithNetwork(
legacy_job.WithNetwork(
opts.NetworkingSettings.Network,
opts.NetworkingSettings.Domains,
),
job.WithTimeout(opts.SpecSettings.Timeout),
job.WithInputs(opts.SpecSettings.Inputs.Values()...),
job.WithOutputs(outputs...),
job.WithAnnotations(labels...),
job.WithNodeSelector(nodeSelectorRequirements),
job.WithDeal(
legacy_job.WithTimeout(opts.SpecSettings.Timeout),
legacy_job.WithInputs(opts.SpecSettings.Inputs.Values()...),
legacy_job.WithOutputs(outputs...),
legacy_job.WithAnnotations(labels...),
legacy_job.WithNodeSelector(nodeSelectorRequirements),
legacy_job.WithDeal(
opts.DealSettings.TargetingMode,
opts.DealSettings.Concurrency,
),
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/rs/zerolog/log"

"github.com/bacalhau-project/bacalhau/cmd/util/flags/cliflags"
"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
)

Expand All @@ -15,7 +15,7 @@ func ExecuteJob(ctx context.Context,
j *model.Job,
runtimeSettings *cliflags.RunTimeSettingsWithDownload,
) (*model.Job, error) {
err := job.VerifyJob(ctx, j)
err := legacy_job.VerifyJob(ctx, j)
if err != nil {
log.Ctx(ctx).Err(err).Msg("Job failed to validate.")
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/flags/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"golang.org/x/exp/slices"

"github.com/bacalhau-project/bacalhau/cmd/util/output"
"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/logger"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/node"
Expand Down Expand Up @@ -333,7 +333,7 @@ func URLFlag(value **url.URL, schemes ...string) *ValueFlag[*url.URL] {

func parseTag(s string) (string, error) {
var err error
if !job.IsSafeAnnotation(s) {
if !legacy_job.IsSafeAnnotation(s) {
err = fmt.Errorf("%q is not a valid tag", s)
}
return s, err
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/opts/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"fmt"
"strings"

"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
flag "github.com/spf13/pflag"
)
Expand Down Expand Up @@ -57,7 +57,7 @@ func (o *PublisherOpt) Set(value string) error {
return fmt.Errorf("invalid publisher option: %s", field)
}
}
v, err := job.PublisherStringToPublisherSpec(destinationURI, options)
v, err := legacy_job.PublisherStringToPublisherSpec(destinationURI, options)
o.value = &v
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/opts/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"net/url"
"strings"

"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
flag "github.com/spf13/pflag"
)
Expand Down Expand Up @@ -70,7 +70,7 @@ func (o *StorageOpt) Set(value string) error {
return fmt.Errorf("unpexted key %s in field %s", key, field)
}
}
storageSpec, err := job.ParseStorageString(sourceURI, destination, options)
storageSpec, err := legacy_job.ParseStorageString(sourceURI, destination, options)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/util/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"k8s.io/apimachinery/pkg/labels"

"github.com/bacalhau-project/bacalhau/cmd/util/flags"
"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/model"
)

Expand All @@ -20,7 +20,7 @@ func Labels(ctx context.Context, labels []string) ([]string, error) {
var jobAnnotations []string
var unSafeAnnotations []string
for _, a := range labels {
if job.IsSafeAnnotation(a) && a != "" {
if legacy_job.IsSafeAnnotation(a) && a != "" {
jobAnnotations = append(jobAnnotations, a)
} else {
unSafeAnnotations = append(unSafeAnnotations, a)
Expand Down
6 changes: 3 additions & 3 deletions cmd/util/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/bacalhau-project/bacalhau/pkg/job"
legacy_job "github.com/bacalhau-project/bacalhau/pkg/legacyjob"
"github.com/bacalhau-project/bacalhau/pkg/logger"
"github.com/bacalhau-project/bacalhau/pkg/model"
"github.com/bacalhau-project/bacalhau/pkg/setup"
Expand Down Expand Up @@ -45,7 +45,7 @@ func (s *UtilsSuite) TestSafeRegex() {
}

for _, tc := range tests {
strippedString := job.SafeStringStripper(tc.stringToTest)
strippedString := legacy_job.SafeStringStripper(tc.stringToTest)
require.LessOrEqual(s.T(), len(strippedString), len(tc.stringToTest))
if tc.predictedLength >= 0 {
require.Equal(s.T(), tc.predictedLength, len(strippedString))
Expand Down Expand Up @@ -148,7 +148,7 @@ func (s *UtilsSuite) TestImages() {
s.Run(name, func() {
sampleJob, _ := model.NewJobWithSaneProductionDefaults()
sampleJob.Spec.EngineSpec = model.NewDockerEngineBuilder(test.Image).Build()
err := job.VerifyJob(context.TODO(), sampleJob)
err := legacy_job.VerifyJob(context.TODO(), sampleJob)
if test.Valid {
require.NoError(s.T(), err, "%s: expected valid image %s to pass", name, test.Image)
} else {
Expand Down

0 comments on commit 9f9ae95

Please sign in to comment.