Correcting dockerize job to run when a release is created #2800

jmherbst · 2021-10-15T15:04:58Z

I believe this step has been broken for a while. The latest tagged docker container on hub is v2.7.5

Looking at the actions log from latest release commit https://github.com/scalameta/scalafmt/runs/3777211161

It shows that the artifact was uploaded in the CI:release run, but the dockerize job was not executed.

The dockerize job was executed in the CI:Push run, but the artifact isn't uploaded in that run, so the dockerize step fails

I think this change will correct this behavior, so that when future v tag'd releases are created, it will upload the artifact, and then dockerize will run.

kitbellew · 2021-10-15T16:21:01Z

.github/workflows/ci.yml

@@ -98,7 +98,7 @@ jobs:
  dockerize:
    needs: [native-image,test]
    runs-on: ubuntu-latest
-    if: startsWith(github.ref, 'refs/tags/v') && github.event_name != 'release'
+    if: startsWith(github.ref, 'refs/tags/v') && github.event_name == 'release' && github.event.action == 'created'


are you sure about this? before, it ran if it was not for a release. now it will no longer run for those cases.

should it be event_name == release || event_action == created?

I'm a bit confused as to why this was to run only if not release, yet the ref is a release version tag?

I used event_name == release && event.action == 'created' because the doc here suggests that release event type has many activity types and I figured this would protect it from running too often

Especially confused because the release artifact is only uploaded when event_name == 'release' https://github.com/scalameta/scalafmt/pull/2800/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR89

Unfortunately, I don't remember the reason for this configuration. It may conflict with upload release assets step at line 85. But I suggest to merge it and see what happens.

my question is this: did this line ever run before? if it did, it will no longer run now. is that ok?

This line was changed at 2020-07-24 (20322e8) and last docker image was published at 2020-10-16. So, yes, it used to work until v3.0.0-RC1 release.

we have tried publishing in two ways: by creating a tag locally and pushing it (works, and triggers a release build), or by editing the draft via the ui (doesn't work, no release build is triggered). with one of the recent releases, i forgot and tried the ui way, then repushed the local tag. the reason i suggested the OR approach is that it extends the rule rather than switching it to something completely different.

startsWith(github.ref, 'refs/tags/v') && github.event_name != 'release' || github.event_name == 'release' && github.event.action == 'created'

may be?

exactly! but can be simplified as

startsWith(github.ref, 'refs/tags/v') && (github.event_name != 'release' || github.event.action == 'created')

removing ... == 'release'

Alright @kitbellew and @poslegm - I did some more digging into this today. Turns out that I may have caused more confusion than help with suggesting we use == 'created'.

It looks like what caused this was Dependabot's update of our build-push-action from v1 to v2+
3801986#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f

That update broke our uploads. The errors we're seeing on the Dockerize Step(link) are due to build-push-action@v2 using buildx context and so the docker build context did not properly include the download-artifact step we do immediately before.

I'm going to add a commit(fcfd249) to this PR to address this finding

kitbellew · 2021-10-15T16:55:21Z

@tgodzik @poslegm can you please take a look?

tgodzik · 2021-10-15T19:01:52Z

I can take a look at it next week, but I am not too familiar with the docker stuff. The change does look sensible though.

poslegm · 2021-10-17T21:45:26Z

.github/workflows/ci.yml

@@ -98,7 +98,7 @@ jobs:
  dockerize:
    needs: [native-image,test]
    runs-on: ubuntu-latest
-    if: startsWith(github.ref, 'refs/tags/v') && github.event_name != 'release'
+    if: startsWith(github.ref, 'refs/tags/v') && github.event_name == 'release' && github.event.action == 'created'


Unfortunately, I don't remember the reason for this configuration. It may conflict with upload release assets step at line 85. But I suggest to merge it and see what happens.

… usage

kitbellew · 2021-10-18T20:08:57Z

.github/workflows/ci.yml

@@ -100,15 +100,21 @@ jobs:
    runs-on: ubuntu-latest
    if: startsWith(github.ref, 'refs/tags/v') && github.event_name != 'release'
    steps:
-      - uses: actions/checkout@v2
-      - run: git fetch --unshallow


do we not need this line anymore?

I don't think so - The Dockerfile doesn't use the checkout at all (it's only copy'ing over the artifact )

The only reason I'm including the checkout at all is because the build-push-action docs mentioned it in "Path" context which was the way I was able to get it working again

jmherbst · 2021-10-19T17:02:35Z

Hey, All 🟢 ! Thanks for re-running the tests @kitbellew.

jmherbst · 2021-10-19T17:30:47Z

Awesome, thanks @kitbellew ! Looking forward to keeping an eye on the next release to see how this does.

P.S.: Any chance you'd be able to apply the hacktoberfest-accepted label to this PR for me to participate in that event?

kitbellew · 2021-10-19T17:52:34Z

Awesome, thanks @kitbellew ! Looking forward to keeping an eye on the next release to see how this does.

P.S.: Any chance you'd be able to apply the hacktoberfest-accepted label to this PR for me to participate in that event?

how do i apply the label? :)

jmherbst · 2021-10-19T17:56:25Z

Hah, honestly - no idea - I figured if you had merge access you'd also have access to create + apply labels to PRs?

kitbellew · 2021-10-19T18:05:21Z

Hah, honestly - no idea - I figured if you had merge access you'd also have access to create + apply labels to PRs?

i probably have access but i have never done it so i don't know which steps to take :)

kitbellew · 2021-10-20T16:28:45Z

@jmherbst could you please take a look:

i pushed a new v3.0.7 tag; this resulted in two builds:
- CI (failed): https://github.com/scalameta/scalafmt/actions/runs/1364303766 (failed)
- Release: https://github.com/scalameta/scalafmt/actions/runs/1364303764
then edited a release via the ui; this resulted in one build:
- CI: https://github.com/scalameta/scalafmt/actions/runs/1364306322

So, comparing to what happened with v3.0.6, I don't see a difference. Dockerize still failed.

jmherbst · 2021-10-20T16:57:42Z

Sure, taking a look @kitbellew

Correcting dockerize job to run when a release is created

f2fad5c

kitbellew reviewed Oct 15, 2021

View reviewed changes

poslegm approved these changes Oct 17, 2021

View reviewed changes

Updating our build-push-action configuration to be correct for our V2…

fcfd249

… usage

kitbellew reviewed Oct 18, 2021

View reviewed changes

kitbellew merged commit bbda2e4 into scalameta:master Oct 19, 2021

jmherbst deleted the fix-release-dockerize branch October 19, 2021 17:30

kitbellew added the hacktoberfest-accepted label Oct 19, 2021

jmherbst mentioned this pull request Oct 20, 2021

Updating dockerize CI step to properly use docker/metadata-action instead of generating tags with shell cmnds #2808

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correcting dockerize job to run when a release is created #2800

Correcting dockerize job to run when a release is created #2800

jmherbst commented Oct 15, 2021

kitbellew Oct 15, 2021 •

edited

Loading

jmherbst Oct 15, 2021

jmherbst Oct 15, 2021

poslegm Oct 17, 2021

kitbellew Oct 17, 2021

poslegm Oct 17, 2021

kitbellew Oct 17, 2021

poslegm Oct 17, 2021

kitbellew Oct 17, 2021 •

edited

Loading

jmherbst Oct 18, 2021 •

edited

Loading

kitbellew commented Oct 15, 2021

tgodzik commented Oct 15, 2021

poslegm Oct 17, 2021

kitbellew Oct 18, 2021

jmherbst Oct 18, 2021 •

edited

Loading

jmherbst commented Oct 19, 2021

jmherbst commented Oct 19, 2021

kitbellew commented Oct 19, 2021

jmherbst commented Oct 19, 2021

kitbellew commented Oct 19, 2021

kitbellew commented Oct 20, 2021

jmherbst commented Oct 20, 2021

Correcting dockerize job to run when a release is created #2800

Correcting dockerize job to run when a release is created #2800

Conversation

jmherbst commented Oct 15, 2021

kitbellew Oct 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitbellew Oct 17, 2021 • edited Loading

Choose a reason for hiding this comment

jmherbst Oct 18, 2021 • edited Loading

Choose a reason for hiding this comment

kitbellew commented Oct 15, 2021

tgodzik commented Oct 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmherbst Oct 18, 2021 • edited Loading

Choose a reason for hiding this comment

jmherbst commented Oct 19, 2021

jmherbst commented Oct 19, 2021

kitbellew commented Oct 19, 2021

jmherbst commented Oct 19, 2021

kitbellew commented Oct 19, 2021

kitbellew commented Oct 20, 2021

jmherbst commented Oct 20, 2021

kitbellew Oct 15, 2021 •

edited

Loading

kitbellew Oct 17, 2021 •

edited

Loading

jmherbst Oct 18, 2021 •

edited

Loading

jmherbst Oct 18, 2021 •

edited

Loading