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

Allow client to restart a build for a jobgroup #1651

Merged
merged 4 commits into from
May 17, 2018

Conversation

Martchus
Copy link
Contributor

See https://progress.opensuse.org/issues/33892


It would work like this:

# just get the list of jobs
openqa-client jobs/overview build=589.1 distri=sle version=15 groupid=123
[
  {
    id => 28,
    name => "sle-15-Installer-DVD-x86_64-Build589.1-create_hdd_minimal_base+sdk\@64bit",
  },
  {
    id => 27,
    name => "sle-15-Installer-DVD-x86_64-Build589.1-install_ltp+sle+15+Installer-DVD\@64bit",
  },
]

# restart such jobs
openqa-client jobs/overview/restart build=589.1 distri=sle version=15 groupid=123
[
  {
    id => 28,
    name => "sle-15-Installer-DVD-x86_64-Build589.1-create_hdd_minimal_base+sdk\@64bit",
  },
  {
    id => 27,
    name => "sle-15-Installer-DVD-x86_64-Build589.1-install_ltp+sle+15+Installer-DVD\@64bit",
  },
]
http://localhost:9526/api/v1/jobs/restart?jobs=28&jobs=27
{ result => [37], test_url => ["/tests/37"] }

That should already cover AC1:

AC1: openQA::Client is able to trigger a build only for the specified jobgroup


Not sure how to deal with jobs which are already running/scheduled. That's what currently happens:

  • If a job is already running, it is canceled and a new job rescheduled. The canceled job has the result 'user_restarted'.
  • If a job is already scheduled, it is skipped and a new job rescheduled.

AC2: Builds that were triggered in the normal way, are not obsoleted or canceled

So jobs which are already running or are scheduled should be excluded?


AC3: Builds for a jobgroup that were previously scheduled can be handled in a similar fashion as other builds (Parameters, et al)

Not sure what that means.


Since @foursixnine mentioned that multi-machine tests might be problematic, I assume we can't just rely on the scheduler to figure this?

@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #1651 into master will increase coverage by 5.68%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1651      +/-   ##
==========================================
+ Coverage   83.16%   88.85%   +5.68%     
==========================================
  Files         132      132              
  Lines        9452     9458       +6     
==========================================
+ Hits         7861     8404     +543     
+ Misses       1591     1054     -537
Impacted Files Coverage Δ
lib/OpenQA/WebAPI/Controller/API/V1/Job.pm 85.38% <100%> (+1.79%) ⬆️
lib/OpenQA/WebAPI.pm 91.84% <100%> (+0.02%) ⬆️
lib/OpenQA/Utils.pm 93.44% <100%> (+4.02%) ⬆️
lib/OpenQA/WebAPI/Controller/Test.pm 96.18% <100%> (-0.3%) ⬇️
lib/OpenQA/Task/Asset/Limit.pm 96.29% <0%> (ø) ⬆️
lib/OpenQA/WebAPI/Plugin/Helpers.pm 94.28% <0%> (+0.57%) ⬆️
lib/OpenQA/Scheduler/Scheduler.pm 89.06% <0%> (+0.78%) ⬆️
lib/OpenQA/Worker/Common.pm 80.86% <0%> (+1.44%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6967914...31926a8. Read the comment docs.

@okurz
Copy link
Member

okurz commented May 17, 2018

AC2: Builds that were triggered in the normal way, are not obsoleted or canceled
So jobs which are already running or are scheduled should be excluded?

I think
https://github.com/os-autoinst/openQA/blob/master/docs/UsersGuide.asciidoc#spawning-multiple-jobs-based-on-templates---isos-post
describes best the different use cases that are there.

@Martchus
Copy link
Contributor Author

As @coolo says in the ticket, we misunderstood the problem.

I think the generic verb 'trigger' used in the issue lead to the confusion (at least for me). Seems like 'Spawning jobs for a build filtered by job group' was mean and not 'Restart jobs ...'.

I created a separate PR for the first commit.

If you like the refactoring, increased test coverage and the 'test overview' API route, you can still merge this PR.

But to solve the issue, I'll have to switch the approach. I've read the documentation @okurz mentioned and I think the approach of @foursixnine makes sense. Alternatively, the POST isos route could be extended to accept a job group ID directly.

@Martchus Martchus changed the title WIP: Allow client to restart a build for a jobgroup Allow client to restart a build for a jobgroup May 17, 2018
@coolo
Copy link
Contributor

coolo commented May 17, 2018

I would go for POST to isos accepts a _GROUP paramter filtering the created jobs before scheduling them.

@coolo coolo merged commit 04373a1 into os-autoinst:master May 17, 2018
@Martchus Martchus deleted the restart_filtered_jobs branch May 17, 2018 10:00
@Martchus
Copy link
Contributor Author

Thanks for merging. Unfortunately we now have the commit 'Update documentation about isos post in client help' two times in the history.

@coolo
Copy link
Contributor

coolo commented May 17, 2018

I noticed, but it's fortunately the same documentation :)

coolo pushed a commit that referenced this pull request May 17, 2018
commit 04373a1
Merge: eec584f 31926a8
Author:     Stephan Kulow <stephan@kulow.org>
AuthorDate: Thu May 17 11:58:59 2018 +0200
Commit:     GitHub <noreply@github.com>
CommitDate: Thu May 17 11:58:59 2018 +0200

    Merge pull request #1651 from Martchus/restart_filtered_jobs

    Allow client to restart a build for a jobgroup
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.

None yet

3 participants