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

Devops/fairspc 57 GitHub actions #45

Merged
merged 6 commits into from
Apr 8, 2024
Merged

Conversation

tgreenwood
Copy link
Contributor

No description provided.

@tgreenwood tgreenwood added the enhancement New feature or request label Apr 3, 2024
@tgreenwood tgreenwood self-assigned this Apr 3, 2024
@tgreenwood tgreenwood force-pushed the devops/FAIRSPC-57_github_actions branch from acb5865 to 47ef222 Compare April 3, 2024 21:49
@tgreenwood tgreenwood force-pushed the devops/FAIRSPC-57_github_actions branch from 47ef222 to 5ec26eb Compare April 3, 2024 21:50
run: |
BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}
echo "Building images from the branch: $BRANCH"
VER=$(cat ./projects/jupyterhub-hub/VERSION)
Copy link
Member

@ewelinagr ewelinagr Apr 5, 2024

Choose a reason for hiding this comment

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

it looks like we've been using the main project ./VERSION file for image tags, I see that the project specific ones haven't been updated for 5 years...
I think it makes sense to have single version file for this repo (and to remove project specific ones), otherwise it will be easy to create inconsistencies and confusion which hub image works with which singleuser one, keep it also consistent with helm chart version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree, done

run: |
BRANCH=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}
echo "Building images from the branch: $BRANCH"
VER=$(cat ./projects/jupyterhub-singleuser/VERSION)
Copy link
Member

Choose a reason for hiding this comment

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

same here, see the comment about repo version file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ewelinagr
Copy link
Member

Similar case as for Keycloak repo - it would be nice to have some build on PR, at least checking if image build is not failing, when creating a PR. That was the case with Travis.

Copy link

sonarcloud bot commented Apr 5, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@tgreenwood
Copy link
Contributor Author

Similar case as for Keycloak repo - it would be nice to have some build on PR, at least checking if image build is not failing, when creating a PR. That was the case with Travis.

@ewelinagr agree with you, reasonable, made the push step conditional

@ewelinagr
Copy link
Member

ewelinagr commented Apr 8, 2024

Similar case as for Keycloak repo - it would be nice to have some build on PR, at least checking if image build is not failing, when creating a PR. That was the case with Travis.

@ewelinagr agree with you, reasonable, made the push step conditional

Sounds good! It looks like the condition on helm chart pushing works, but from the docker action log it looks like the images were pushed anyway. Maybe it is just the way the jobs are grouped now. For helm pushing is separate action, for docker build and push is together so it is shown as done

@tgreenwood
Copy link
Contributor Author

Similar case as for Keycloak repo - it would be nice to have some build on PR, at least checking if image build is not failing, when creating a PR. That was the case with Travis.

@ewelinagr agree with you, reasonable, made the push step conditional

Sounds good! It looks like the condition on helm chart pushing works, but from the docker action log it looks like the images were pushed anyway. Maybe it is just the way the jobs are grouped now. For helm pushing is separate action, for docker build and push is together so it is shown as done

@ewelinagr it might look confusing, I understand, but yes, it is done because it is combined step including both building and pushing. There is a conditional property push: ${{github.ref == 'refs/heads/release' || github.ref == 'refs/heads/dev'}}, added on Friday. Once it is true, the built image is pushed as well. Checked just in case, it works correctly

@tgreenwood tgreenwood merged commit ac15020 into dev Apr 8, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants