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

Automatic updates from tags #3168

Merged
merged 19 commits into from May 29, 2019

Conversation

3 participants
@nphilipp
Copy link
Member

commented Apr 18, 2019

This allows configuring releases for automatic updates from their respective candidate tag through the new Release.create_automatic_updates flag (which is settable via the CLI).

This allows configuring automatic updates for releases (e.g. Rawhide, see issue #3007) like this:

<releasename>.autoupdate.from_tags = tag1[, tag2...]

The configured release must be added to the database beforehand, otherwise the handler will bail out at some point (when it doesn't find the release in question).

As discussed, it also introduces a new enum value, UpdateType.unspecified because we don't know of what type the created updates are.

Open questions, why I marked this as WIP/draft:

  • Rawhide is different from branched releases. It doesn't have an associated version number for starters and it will probably be created once and stay in 'pending' forever. The only thing that will change with the current setup is, as I see it, the tag with which Rawhide builds get tagged with. Right now, one would have to give it a random, suitably high version number (e.g. 1000) to not violate the NOT NULL constraint on the DB column and avoid tracebacks from the version_int property. Shall we keep it like this, or allow NULL or empty versions (and let version_int cope with this)?

  • While working on this, I had the idea that it may be better to record information that maps tags to releases for automatic updates with the Release object instead of the instance configuration, to avoid having to change things in two places at branching (creating the new branched release, updating the Rawhide release in DB, tag->release mapping in configuration). This would require some changes to the DB schema, in the simplest case e.g. a create_automatic_updates flag in the release which would express that builds tagged into the respective dist_tag get an update created automatically. If we want more flexibility (e.g. from other tags associated with the release, or even more than one tag), it'd probably require more involved additions to the schema. Is doing it in configuration (with the req'd restart of the instance after changes) good enough for now, or shall I work on one of the options to specify this in the DB (and expose it via API)?

@nphilipp nphilipp changed the title Automatic updates from z Automatic updates from tags Apr 18, 2019

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from b630b9d to efd759a Apr 25, 2019

@nphilipp nphilipp marked this pull request as ready for review Apr 25, 2019

@nphilipp nphilipp requested a review from fedora-infra/bodhi as a code owner Apr 25, 2019

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 1, 2019

* Rawhide is different from branched releases. It doesn't have an associated version number for starters and it will probably be created once and stay in 'pending' forever. The only thing that will change with the current setup is, as I see it, the tag with which Rawhide builds get tagged with. Right now, one would have to give it a random, suitably high version number (e.g. 1000) to not violate the NOT NULL constraint on the DB column and avoid tracebacks from the `version_int` property. Shall we keep it like this, or allow NULL or empty versions (and let `version_int` cope with this)?

Rawhide does actually have a version number - it's currently 31. I think we will probably just set it to that number, and every time we branch we will raise the number and then create the branched release.

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 1, 2019

* While working on this, I had the idea that it may be better to record information that maps tags to releases for automatic updates with the `Release` object instead of the instance configuration, to avoid having to change things in two places at branching (creating the new branched release, updating the Rawhide release in DB, tag->release mapping in configuration). This would require some changes to the DB schema, in the simplest case e.g. a `create_automatic_updates` flag in the release which would express that builds tagged into the respective `dist_tag` get an update created automatically. If we want more flexibility (e.g. from other tags associated with the release, or even more than one tag), it'd probably require more involved additions to the schema. Is doing it in configuration (with the req'd restart of the instance after changes) good enough for now, or shall I work on one of the options to specify this in the DB (and expose it via API)?

We do map tags to a release in the database today:

dist_tag = Column(Unicode(20), nullable=False)
stable_tag = Column(UnicodeText, nullable=False)
testing_tag = Column(UnicodeText, nullable=False)
candidate_tag = Column(UnicodeText, nullable=False)
pending_signing_tag = Column(UnicodeText, nullable=False)
pending_testing_tag = Column(UnicodeText, nullable=False)
pending_stable_tag = Column(UnicodeText, nullable=False)
override_tag = Column(UnicodeText, nullable=False)

Or do you mean something different than that?

Show resolved Hide resolved bodhi/server/buildsys.py
Show resolved Hide resolved bodhi/server/consumers/automatic_updates.py Outdated
Show resolved Hide resolved bodhi/server/consumers/automatic_updates.py Outdated
Show resolved Hide resolved production.ini Outdated

@nphilipp nphilipp requested a review from cverna May 2, 2019

@nphilipp nphilipp added this to To do in CI Gating via automation May 2, 2019

@nphilipp nphilipp added the WIP label May 2, 2019

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from efd759a to 7c2dd99 May 2, 2019

@nphilipp

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

The above is missing the bits allowing to configure automatic updates as a flag on Release from the respective candidate tag (instead of instance configuration). I'm working on it.

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch 2 times, most recently from 79f1a16 to 8a90779 May 2, 2019

@nphilipp nphilipp removed the WIP label May 3, 2019

@nphilipp

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

The latest two commits should move the configuration of automatic updates from the instance configuration to the Release model, respectively the create_automatic_updates boolean.

@cverna cverna moved this from To do to Need Review in CI Gating May 6, 2019

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from 8a90779 to 10d0b9c May 6, 2019

@nphilipp nphilipp requested a review from bowlofeggs May 6, 2019

@nphilipp nphilipp added the Migration label May 6, 2019

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from 10d0b9c to 828e17b May 6, 2019

@nphilipp

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

Hmm, looks like something failed in the CI tests.

java.lang.IllegalArgumentException: The supplied credentials are invalid to login
	at org.jenkinsci.plugins.pipeline.githubstatusnotification.GitHubStatusNotificationStep.getGitHubIfValid(GitHubStatusNotificationStep.java:263)
	at org.jenkinsci.plugins.pipeline.githubstatusnotification.GitHubStatusNotificationStep.getRepoIfValid(GitHubStatusNotificationStep.java:268)
	at org.jenkinsci.plugins.pipeline.githubstatusnotification.GitHubStatusNotificationStep.access$100(GitHubStatusNotificationStep.java:79)
	at org.jenkinsci.plugins.pipeline.githubstatusnotification.GitHubStatusNotificationStep$Execution.run(GitHubStatusNotificationStep.java:373)
	at org.jenkinsci.plugins.pipeline.githubstatusnotification.GitHubStatusNotificationStep$Execution.run(GitHubStatusNotificationStep.java:355)
	at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1$1.call(AbstractSynchronousNonBlockingStepExecution.java:47)
	at hudson.security.ACL.impersonate(ACL.java:290)
	at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1.run(AbstractSynchronousNonBlockingStepExecution.java:44)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Finished: FAILURE

Is there a way to resubmit?

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 7, 2019

@cverna

This comment has been minimized.

Copy link
Member

commented May 7, 2019

On Tue, 2019-05-07 at 05:42 -0700, Nils Philippsen wrote: Is there a way to resubmit?
Sadly there's not a formal way to get it to re-run. When things like this happen, I make an empty commit, force push, then remove the empty commit and force push again.

I think you can also use git commit --amend --no-edit and force push

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch 2 times, most recently from aba8d56 to 56b19ee May 7, 2019

@cverna

This comment has been minimized.

Copy link
Member

commented May 28, 2019

One more thing we need to manage the unspecified type on the UI, it is currently not styled and the text is white on a white background so not ideal :)

I think it just need a new value here -->

cls = {

Similar to what is done here -->

cls = {

See image here https://paste.opensuse.org/view/raw/5218800

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from 11f7658 to 4b48f7a May 28, 2019

@nphilipp

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

The latest push fixes the above issues.

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from 4b48f7a to 94d8c4f May 28, 2019

@nphilipp

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

And I've rebased this on top of #3247 because otherwise I couldn't test the latest commit fixing CSS classes for 'unspecified' updates.

nphilipp added some commits Apr 18, 2019

Add UpdateType.unspecified enum.
This is used for automatically created updates because we can't know
what type of an update some particular build is.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Add AutomaticUpdateHandler.
This handler automatically creates updates when builds are tagged into
candidate tags of releases configured for automatic updates.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Release.version_int(): reuse compiled regex
Signed-off-by: Nils Philippsen <nils@redhat.com>
Allow test methods to receive pytest fixtures directly.
Methods in test cases derived from unittest.TestCase can't directly
receive pytest fixture arguments, e.g. to capture log messages.

This change moves the functionality into the BaseTestCaseMixin class and
derives two subclasses from it:

- BaseTestCase: inherits unittest.TestCase, for existing users of the class
- BasePyTestCase: for new users, lets test methods directly receive fixture
  arguments

Signed-off-by: Nils Philippsen <nils@redhat.com>
Add unit tests for automatic updates.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Cope with pytest < 3.7 when capturing log output.
The logging capture fixture in version of pytest < 3.7 doesn't have the
'messages' property yet. Monkey-patch old versions of the class so it
can be used uniformly in tests.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Verify the type of automatically created updates.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Expose mock koji build data.
This allows verifying certain properties in tests using the mock
DevBuildsys.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Verify the user of an automatically created update.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Test handling of Koji builds without owner.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Add Release.create_automatic_updates boolean.
This is used to mark that updates should be created automatically for
Koji builds tagged into the candidate tag of a Release.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Create automatic updates with Koji build user.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Test automatic updates with/without existing user.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Allow creating/editing releases with automatic updates.
With this, users can specify --create-automatic-updates or
--no-create-automatic-updates when creating or editing releases to set
the relevant flag.

Signed-off-by: Nils Philippsen <nils@redhat.com>
CLI: show value of create_automatic_updates flag.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Test editing of the create_automatic_updates flag.
Signed-off-by: Nils Philippsen <nils@redhat.com>
Set request for automatically created updates.
This ensures that the creation of an update is announced on the
messaging bus.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Test missing message elements individually.
Previously, the test would remove all elements to be tested from the
message, now each one gets tested on its own.

Signed-off-by: Nils Philippsen <nils@redhat.com>
Add CSS class to updates of type 'unspecified'.
Without an appropriate CSS class, the update type would be rendered white
on white background.

Signed-off-by: Nils Philippsen <nils@redhat.com>

@nphilipp nphilipp force-pushed the nphilipp:develop--automatic-updates branch from 94d8c4f to 1e5d9f1 May 29, 2019

@cverna

cverna approved these changes May 29, 2019

Copy link
Member

left a comment

👍 Looks good to me 🎆 . Thanks @nphilipp

@mergify mergify bot merged commit 70ba6b0 into fedora-infra:develop May 29, 2019

28 of 30 checks passed

continuous-integration/jenkins/pr-head This commit is being built
Details
rawhide-integration running
Details
DCO DCO
Details
Summary 2 rules match and 4 potential rules
Details
f29-build
Details
f29-diff-cover
Details
f29-docs
Details
f29-integration
Details
f29-integration-build
Details
f29-unit
Details
f30-build
Details
f30-diff-cover
Details
f30-docs
Details
f30-integration
Details
f30-integration-build
Details
f30-unit
Details
pip-build
Details
pip-diff-cover
Details
pip-docs
Details
pip-flake8
Details
pip-integration
Details
pip-integration-build
Details
pip-mypy
Details
pip-pydocstyle
Details
pip-unit
Details
rawhide-build
Details
rawhide-diff-cover
Details
rawhide-docs
Details
rawhide-integration-build
Details
rawhide-unit
Details

CI Gating automation moved this from Need Review to Merged to develop May 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.