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

[Workflow]: Added workflow for automation #9

Closed
wants to merge 18 commits into from

Conversation

soumyaDghosh
Copy link
Member

Process:

  1. PR merged and workflow is triggered.
  2. workflow builds and releases the sdk to a particular channel of edge. (edge-${{ github.event.number }})
  3. the content snap is then updated to that same release and released.
  4. called for testing.
  5. sdk, promoted to stable
  6. content is then pointed to stable
  7. content is released to stable

@soumyaDghosh soumyaDghosh requested review from a team, brlin-tw and merlijn-sebrechts and removed request for a team December 10, 2023 12:16
@jnsgruk
Copy link
Member

jnsgruk commented Dec 11, 2023

It feels like it might be better to split this down into two distinct sets of files:

  • Workflows that are triggered on changes to ffmpeg-2204-sdk/**
  • Workflows that are triggered on changes to ffmpeg-2204/**

At the moment, I don't think the sequencing is quite right, and we appear to adjust the versions for the same files in multiple places.

Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

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

Initial comments. Workflow should be

  1. PR merged and workflow is triggered.
  2. workflow builds and releases the sdk to a particular channel of candidate. (candidate/build-${{ github.event.number }})
  3. the content snap is then updated to that same release and built.
  4. two calls for testing.
  5. sdk, promoted to stable
  6. content is released to stable

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved
.github/workflows/promote-to-stable.yml Outdated Show resolved Hide resolved
.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved
@soumyaDghosh soumyaDghosh removed the request for review from brlin-tw December 22, 2023 07:20
Comment on lines 28 to 62
update-content-snap:
name: Update the content snap
needs: [ release-sdk, get-architectures ]
environment: "2204 Branch"
runs-on: ubuntu-latest
steps:
- name: Checkout latest commit
uses: actions/checkout@v4
with:
token: ${{ secrets.SNAPCRAFTERS_BOT_COMMIT }}
- name: Update the content snap
uses: snapcrafters/ci/sync-version@main
with:
token: ${{ secrets.SNAPCRAFTERS_BOT_COMMIT }}
snapcraft-project-root: "ffmpeg-2204"
update-script: |
sed -i 's/^\(- ffmpeg-2204-sdk\/latest\/\)\(.*\)$/\1stable/' ffmpeg-2204/snapcraft.yaml

release-content:
name: 🚢 Release to latest/stable
needs: get-architectures
runs-on: ubuntu-latest
environment: "2204 Branch"
strategy:
matrix:
architecture: ${{ fromJSON(needs.get-architectures.outputs.architectures-list) }}
steps:
- name: 🚢 Release to latest/stable
uses: snapcrafters/ci/release-to-candidate@main
with:
architecture: ${{ matrix.architecture }}
launchpad-token: ${{ secrets.LP_BUILD_SECRET }}
store-token: ${{ secrets.SNAP_STORE_CANDIDATE }}
channel: "stable"
snapcraft-project-root: "ffmpeg-2204"
Copy link
Member

Choose a reason for hiding this comment

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

I don't get what update-content-snap and release-content is doing here. The promote to stable workflow should only promote already built snaps to stable. It should not build anything.

.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved
.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved
.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved

release-content:
name: 🚢 Release to latest/edge
needs: get-architectures
Copy link
Member

Choose a reason for hiding this comment

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

This needs update-content-snap too

architecture: ${{ fromJSON(needs.get-architectures.outputs.architectures-list) }}
steps:
- name: Checkout latest commit
uses: actions/checkout@v4
Copy link
Member

Choose a reason for hiding this comment

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

This still checks out the commit associated with this action; not the latest commit. You can checkout the latest commit using this method: https://github.com/actions/checkout#checkout-head


call-for-testing:
name: 📣 Create call for testing
needs: [release-sdk, get-architectures]
Copy link
Member

Choose a reason for hiding this comment

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

This also needs release-content, so the call for testing only gets published when both SDK and content succeed

.github/workflows/promote-to-stable.yml Show resolved Hide resolved
@soumyaDghosh
Copy link
Member Author

@merlijn-sebrechts So, in the promote-to-stable.yml are you suggesting adding promote-sdk to stable and release content to stable?

@jnsgruk
Copy link
Member

jnsgruk commented Feb 21, 2024

@soumyaDghosh I saw your call for help on this on Matrix. It looks like there is a fair bit of work to do here.

The first thing I think we need to do is to have two sets of some of the workflow files that use a path filter - that way we can maintain some seperation such that certain workflows are only triggered by ffpmpeg-2204/** and others by ffmpeg-2204-sdk/**. There is an example here: https://github.com/jnsgruk/zinc-k8s-operator/blob/8f01850d652c7974830a3fd3e2fc5e3463663a38/.github/workflows/publish-oci.yaml#L6-L7

That way we can essentially control the actual release of both snaps independently, which I think is the most sensible way forward.

Of course, if one is updated and that needs to trigger the update of another, that's perfectly achievable.

@soumyaDghosh soumyaDghosh requested review from jnsgruk and removed request for merlijn-sebrechts February 24, 2024 15:14
Copy link
Member

@jnsgruk jnsgruk left a comment

Choose a reason for hiding this comment

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

Some comments to get started with; I think we'll need a few more iterations but we're making progress. Thanks!

.github/workflows/promote-content-to-stable.yml Outdated Show resolved Hide resolved
.github/workflows/promote-content-to-stable.yml Outdated Show resolved Hide resolved
.github/workflows/promote-content-to-stable.yml Outdated Show resolved Hide resolved
.github/workflows/promote-to-stable.yml Outdated Show resolved Hide resolved
.github/workflows/release-content-to-candidate.yml Outdated Show resolved Hide resolved
.github/workflows/release-content-to-candidate.yml Outdated Show resolved Hide resolved
.github/workflows/release-sdk-to-candidate.yml Outdated Show resolved Hide resolved
.github/workflows/release-to-edge.yml Outdated Show resolved Hide resolved
.github/workflows/update-content-snap.yml Show resolved Hide resolved
.github/workflows/update-sdk-snap.yml Show resolved Hide resolved
@soumyaDghosh soumyaDghosh requested a review from jnsgruk March 3, 2024 07:06
Copy link
Member

@jnsgruk jnsgruk left a comment

Choose a reason for hiding this comment

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

Looking better, a couple more changes!

channel: "candidate"
launchpad-token: ${{ secrets.LP_BUILD_SECRET }}
snapcraft-project-root: "ffmpeg-2204-sdk"
store-token: ${{ secrets.SNAP_STORE_CANDIDATE }}
Copy link
Member

Choose a reason for hiding this comment

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

This file is missing the call for testing phase

Copy link
Member Author

Choose a reason for hiding this comment

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

There is nothing to test in a sdk. The content snap is only what we can test.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but we still need a control mechanism for getting newly released revisions from candidate -> stable.

Copy link
Member Author

@soumyaDghosh soumyaDghosh Mar 4, 2024

Choose a reason for hiding this comment

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

Uhh.. we can't directly push it right? Hmm.. is it possible, to promote both to stable together? because that'd make more sense. We don't need to rebuild the content snap twice. Rather, the user needs the exactly same sdk snap, which is used in the content snap. So, promotion will promote both the snap together. Is it possible?

@@ -1,4 +1,4 @@
name: Update
name: Update SDK Snap

on:
# Runs at 10:00 UTC every day
Copy link
Member

Choose a reason for hiding this comment

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

Somewhere in this workflow you need to call the other update script.

This one runs on a cron job, if changes are made it should call the workflow to also update the content snap.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure how to do that. In the update-content-snap part, I have made it to run only when specific workflow runs

https://github.com/snapcrafters/ffmpeg-sdk/pull/9/files#diff-2e98a5de67485977aef361f5f547ec6b1ba8be7393b696ed2d8d1daf97fe499aR3-R9

and makes changes depending on the workflow. I am not sure. How to run the update workflow from within this snap. And also, if it's necessary, I think that should be ran after the sdk is published in the candidate channel.

@soumyaDghosh soumyaDghosh requested a review from jnsgruk March 4, 2024 13:10
@soumyaDghosh soumyaDghosh deleted the core22 branch March 16, 2024 08:22
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.

4 participants