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

Revert "Fix unpack job cache issue (#3204)" #3211

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

dtfranz
Copy link
Contributor

@dtfranz dtfranz commented Apr 24, 2024

This reverts commit 47aaa6b.

We previously thought this would:

  1. Fix jobs not getting listed due to labeler changes
  2. Add the labels to jobs when creating existing jobs then updating instead

Instead we've found that jobs created in 4.14 already meet the requirements to be listed by the labeller and that also updating the jobs in this manner can be detrimental because every field in the pod template is immutable and thus generates lots of errors when the update runs.

We'll need to find out why the jobs aren't getting listed and how to handle that in this case in another way. We think the root cause has something to do with the filtering we do here.

@grokspawn grokspawn added this pull request to the merge queue Apr 25, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 25, 2024
@grokspawn grokspawn removed this pull request from the merge queue due to a manual request Apr 25, 2024
@grokspawn grokspawn added this pull request to the merge queue Apr 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 25, 2024
@dinhxuanvu
Copy link
Member

dinhxuanvu commented Apr 25, 2024

Well, for this list call, you should use this func SelectorFromValidatedSet or ValidatedSelectorFromSet here to properly validate and construct the label selector. That should avoid potential bad list call to the apiserver.

On a related note, there is no point to update the pod given the pod is created by the job. The proper flow should be to delete the pod and the job and then recreate the job in case of failed pod or in case of rerunning the job (the job cannot be rereun without recreating).

@dinhxuanvu dinhxuanvu added this pull request to the merge queue Apr 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 25, 2024
@dinhxuanvu dinhxuanvu added this pull request to the merge queue Apr 25, 2024
Merged via the queue into operator-framework:master with commit 7ec4770 Apr 25, 2024
14 checks passed
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

Successfully merging this pull request may close these issues.

2 participants