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

Release redo #3710

Merged
merged 2 commits into from
Aug 29, 2019
Merged

Release redo #3710

merged 2 commits into from
Aug 29, 2019

Conversation

cevich
Copy link
Member

@cevich cevich commented Aug 2, 2019

The initial implementation was far more complicated than necessary.
Strip out the complexities in favor of a simpler and more direct
approach.

Note: The new cross-compile tasks will likely fail when this
merges, due to the container image not being built yet
(they're defined in this PR). Re-running them after 30m or so
should allow them to pass.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 2, 2019
@cevich
Copy link
Member Author

cevich commented Aug 2, 2019

@jwhonce @baude @mheon the 'release' task is badly broken and far too complex. This is the beginnings (somewhat incomplete / untested) re-implementation. Key elements are:

  • No more 'release' task
  • Fixed & re-purposed (broken) 'brew-pkg' Makefile target for windows and darwin
  • Produce the release archives directly in the testing tasks (cross-compile-test for windows|darwin)
  • Use the just built & tested podman, with a purpose-built container, to upload the archives.
  • For podman on linux, make a tarball
  • For podman-remote on windows/darwin, use a zip file

PTAL at the Makefile - it seems to work for me. Better this way?

Should I put back the old 'brew-pkg' target, and if so, does the archive-subdirectory have to be 'brew'?

@cevich cevich force-pushed the release_redo branch 2 times, most recently from 4ec7964 to 6933f67 Compare August 6, 2019 18:39
@jwhonce
Copy link
Member

jwhonce commented Aug 6, 2019

@baude , If @cevich is going through all this work should a linux podman-remote tarball also be created for completeness?

@cevich
Copy link
Member Author

cevich commented Aug 7, 2019

going through all this work

Arguably it's necessary work, the current implementation is too complicated and broken. Simplification was demanded, but you're correct to ask "while I'm at it"...

should a linux podman-remote tarball also be created for completeness?

At this stage, this would be a nearly trivial addition.

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #3699) made this pull request unmergeable. Please resolve the merge conflicts.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2019
@cevich cevich force-pushed the release_redo branch 16 times, most recently from 9e7c621 to 8ed4240 Compare August 21, 2019 15:26
@cevich cevich changed the title WIP: Release redo Release redo Aug 21, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 23, 2019
@ashley-cui
Copy link
Member

@cevich I would prefer if the subdir would not be named podman-remote, since from a user on the Mac's point of view, podman is just podman, and not very remote. Shouldn't be a big deal though, the version'd folder could work fine.
I'm not really familiar with webhooks, but it would sound ideal if podman in homebrew could be updated every time a version is released. The webhook would need to edit a ruby file and open a PR for homebrew. If this is possible, let me know and I can take a look at it or something. (probably would need help though)

@cevich
Copy link
Member Author

cevich commented Aug 23, 2019

I would prefer if the subdir would not be named podman-remote, since from a user on the Mac's point of view, podman is just podman, and not very remote. Shouldn't be a big deal though, the version'd folder could work fine.

oh okay, no problem, I put it back to 'podman-$VERSION'. I'd like to include the version number in the sub-dir name, since for humans that prevents clashes when unzipping. However, it can be a pain for automation to deal with, since the number will inevitably change.

If you have a mac, take a peek and make sure this works: https://storage.cloud.google.com/libpod-pr-releases/libpod-remote-pr3710-darwin---amd64.zip

I'm not really familiar with webhooks, but it would sound ideal if podman in homebrew could be

They're simple in concept: It's just a URL that automation would send some custom JSON to. In other words, it's just a REST API endpoint. I'm not familiar with homebrew to know if this is supported or not. It's fairly common for SaaS tools to have them, so worth checking their docs.

updated every time a version is released. The webhook would need to edit a ruby file and open a PR for homebrew. If this is possible, let me know and I can take a look at it or something. (probably would need help though)

A webhook is a one-shot deal, though the JSON can be whatever is required. If homebrew does not support webhooks, it is possible ti bolt-on automation to do what you describe. The down-sides are:

  • It doesn't care if there's a human on the other end or not. It will gladly open up 10,000 PRs. This could be limited to only running for tagged-releases (every few weeks), but it still will not care if it's 1 or 100 (if something goes wrong).

  • it's somewhat involved (from a security standpoint) to (safely) automate opening of PRs. I know how to do it, but it's likely a week or two worth of work (including testing), so we should try to avoid this if possible.

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #3755) made this pull request unmergeable. Please resolve the merge conflicts.

@ashley-cui
Copy link
Member

ashley-cui commented Aug 26, 2019

oh okay, no problem, I put it back to 'podman-$VERSION'. I'd like to include the version number in the sub-dir name, since for humans that prevents clashes when unzipping. However, it can be a pain for automation to deal with, since the number will inevitably change.

If you have a mac, take a peek and make sure this works: https://storage.cloud.google.com/libpod-pr-releases/libpod-remote-pr3710-darwin---amd64.zip

It looks like the docs are missing in the zip file, other than podman.1. It should have all the relevant docs generated by the remote-docs script.

A webhook is a one-shot deal, though the JSON can be whatever is required. If homebrew does not support webhooks, it is possible ti bolt-on automation to do what you describe.

Okay, I think it's just for the best to open the PR's manually once a new version is released, but we can definitely configure a webhook to upload the zip file asset triggered by a new release

@cevich
Copy link
Member Author

cevich commented Aug 26, 2019

It looks like the docs are missing in the zip file, other than podman.1.

Oh, thought it was expected to be that way 😄 I'm a bit worried about the size of this PR, want me to fix that here or can do it in a separate one?

Okay, I think it's just for the best to open the PR's manually once a new version is released

At first, for sure yes. Bother me after a while, if you find that too tedious, and I'll help try and figure something out or dig into docs more. It's definitely possible to trigger "stuff" from libpod CI running on master only when a new release tag is added. So we just need to figure out what "stuff" isn't an overwhelming amount of work 😄

Makefile Show resolved Hide resolved
@rhatdan
Copy link
Member

rhatdan commented Aug 26, 2019

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

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

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2019
@ashley-cui
Copy link
Member

ashley-cui commented Aug 26, 2019

Oh, thought it was expected to be that way I'm a bit worried about the size of this PR, want me to fix that here or can do it in a separate one?

Either works for me 😄

At first, for sure yes. Bother me after a while, if you find that too tedious, and I'll help try and figure something out or dig into docs more. It's definitely possible to trigger "stuff" from libpod CI running on master only when a new release tag is added. So we just need to figure out what "stuff" isn't an overwhelming amount of work

I believe the workflow should be this: libpod CI is triggered when a new release tag is added, and uploads zip file as a release asset via github api 1 2. Then, to get this zip file into homebrew, we need to fork the brew repo, update it, and open a pr against their master. The brew part is what would happen by hand for now. If you have any ideas on how to automate the last part, let me know and maybe we can figure this out together? 😊

@rhatdan
Copy link
Member

rhatdan commented Aug 27, 2019

LGTM
@cevich @ashley-cui Is this ready to go in?

@cevich
Copy link
Member Author

cevich commented Aug 28, 2019

let me know and maybe we can figure this out together

@ashley-cui Yes, that's the workflow I have in mind, and yes, I'm happy to help. This PR ready to go in then, so we can get started on the followup items?

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #3889) made this pull request unmergeable. Please resolve the merge conflicts.

@cevich
Copy link
Member Author

cevich commented Aug 28, 2019

Opened #3899 as followup for docs

cevich added 2 commits August 28, 2019 11:53
The initial implementation was far more complicated than necessary.
Strip out the complexities in favor of a simpler and more direct
approach.

Signed-off-by: Chris Evich <[email protected]>
@baude
Copy link
Member

baude commented Aug 29, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 29, 2019
@openshift-merge-robot openshift-merge-robot merged commit ab5f52c into containers:master Aug 29, 2019
@cevich cevich deleted the release_redo branch June 30, 2021 18:04
@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 Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

8 participants