Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for deploying Cloud Run Jobs. #7915

Merged
merged 6 commits into from
Nov 8, 2022

Conversation

bskaplan
Copy link
Contributor

@bskaplan bskaplan commented Oct 5, 2022

Related: #7914

Description
Add initial support for deploying Jobs in the Cloud Run deployer.

User facing changes (remove if N/A)
The Cloud Run deployer will deploy a Job if provided with a manifest for one.

Follow-up Work (remove if N/A)

  • Support clean-up of Cloud Run Jobs.
  • Add an integration test (don't want to do that before I add clean-up support).

@codecov
Copy link

codecov bot commented Oct 5, 2022

Codecov Report

Merging #7915 (c76a652) into main (290280e) will decrease coverage by 3.80%.
The diff coverage is 54.09%.

@@            Coverage Diff             @@
##             main    #7915      +/-   ##
==========================================
- Coverage   70.48%   66.68%   -3.81%     
==========================================
  Files         515      596      +81     
  Lines       23150    29010    +5860     
==========================================
+ Hits        16317    19344    +3027     
- Misses       5776     8235    +2459     
- Partials     1057     1431     +374     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/test.go 44.44% <0.00%> (ø)
cmd/skaffold/app/exitcode.go 100.00% <ø> (+6.66%) ⬆️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/render.go 35.48% <18.18%> (-5.90%) ⬇️
cmd/skaffold/app/cmd/lsp.go 28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/run.go 64.28% <33.33%> (-9.63%) ⬇️
cmd/skaffold/app/cmd/fix.go 56.41% <37.50%> (-20.07%) ⬇️
... and 391 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ericzzzzzzz ericzzzzzzz added the kokoro:run runs the kokoro jobs on a PR label Oct 6, 2022
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Oct 6, 2022
@tejal29
Copy link
Contributor

tejal29 commented Oct 7, 2022

@bskaplan we could potentially merge this before 2.0.0 release next week. Would that be good? Is there any follow up work for this feature completion e.g. feature work, metrics, tests ?

If yes, lets wait before we merge this in.

@bskaplan
Copy link
Contributor Author

bskaplan commented Oct 8, 2022

There's no hurry to merge this. I still plan on adding an integration test after this PR.

@aaron-prindle
Copy link
Contributor

aaron-prindle commented Nov 8, 2022

Sorry for the delay here, looking at this now. In testing with the following yaml I was not able to deploy a Job building from this PR (more info below). Do you have a small test project that this works against currently that you could share? :

job.yaml

apiVersion: run.googleapis.com/v1
kind: Job
metadata:
  name: job-v2
  annotations:
    run.googleapis.com/launch-stage: BETA
spec:
  template:
    metadata:
      annotations:
        run.googleapis.com/client-name: cloud-console
        client.knative.dev/user-image: us-docker.pkg.dev/cloudrun/container/job:latest
        run.googleapis.com/execution-environment: gen2
    spec:
      taskCount: 1
      template:
        spec:
          containers:
          - image: us-docker.pkg.dev/cloudrun/container/job:latest
            resources:
              limits:
                cpu: 1000m
                memory: 512Mi
          maxRetries: 0
          timeoutSeconds: '600'
          serviceAccountName: <project-#>-compute@developer.gserviceaccount.com

skaffold.yaml

apiVersion: skaffold/v3
kind: Config
metadata:
   name: cloud-run-test
manifests:
  rawYaml:
    - job.yaml
deploy:
  cloudrun:
    projectid: aprindle-test-cluster
    region: us-central1

skaffold logs:

aprindle@aprindle-ssd ~/deploy-cloudrun-job $ skaffold dev --default-repo=gcr.io/aprindle-test-cluster
Listing files to watch...
Generating tags...
Checking cache...
Tags used in deployment:
Starting deploy...
Deploying Cloud Run service:
	 job-v2
Cloud Run Job job-v2 finished: Job started. 0/1 deployment(s) still pending
Press Ctrl+C to exit
Watching for changes...
# < ---- hangs indefinitely w/ no logs/deploying
^CCleaning up...
WARN[0008] deployer cleanup:googleapi: Error 404: Resource 'job-v2' of kind 'SERVICE' in region 'us-central1' in project 'aprindle-test-cluster' does not exist.
Details:
[
  {
    "@type": "type.googleapis.com/google.rpc.DebugInfo",
    "detail": "[ORIGINAL ERROR] generic::not_found: userFacingMessage: Resource 'job-v2' of kind 'SERVICE' in region 'us-central1' in project 'aprindle-test-cluster' does not exist.; \ncom.google.cloud.eventprocessing.serverless.error.NotFoundException: userFacingMessage: Resource 'job-v2' of kind 'SERVICE' in region 'us-central1' in project 'aprindle-test-cluster' does not exist.;  Code: NOT_FOUND [google.rpc.error_details_ext] { message: \"Resource \\'job-v2\\' of kind \\'SERVICE\\' in region \\'us-central1\\' in project \\'aprindle-test-cluster\\' does not exist.\" }"
  }
]  subtask=-1 task=DevLoop


@aaron-prindle
Copy link
Contributor

aaron-prindle commented Nov 8, 2022

Ah it seems it did create the job I specified - job-v2, awesome!:
image

It did not execute the job though which I think would be an expectation/desired-feature from a user. Perhaps we can also support Execution as well

Copy link
Contributor

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

LGTM. Perhaps we can discuss a bit more the vision for Job vs Execution support in the integration test PR as I am still a bit confused how a user can execute a job from skaffold atm

@aaron-prindle aaron-prindle merged commit 8dc3b9b into GoogleContainerTools:main Nov 8, 2022
@bskaplan
Copy link
Contributor Author

bskaplan commented Nov 8, 2022

Yeah, I couldn't figure out a way to map Executions to Skaffold's existing functionality. I figured it was still worth it to have support in Skaffold in case you wanted to have a Job managed by a Service (where the Service would trigger the executions to do asynchronous work).

@aaron-prindle
Copy link
Contributor

Ah this makes sense to me now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants