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.

Sign up for GitHub

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

Jump to bottom

[CI:BUILD] Cirrus: Publish binary artifacts on success #13559

Merged
merged 1 commit into from
Mar 23, 2022
Merged

[CI:BUILD] Cirrus: Publish binary artifacts on success #13559

merged 1 commit into from
Mar 23, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented Mar 18, 2022 •
edited
Loading

The 'Total Success' task only ever executes when all dependencies are
successful. Since other task names can vary across different OS releases,
their artifact URLs will be inconsistent over time. When a non
[CI:DOCS] build is successful, gather all binary/release artifacts
in a new task which depends on 'Total Success'. This will provide a
uniform name and URL for downstream users. For example:

https://api.cirrus-ci.com/v1/artifact/github/containers/podman/artifacts/binary.zip

or

https://api.cirrus-ci.com/v1/artifact/github/containers/podman/artifacts/binary/ FILENAME

Where FILENAME is one of:

  • podman
  • podman-remote
  • rootlessport
  • podman-release-386.tar.gz
  • podman-release-amd64.tar.gz
  • podman-release-arm64.tar.gz
  • podman-release-arm.tar.gz
  • podman-release-mips64le.tar.gz
  • podman-release-mips64.tar.gz
  • podman-release-mipsle.tar.gz
  • podman-release-mips.tar.gz
  • podman-release-ppc64le.tar.gz
  • podman-release-s390x.tar.gz
  • podman-remote-release-darwin_amd64.zip
  • podman-remote-release-darwin_arm64.zip
  • podman-remote-release-windows_amd64.zip
  • podman-v4.0.0-dev.msi

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 18, 2022
@cevich cevich requested a review from edsantiago March 18, 2022 19:46
@cevich cevich force-pushed the success_artifacts branch 3 times, most recently from 6f81837 to e894623 Compare March 21, 2022 17:13
@cevich cevich changed the title [WIP] [CI:BUILD] Cirrus: Publish binary artifacts on success [CI:BUILD] Cirrus: Publish binary artifacts on success Mar 21, 2022
@cevich cevich force-pushed the success_artifacts branch 4 times, most recently from dc465c5 to d0f5e5e Compare March 21, 2022 18:40
@cevich cevich requested a review from baude March 21, 2022 18:40
@cevich cevich marked this pull request as ready for review March 21, 2022 18:40
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 21, 2022
@cevich
Copy link
Member Author

cevich commented Mar 21, 2022

@baude asked for this for something. Are those binaries/tarballs enough? Is there some other specific CI artifact you'd like available from a consistently named URL?

@edsantiago
Copy link
Member

The commit message (and github comment) describe how you're solving the problem, but give no indication of what problem you're trying to solve. Could you include a 'why' portion, explaining the purpose behind all this?

edsantiago
edsantiago reviewed Mar 21, 2022
View reviewed changes
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the motivation here, nor do I really understand cirrus yaml anyway, so all I can do is comment on some unnecessary (to my eye) duplication

.cirrus.yml Outdated Show resolved Hide resolved
.cirrus.yml Outdated Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

The commit message (and github comment) describe how you're solving the problem, but give no indication of what problem you're trying to solve. Could you include a 'why' portion, explaining the purpose behind all this?

Brent asked for it for something, I didn't ask why at the time. It's for aardvark-dns and netavark CI, presumably some kind of reverse-dependency testing. Though in general having "CD" available as a CI-feature is good-practice. Heck, you could use it to do a binary-size check in about 3-lines of bash 😉 I'll update the commit and description accordingly.

@cevich
Cirrus: Publish binary artifacts on success
1a7f5b3 
In general continuous-delivery (CD) tends to pair well with CI.  More
specifically, there is a need for some reverse-dependency CI testing in
netavark/aardvark-dns.  In all cases, the download URL needs to remain
consistent, without elements like `Build%20for%20fedora-35`.

The 'Total Success' task only ever executes when all dependencies are
successful.  When a non `[CI:DOCS]` build is successful, gather all
binary/release artifacts in a new task which depends on 'Total Success'.
This will provide a uniform name (`artifacts`) and URL for downstream
users to use.  For example:

https://api.cirrus-ci.com/v1/artifact/github/containers/podman/artifacts/binary.zip

or

https://api.cirrus-ci.com/v1/artifact/github/containers/podman/artifacts/binary/FILENAME

Where ***FILENAME*** is one of:

* `podman`
* `podman-remote`
* `rootlessport`
* `podman-release-386.tar.gz`
* `podman-release-amd64.tar.gz`
* `podman-release-arm64.tar.gz`
* `podman-release-arm.tar.gz`
* `podman-release-mips64le.tar.gz`
* `podman-release-mips64.tar.gz`
* `podman-release-mipsle.tar.gz`
* `podman-release-mips.tar.gz`
* `podman-release-ppc64le.tar.gz`
* `podman-release-s390x.tar.gz`
* `podman-remote-release-darwin_amd64.zip`
* `podman-remote-release-darwin_arm64.zip`
* `podman-remote-release-windows_amd64.zip`
* `podman-v4.0.0-dev.msi`

Signed-off-by: Chris Evich <cevich@redhat.com>
@cevich cevich requested a review from edsantiago March 22, 2022 15:35
@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

All fixed, and added some (hopefully) helpful comments.

@cevich cevich mentioned this pull request Mar 22, 2022
Merged
@edsantiago
Copy link
Member

Did you forget to push? Or perhaps your push got swallowed by the github outage?

@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

yep, swallowed. Pushed fine just now.

@cevich
Copy link
Member Author

cevich commented Mar 22, 2022

Ugh, it's also affecting Cirrus-CI apparently ☹️

edsantiago
edsantiago approved these changes Mar 22, 2022
View reviewed changes
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks a lot better except for one probably minor issue with extraction directories.

.cirrus.yml
- chmod +x podman* rootlessport
# Architecture in filename & can't use wildcards in a URL
- mkdir -p /tmp/alt
- cd /tmp/alt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous iteration had these as subdirectories of $CIRRUS_WORKING_DIR, a directory which (assuming line 829 is correct) is empty. Do we have such guarantees about /tmp/alt and /tmp/win? I kind of prefer the all-under-one-subdirectory approach.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's a fresh VM image at boot so /tmp/ will be empty except for the usual systemd stuff.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, edsantiago

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2022
@edsantiago
Copy link
Member

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 23, 2022
@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 23, 2022
@openshift-merge-robot openshift-merge-robot merged commit f049cba into containers:main Mar 23, 2022
@cevich cevich deleted the success_artifacts branch April 18, 2023 14:52
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Aug 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@edsantiago edsantiago edsantiago approved these changes

@baude baude Awaiting requested review from baude

Assignees

@edsantiago edsantiago

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cevich @edsantiago @rhatdan @openshift-merge-robot