jenkins-infra / github-reusable-workflows

Repository for reusable workflows
MIT License
1 stars 6 forks source link

setup maven cd workflow #1

Closed jetersen closed 2 years ago

jetersen commented 2 years ago

fixes https://github.com/jenkins-infra/jenkins-maven-cd-action/issues/10

jetersen commented 2 years ago

@lemeurherve who had you intended to review this, with write access 😅

jetersen commented 2 years ago

Seems to work as intended: First use: https://github.com/jenkinsci/gitlab-api-plugin/actions/runs/2317760237 Making a release: https://github.com/jenkinsci/gitlab-api-plugin/actions/runs/2317774691 Actual release: https://github.com/jenkinsci/gitlab-api-plugin/releases/tag/5.0.1-72.vb_8c272862e86

jetersen commented 2 years ago

@jglick the question becomes do we want versioning on our reusable workflows?

My suggestion has been to point to main and never look back. Because it allows infra team to fix the workflow if something broke and distribute fixes to workflows faster.

timja commented 2 years ago

@jglick the question becomes do we want versioning on our reusable workflows?

My suggestion has been to point to main and never look back. Because it allows infra team to fix the workflow if something broke and distribute fixes to workflows faster.

pinning to main should be fine for this, you can always pin to a commit if something breaks.

timja commented 2 years ago

cc @notmyfault and @olamy who were looking at similar for release drafter and maybe crowdin

daniel-beck commented 2 years ago

pinning to main should be fine for this, you can always pin to a commit if something breaks.

Better hope you never have to evolve a workflow in a backward incompatible way. Took me just 11 days for jenkins-security-scan 😅

timja commented 2 years ago

pinning to main should be fine for this, you can always pin to a commit if something breaks.

Better hope you never have to evolve a workflow in a backward incompatible way. Took me just 11 days for jenkins-security-scan 😅

we do the same with buildPlugin. Generally send PRs to consumers.

Not ideal but a lot less maintenance for maintainers.

Far less dependabot spam

timja commented 2 years ago

@daniel-beck reminded me that you can move tags around etc.

What he did with security scan was have a v1 tag and then when he did a breaking change he added a v2 tag.

which may be a better option?

daniel-beck commented 2 years ago

What he did with security scan was have a v1 tag and then when he did a breaking change he added a v2 tag.

And I kept updating each tag to point to the newest commit that made sense. So some management overhead is involved.

Only makes sense when you have one repo per workflow though.

jglick commented 2 years ago

I (probably) would recommend pinning versions for buildPlugin. I do not recommend it here, for the same reason discussed recently w.r.t. foreign GHA in the CD workflow like actions/checkout: the effectiveness of the CD process is only tested if and when you release, so pinning a version would just mean that DB offers updates in PRs which would always pass (assuming no flaky tests or infra) even if they are actually broken.

jglick commented 2 years ago

Pinning client repos to @v1 would be fine I guess—we would not expect that to ever be edited, nor would we expect DB to offer a bump to v2.

jglick commented 2 years ago

Only makes sense when you have one repo per workflow though.

Either that, or use a orphaned branch per workflow, or include the workflow name in tags.

I tend to agree that using a repo dedicated to this workflow would make more sense from a versioning perspective. Maybe we can just archive this repo and use jenkins-maven-cd-action, as the home of the most salient action in the workflow? In which case the workflow and this third action would share tags?

daniel-beck commented 2 years ago

use a orphaned branch per workflow

I expect that may be a barrier to successfully contributing for some people unfamiliar with those; GitHub seems to be pushing "just dump everyone on the default branch" hard. (I may well be wrong though, maintainers seem to handle the weird default orphan branch of private CERT "fork" repos well.)

as the home of the most salient action in the workflow? In which case the workflow and this third action would share tags?

FWIW I considered something like that for jenkins-security-scan and ended up with separate repos for actions and workflows to limit weirdness of version combinations -- which version of each action would be checked out by the workflow? How would I do evolution of the action that requires a corresponding change in the workflow? Having a separate upstream/producer (actions) and downstream/consumer (workflow) seemed more straightforward.

jetersen commented 2 years ago

I would keep this repository and have multiple workflows hosted behind main or v1. No need to make it more complicated. Again I would compare shared pipeline library to reusable workflows.

Keeps maintenance simple and separates consumer and producer as noted by @daniel-beck

jglick commented 2 years ago

Keeps maintenance simple

Unless and until we find the need to make some incompatible change to the workflow that would affect so many repos we cannot feasibly update them all promptly enough.

Yes we have avoided any such change to buildPlugin()—we have managed to deprecate some things (javaLevel for example) and roll out changes incrementally. So maybe only a theoretical problem.

jetersen commented 2 years ago

Theoretical problem, I am pretty sure we can make something that can also solve that.

Use workflow inputs, create another reusable workflow, ask users to stay on v1 while we move on in v2 or they could self host their special workflow.

This solves a immediate problem. I don't want to speculate too much future problems or never problems.

This is simply hosting a group of actions in a workflow and handling updates dependabot noise, if anything the actions are more likely to run into this issue.

jglick commented 2 years ago

create another reusable workflow

Indeed that would be the most straightforward approach.

jglick commented 2 years ago

Filed https://github.com/jenkinsci/jenkins-infra-test-plugin/pull/20 as a sanity check. I think we can move ahead with this?

daniel-beck commented 2 years ago

IIRC both the workflow template as well as the reusable workflow need to declare permissions, otherwise commits from Dependabot will fail the run because the token lacks permissions. See https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions#changing-github_token-permissions and https://github.com/jenkins-infra/jenkins-security-scan/pull/7 / https://github.com/jenkinsci/.github/pull/75 for when I did this on Jenkins Security Scan workflows.

Mostly cosmetic here because these are unlikely to create releases, but still FYI.

jglick commented 2 years ago

https://github.com/jenkinsci/jenkins-infra-test-plugin/pull/20#issuecomment-1135195723 and then https://github.com/jenkinsci/workflow-aggregator-plugin/runs/6573189329?check_suite_focus=true#step:4:14 again failed to create a release, due to what I suspect is a race condition. Can we try introducing a delay between Release Drafter and the interesting categories action?