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

docker: CI update and simplification #3075

Merged
merged 8 commits into from Aug 18, 2023

Conversation

mmacata
Copy link
Contributor

@mmacata mmacata commented Jul 7, 2023

!! Description updated to reflect merged status after discussions!!

This PR simplifies the github workflow for docker build, tag and push.
Due to some tag restructuring and new features of docker/metadata-action it now only uses one job and, depending on the trigger action, creates different tags. It will create four different types of tags:

  • latest for latest release (at this time 8.3) and with ubuntu os
  • current-{{ os }} for a still hard-coded branch, here update to releasebranch_8_3, e.g current-ubuntu
  • {{ branch }}-{{ os }} for all configured branches, e.g. releasebranch_8_3-alpine, main-debian
  • {{ tag }}-{{ os }} for all published releases. e.g. 8.3.0-alpine

This new naming conforms more with the GRASS GIS naming style of versions. While naming of branches and tags is still the same, it adds current-{{ os }} and abandons former naming style with latest-{{ os }} and stable-{{ os }} like latest-alpine and stable-alpine to avoid confusion. Also content of latest tag was changed from main branch to latest release.

The plain latest tag is still created because it is default for docker pull osgeo/grass-gis and would be nice to have, although usage of it is not recommended. No strong opinion if it should point to main branch or releasebranch_8_3. Open for discussion. See also OSGeo/grass-website#371

As the action was triggered twice for a release, on: [tags ](tags: ['*.*.*']) was outcommented.

Whole thing was tested as can be seen in first commit of this PR on a branch named docker-workflow-update with mmacata remote and dockerhub account. Created and pushed image tags can be seen here on dockerhub.

@wenzeslaus
Copy link
Member

While the main branch of GRASS GIS is fairly stable, I think it is more expected that it is the latest release rather than the latest state of the code.

@mmacata
Copy link
Contributor Author

mmacata commented Aug 15, 2023

Updated workflow to build and push latest tag for latest stable release only. Tested again with mmacata remote.

@neteler neteler added docker Docker related CI Continuous integration backport to 8.3 PR needs to be backported to release branch 8.3 backport to 8.2 PR needs to be backported to release branch 8.2 labels Aug 16, 2023
@neteler neteler added this to the 8.4.0 milestone Aug 16, 2023
@mmacata
Copy link
Contributor Author

mmacata commented Aug 16, 2023

@wenzeslaus I updated the file according to your suggestion. Do you agree now?

Copy link
Member

@neteler neteler left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me (but another reviewer is welcome)

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

I appreciate that this is much shorter. The need to understand the many options of docker/metadata-action is unfortunate, but seem like a better alternative to the long custom code.

I'm still unclear about how some of the cases will work out here. I left individual comments for that.

.github/workflows/docker.yml Outdated Show resolved Hide resolved
.github/workflows/docker.yml Show resolved Hide resolved
.github/workflows/docker.yml Show resolved Hide resolved
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

With the clarifications and plan for backporting, this makes a lot of sense and seems to fit with the GitHub and docker/metadata-action doc.

@neteler
Copy link
Member

neteler commented Aug 18, 2023

Great work, I take liberty to merge it and backport now.

@neteler neteler merged commit 14d056a into OSGeo:main Aug 18, 2023
20 checks passed
neteler pushed a commit that referenced this pull request Aug 18, 2023
* simplify docker gh workflow (see PR for details)
@neteler neteler removed the backport to 8.3 PR needs to be backported to release branch 8.3 label Aug 18, 2023
@neteler neteler modified the milestones: 8.4.0, 8.3.1 Aug 18, 2023
neteler pushed a commit that referenced this pull request Aug 18, 2023
* simplify docker gh workflow (see PR for details)
@neteler neteler removed the backport to 8.2 PR needs to be backported to release branch 8.2 label Aug 18, 2023
@neteler neteler removed this from the 8.3.1 milestone Aug 18, 2023
@neteler neteler added this to the 8.2.2 milestone Aug 18, 2023
landam pushed a commit to landam/grass that referenced this pull request Oct 25, 2023
* simplify docker gh workflow (see PR for details)
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
* simplify docker gh workflow (see PR for details)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration docker Docker related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants