Skip to content

Conversation

@skoech
Copy link
Collaborator

@skoech skoech commented Jul 29, 2025

Description

When a MicroCeph snap revision is promoted to stable, the corresponding Git commit is tagged manually.This PR adds a GitHub Actions workflow to help us automate updating Git tags (which we also use for marking our doc versions - and hopefully for release notes using the GitHub release feature). The workflow uses the github-script action, and will:

  1. Install the MicroCeph snap,
  2. Parse the snap info output,
  3. Get the latest stable channel,
  4. Find its commit ID,
  5. Go to the repo and find that commit hash,
  6. and tagging it.

The workflow will run on a regular cadence (twice a week).

Type of change

Delete options that are not relevant.

  • Clean code (code refactor, test updates; does not introduce functional changes)

How has this been tested?

This workflow hasn't been tested. I am hoping to test it by triggering it manually first, before enabling the schedule event and cron expressions.

Contributor checklist

Please check that you have:

  • self-reviewed the code in this PR
  • added code comments, particularly in less straightforward areas
  • checked and added or updated relevant documentation
  • checked and added or updated relevant release notes
  • added tests to verify effectiveness of this change

@skoech skoech changed the title Add workflow for tagging stable commit ci: add workflow for tagging stable commit Jul 29, 2025
@skoech skoech changed the title ci: add workflow for tagging stable commit ci: Add workflow for tagging stable commit Jul 29, 2025
Copy link
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey @skoech thanks for this!
Some minor nitpickery inline

# Allows you to run this workflow manually from the Actions tab
workflow_dispatch: null
# schedule:
# - cron: '0 0 * * MON,THU' # Runs biweekly on Tuesdays and Thursdays
Copy link
Collaborator

Choose a reason for hiding this comment

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

Eventually we will want to enable the cron line

Copy link
Contributor

Choose a reason for hiding this comment

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

why not tag every push to release branch with the snap artefact they be building ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Eventually we will want to enable the cron line

Yes, I only disabled it for testing purposes.

Copy link
Collaborator Author

@skoech skoech Aug 2, 2025

Choose a reason for hiding this comment

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

why not tag every push to release branch with the snap artefact they be building ?

I don't quite understand what you mean by this, @UtkarshBhatthere . Could you elaborate some more? :)
The main aim of this job is only to tag the stable commit, how would tagging every push to the release branch work?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can keep the "tag all pushes" thing to another PR. For documentation purposes, tagging the stable release commit alone is sufficient.

runs-on: ubuntu-latest
permissions:
contents: read # Needed for commit search via the API
# Steps represent a sequence of tasks that will be executed as part of the job
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aestethic/readability nitpickery: in general it would be good to line up comments similarly to the workflow statements (here as well as below)

# -----------------------------------------------------------
# 2. Find the first name under “channels:”
# 3. and then parse <release version> and <commit ID>
# -----------------------------------------------------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this comment accurate? Its not looking for the first but the first line that contains stable right?

Copy link
Collaborator Author

@skoech skoech Aug 4, 2025

Choose a reason for hiding this comment

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

Right, first line that includes /stable:, thanks!

uses: actions/github-script@v7
with:
script: |
const { execSync } = require('child_process');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent needs one more whitespace :-)

core.setFailed('Failed to parse channel line');
return;
}
const [, flavour, version, commit] = m;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Naming nit: not sure about flavour, maybe channel?

Copy link

Choose a reason for hiding this comment

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

"flavour" here is grabbing the unique "name" assigned to the release; which is not technically the full "channel" name. :)

Copy link
Collaborator Author

@skoech skoech Aug 4, 2025

Choose a reason for hiding this comment

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

@AnneCYH, you're right. It's taking the code name, i.e. Squid.

core.info(`Channel line: "${channelRaw}"`);
// Parse version, and commit ID
const m = channelRaw.match(
/^\s*([^/]+)\/stable:\s+([^+\s]+)\+snap([a-f0-9]+)\s/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest to tighten RE a bit:

  • channel name [a-z]+
  • version: [0-9.]+

- name: Verify commit exists
uses: actions/github-script@v7
with:
script: |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent one less whitespace

);
const hit = commits.find(c => c.sha.startsWith(target));
if (hit) {
core.info(`✅ Found commit: ${hit.sha} — ${hit.html_url}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally I try to keep source code ASCII just for simplicity

Copy link
Collaborator Author

@skoech skoech Aug 2, 2025

Choose a reason for hiding this comment

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

Haha, I'll get rid of the emojis. :D

Copy link
Contributor

Choose a reason for hiding this comment

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

😢

@skoech skoech force-pushed the stable-commit-tagging-workflow branch from 802fe6c to 02fee74 Compare August 4, 2025 16:48
skoech added 4 commits August 7, 2025 18:05
Signed-off-by: Sharon Koech <sharon.koech@canonical.com>
Signed-off-by: Sharon Koech <sharon.koech@canonical.com>
Signed-off-by: Sharon Koech <sharon.koech@canonical.com>
Signed-off-by: Sharon Koech <sharon.koech@canonical.com>
@skoech skoech force-pushed the stable-commit-tagging-workflow branch from b41f2b2 to 0caeeac Compare August 7, 2025 15:05
Copy link
Collaborator

@sabaini sabaini left a comment

Choose a reason for hiding this comment

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

Hey thanks again -- some commentary / requests inline 🙏

# Allow manual trigger from Actions UI
workflow_dispatch:
schedule:
- cron: '0 0 * * MON,THU' # Runs on Tuesdays and Thursdays at midnight UTC
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment not accurate

# Install the MicroCeph snap
- name: Install MicroCeph snap
run: |
sudo snap install microceph
Copy link
Collaborator

Choose a reason for hiding this comment

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

For debugging purposes I'd suggest to run snap info microceph right here

core.setFailed('Could not find "channels:" in snap info output');
return;
}
// Get the first non-empty channel line after the header
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment not accurate

}
// Get the first non-empty channel line after the header
const channelRaw = lines.slice(headerIdx + 1)
.find(l => l.includes('/stable:'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This finds the first line that has "/stable" in it. Can we assume that this is the correct stable release though, can we rely on the ordering?

When I run snap info microceph I get

...
channels:
  squid/stable:     19.2.0+snapab139d4a1f 2025-06-26 (1393) 117MB -
  squid/candidate:  19.2.0+snap4910cfbd93 2025-07-15 (1407) 117MB -
  squid/beta:       ↑                                             
  squid/edge:       19.2.1+snap54a98b3cec 2025-08-07 (1464) 118MB -
  latest/stable:    18.2.4+snapc9f2b08f92 2024-09-19 (1139) 102MB -
  latest/candidate: ↑                                             
  latest/beta:      ↑                                             
  latest/edge:      19.2.1+snap54a98b3cec 2025-08-07 (1464) 118MB -
  reef/stable:      18.2.4+snapc9f2b08f92 2024-09-02 (1139) 102MB -
  reef/candidate:   18.2.4+snapa97ae91192 2024-11-12 (1234) 102MB -
  reef/beta:        ↑                                             
  reef/edge:        ↑                                             
  quincy/stable:    0+git.4a608fc         2024-01-10  (793)  96MB -
...

So here the first line with squid/stable is indeed correct. Can we rely on that ordering though?

The other issue I see here is that we're only handling the /stable releases in the main branch (ie. the latest major release). It would be great if we could also auto-tag other stable releases -- at the moment this would be squid, reef, quincy. Those would correspond to the branches main, reef and quincy respectively.

Maybe the workflow could loop through all the foo/stable channels, exclude the one named "latest/stable" and look to tag in the main and named branches.

core.setFailed('Failed to parse channel line');
return;
}
const [, codeName, version, commit] = m;
Copy link
Contributor

Choose a reason for hiding this comment

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

is codename here referring to the release channel name ? (like squid, or quincy etc ?).

const commitSha = '${{ steps.verify.outputs.full_sha }}';
const codeName = '${{ steps.snap.outputs.codeName }}';
const version = '${{ steps.snap.outputs.version }}';
const stableTag = `v${version}+${codeName}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

So from what I've read above, if you use v${Version}+${codeName} as the tag, an example tag could look like:
-> v19.2.0+squid right ?

This has a slight problem that the tag will change everytime the underlying ceph point release changes. for example: in future the tag could become:
-> v19.2.2+squid.

Now since the tag is changing based on ceph-release, one of us would have to manually go and pull a new version on the RTD dashboard based on the new tag.

I propose we do not use the full semantic version but just the constant bit (something like v19+squid) as it will stay constant throughout the squid release life. And every time this workflow re-tags a commit, the documentation will update automatically.

Copy link
Contributor

@UtkarshBhatthere UtkarshBhatthere left a comment

Choose a reason for hiding this comment

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

some thoughts and nitpicking.

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.

5 participants