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

Update Job settings #14

Open
grossvogel opened this issue May 20, 2020 · 5 comments
Open

Update Job settings #14

grossvogel opened this issue May 20, 2020 · 5 comments

Comments

@grossvogel
Copy link
Collaborator

grossvogel commented May 20, 2020

  1. When deleting a job, pass a request body containing a DeleteOptions object like this
  2. Reduce the default number of build retries as in the PR linked above as well
  3. Recognize a case when the job status changes to failed w/o being caused by too many retries
@grossvogel grossvogel self-assigned this Jun 3, 2020
@grossvogel
Copy link
Collaborator Author

Working on this now

@grossvogel
Copy link
Collaborator Author

grossvogel commented Jun 3, 2020

Hmm... the k8s API doesn't appear to support DeleteOptions out of the box. I thought I figured out how to pass the body with the delete request, but it doesn't seem to be doing what I'd expect.

The pods created by the jobs don't appear to have an ownerReference set, so I suspect they're not being recognized as dependents.

@vaxinate
Copy link

vaxinate commented Jun 3, 2020

hhmmm, I didn't have to do anything on the TSS cluster to enable passing DeleteOptions, so that seems weird. I just put them as the body of my DELETE request: https://github.com/revelrylabs/review_bot/blob/master/bot/lib/Kube.js#L172

@grossvogel
Copy link
Collaborator Author

The elixir k8s API is just a bit higher-level so the functions they provide for delete assume there'll be no request body. I manually created a delete operation that had data where the body usually goes, which I would expect to work, but I haven't gotten into the lower level stuff to see if the body is being sent.

I also noticed the ownerReference thing... if you have insight as to whether that matters lemme know.

@vaxinate
Copy link

vaxinate commented Jun 3, 2020

Ah i see so it's just a problem of the API client included in Bonny not supporting it. That makes sense.

@grossvogel grossvogel removed their assignment Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants