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

[ci]: skipping debug builds when not making a release #4496

Merged
merged 4 commits into from
Jun 4, 2023
Merged

[ci]: skipping debug builds when not making a release #4496

merged 4 commits into from
Jun 4, 2023

Conversation

psk001
Copy link
Contributor

@psk001 psk001 commented Jun 2, 2023

Adjusted the build scripts to skip building debug versions unless the GH actions runs for a release by including a SKIP_DEBUG variable in command.

Resolves #4471

@psk001 psk001 requested a review from a team as a code owner June 2, 2023 16:58
@psk001 psk001 requested a review from yurishkuro June 2, 2023 16:58
Signed-off-by: pushpak <puspak9208@gmail.com>

fix(docker img build): skipping debug builds when not a release
@psk001
Copy link
Contributor Author

psk001 commented Jun 2, 2023

@yurishkuro please review the PR and suggest changes.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Another place where debug images are built is scripts/build-upload-docker-images.sh

Note that that script already accepts pr-only argument, so we can adjust it directly to exclude debug logic for pr-only, without any changes to the workflows. And for consistency we should implement a similar flag for scripts/build-all-in-one-image.sh, instead of the new SKIP_DEBUG variable.

#build all-in-one-debug image and upload to dockerhub/quay.io
bash scripts/build-upload-a-docker-image.sh -b -c all-in-one-debug -d cmd/all-in-one -t debug
else
echo "Skipping debug commands"
Copy link
Member

Choose a reason for hiding this comment

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

looks like this should be in the then part

@@ -48,3 +48,4 @@ jobs:
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
SKIP_DEBUG: ${{inputs.SKIP_DEBUG}}
Copy link
Member

Choose a reason for hiding this comment

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

We don't run actions manually. The main release is done by .github/workflows/ci-release.yml, so there it should be enabling debug builds, and here they should be explicitly disabled.

@psk001
Copy link
Contributor Author

psk001 commented Jun 4, 2023

Got them all, working on it

@codecov
Copy link

codecov bot commented Jun 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (c79cee3) 97.05% compared to head (82f0a7c) 97.06%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4496   +/-   ##
=======================================
  Coverage   97.05%   97.06%           
=======================================
  Files         300      300           
  Lines       17813    17813           
=======================================
+ Hits        17289    17290    +1     
+ Misses        420      419    -1     
  Partials      104      104           
Flag Coverage Δ
unittests 97.06% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yurishkuro yurishkuro changed the title fix(docker img build): skipping debug builds when not making a release [ci]: skipping debug builds when not making a release Jun 4, 2023
@yurishkuro yurishkuro enabled auto-merge (squash) June 4, 2023 05:07
@yurishkuro yurishkuro merged commit a18af23 into jaegertracing:main Jun 4, 2023
yurishkuro added a commit that referenced this pull request Aug 18, 2023
## Which problem is this PR solving?
- There was a bug introduced in #4496 where `pr-only` mode was used
unconditionally, instead of for PRs-only.

## Description of the changes
- fork the build step into on-main and no-on-main

Signed-off-by: Yuri Shkuro <github@ysh.us>
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.

[Feature]: Split main & debug image building CI steps
2 participants