CI Director

Merge lp://staging/~sinzui/ci-director/pub-win-jobs-be-resourceful into lp://staging/ci-director

  1. pub-win-jobs-be-resourceful
  2. Merge into trunk
Proposed by Curtis Hovey
Status: Merged
Merged at revision: 120
Proposed branch: lp://staging/~sinzui/ci-director/pub-win-jobs-be-resourceful
Merge into: lp://staging/ci-director
Diff against target: 246 lines (+109/-23)
4 files modified
cidirector/cidirector.py (+7/-12)
cidirector/storage.py (+20/-1)
cidirector/tests/test_cidirector.py (+24/-10)
cidirector/tests/test_storage.py (+58/-0)
To merge this branch: bzr merge lp://staging/~sinzui/ci-director/pub-win-jobs-be-resourceful
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+227264@code.staging.launchpad.net

Description of the change

This branch addresses the case where the publication and both win jobs are created even though they were configured to be resourceful. The state file had the old jobs and the new jobs. ci-director saw the entry for the resourceful job and started a build. When it completed, the specialised job got the status update...so ci started the resourceful job again. I unconfigured the publish-revision job and both win jobs to restore CI operation. All other jobs are configured; I am confident the group-name, requires, conflicts, and failure-threshold work. We are not using the test/functional/unitest jobs now.

I believe CIDirector.update_builds() was creating the specialised jobs. I failed to include this in the my initial work, possibly because this method worked with the StateFile directly. It took some time to reproduce the the "state" of the state file I saw yesterday to understand how this method was causing the problem.

I Found that adding "data.setdefault('failures', 0)" to StateFileJob.__init__() made the test data more like the real data, the problem I saw in production appeared in the test I wrote for update_builds(). This change makes iter_grouped_jobs() more accurate in finding jobs.

I changed current_version_published() to find the Resourceful job in the state file then fall back to publication_job(). This may have been an issue last night.

I changed update_builds() to use iter_grouped_jobs() to find all the jobs (that exist at that moment), then only add the specialized jobs when the resourceful statefilejob didn't exist. Note that I made the publicationjob subordinate to the "current_version is not None" block. There was no test failures when changed the indentation. I also tested the indentation change with the previous rules and didn't see a test change. I deleted test_update_builds_skips_tests_when_unpublished() because iter_grouped_jobs() doesn't see the "tests" jobs different from other jobs. I don't think they are different from functional tests, so I think the test/rule was not required.

5 tests failed when I changed update_builds(). The tests related to update_from_jenkins() but the errors showed up in FakeJenkins. I opted to teach the method to avoid the missing KeyError by adapting the specialised job names to the resourceful names used by iter_grouped_jobs(). This ensures compatibility with the tests, but I am not sure this would be an issue in production when all the jobs would be configured to be resourceful. I didn't want to change the tests to force a pass, so I added the adapter.

To post a comment you must log in.
lp://staging/~sinzui/ci-director/pub-win-jobs-be-resourceful updated
120. By Curtis Hovey

Merged trunk and resolved conflicts

121. By Curtis Hovey

Push the rule to find resourceful publicatin jobs in to publication_job which is simpler to test.

122. By Curtis Hovey

Update SF.get_build_windows_installer_job() to return resourceful jobs.

123. By Curtis Hovey

Update get_windows_deploy_job() to return resourceful jobs.

124. By Curtis Hovey

Oops, the SFJ wants an failure_threshold, not data.

125. By Curtis Hovey

Update CID.update_builds() to build a set of jobs to simplify the condition of
adding jobs to build.

126. By Curtis Hovey

Restore the line postions.

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Aaron.

I have made several simplifations largely from your encouragement to push the resourceful job detection into the state-file.
   * We can trust that sf.publication_jib, sf.get_build_windows_installer_job() and sf.get_windows_deploy_job()
     will return the job created by ResourcefulJob.
     * Note that I remove the guard for pub_job being None because it cannot be done when subordinate to
       if current_version is not None:
   * With my change to summary in my previous branch, the jobs are hashable so that I can use a set()
     to gather all jobs.
   * Reverted other changes to CIDirector and its tests since it didn't need extra logic to filter duplicate jobs.

Revision history for this message
Aaron Bentley (abentley) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches