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

Fix Check Job Creation escapade #266

Merged
merged 2 commits into from
Jan 11, 2021
Merged

Fix Check Job Creation escapade #266

merged 2 commits into from
Jan 11, 2021

Conversation

cimnine
Copy link
Contributor

@cimnine cimnine commented Jan 8, 2021

Summary

Besides two refactoring (which are their own commits and which I could extract into their own PR if needed), this PR aims to fix that a lot of Check jobs are created and scheduled, even though they already exist, rsp. have been scheduled before.
The culprit I have identified was that the Check.Status.Started property was never set. But it's that property which the check handler uses to determine whether it should create & schedule another check job.

The logic of the Check handler was adjusted to match the logic of the other handlers in that regard.

This PR is part of the investigations of #256 and is based on #260. Fixes #257 .

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking,
    as they show up in the changelog
  • Update the documentation.
  • Update tests.
  • Link this PR to related issues.

@cimnine cimnine added the bug Something isn't working label Jan 8, 2021
@cimnine cimnine changed the base branch from master to 256PassBackendConfig January 8, 2021 11:08
@cimnine
Copy link
Contributor Author

cimnine commented Jan 8, 2021

Rebased to include latest changes from #260

This fix is implemented on two levels:
First, when the Check handler tries to create a job
and it exists, it will not attemp to schedule the job again.
Second, it actually sets the Status.Started property on a
Check resource once the job is scheduled, which prevents
jobs to be created a second time in the first place.
Base automatically changed from 256PassBackendConfig to master January 11, 2021 16:11
@cimnine cimnine marked this pull request as ready for review January 11, 2021 16:11
@cimnine cimnine merged commit b533cd5 into master Jan 11, 2021
@cimnine cimnine deleted the 256CreateThemAll branch January 11, 2021 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

K8up spams the same jobs to the queue
2 participants